w3c / web-locks

Cross-tab resource coordination API
https://w3c.github.io/web-locks/
Other
124 stars 16 forks source link

Specify bfcache behavior with proper tests #81

Open saschanaz opened 2 years ago

saschanaz commented 2 years ago

I intend to add a bfcache test where any active lock/request prevents caching, matching the current Chrome Dev 96 behavior.

Is this something you want to change or are you okay with the current Chrome behavior? To me it sounds reasonable enough, and in that case I think we should at least add a note about it, if it's not something can be specced.

inexorabletash commented 2 years ago

Yes, thank you! I think we're okay with the current Chrome behavior.

saschanaz commented 2 years ago

@inexorabletash In my observation Chrome disables bfcache at the first lock request and never enables it again, is this something Google wants to enhance or does it actually enable bfcache again?

inexorabletash commented 2 years ago

I've reached out to our BFCache folks for an opinion.

inexorabletash commented 2 years ago

Per @rakina - ideally we'd only make pages disable bfcache when it is necessary, so we could consider pages that have released all locks to be eligible for bfcache again.

So... might be a bug in Chrome (I haven't tested myself!). But let's not spec (or codify via WPT) an assumption that Chrome's current behavior is desired.

saschanaz commented 2 years ago

Cool. Gecko also wants to enable bfcache again when eligible, and I'm planning to add tests for that. It seems bfcache tests are marked as optional (by using assert_optional_implements) so I guess doing so shouldn't flag Chrome behavior as invalid, at least for now.

fergald commented 2 years ago

Yeah, it's not a bug in Chrome in the sense that it's perfectly fine to block BFCaching for any reason. It's not the situation we want long-term though, we just took the fastest path to correctness, knowing that we were over-blocking with a plan to come back and optimise.

As for the test, yes, it should give PRECONDITION_FAILED, please make use of the helpers, e.g. assert_bfcached

@hiroshige-g who is also adding BFCache tests FYI.

saschanaz commented 2 years ago

From https://github.com/WICG/web-locks/issues/78#issuecomment-920543922

Coincidently, the following guidance just got merged into the TAG design guidelines: https://w3ctag.github.io/design-principles/#support-non-fully-active

It seems this also covers bfcache which should help us spec the behavior.

hiroshige-g commented 2 years ago

Great. FYI There are also some docs about writing bfcache/navigation tests at

and some more test examples under codereview (they will be refactored before landing though):

saschanaz commented 2 years ago

Another FYI, WPT now has some bfcache tests specifically for Web Locks, e.g. https://wpt.live/web-locks/bfcache/held.tentative.https.html

rakina commented 2 years ago

It seems like we have a WPT already. Is there anything that needs to be updated on the spec side for this? BTW there's a guide on how to handle non-fully active documents (which includes BFCached documents).

saschanaz commented 2 years ago

Is there anything that needs to be updated on the spec side for this?

It currently says nothing about blocking bfcache, so maybe we should add some?

It seems setting salvagable to false does that work, but in this way we can't safely set it true again.

@annevk, is salvagable the right tool to use here?

annevk commented 2 years ago

Yeah, you'd set salvageable to false during the unloading document cleanup steps.

cc @domenic