Intent to Ship: Back/forward cache NotRestoredReasons API

[Yuzu Saijo‏]

Contact emails

yu...@google.com, yu...@chromium.org, fer...@chromium.org

Explainer

https://github.com/WICG/bfcache-not-restored-reason/blob/main/NotRestoredReason.md

Specification

https://github.com/whatwg/html/pull/9360

Design docs

https://github.com/WICG/bfcache-not-restored-reason/blob/main/NotRestoredReason.md

Summary

NotRestoredReason API will report the list of reasons why a page is not served from BFcache in a frame tree structure, via PerformanceNavigationTiming API.

Blink component

UI>Browser>Navigation>BFCache

TAG review

https://github.com/w3ctag/design-reviews/issues/739

TAG review status

Issues addressed

Risks

Interoperability and Compatibility

Gecko: Defer (https://github.com/mozilla/standards-positions/issues/766) Once issues (standardized reasons & unsalvageable documents), they would switch to positive.

WebKit: No signal (https://github.com/WebKit/standards-positions/issues/154)

Web developers: Positive (https://github.com/w3c/navigation-timing/issues/171#issuecomment-1062672989)

Other signals: Positive from Origin Trial users:

How likely are you to keep using this feature?

92% answered likely, 8% (1 vote) is unsure

Security

We do not report detailed information about cross-origin iframes. See Security and Privacy section in the explainer.

WebView application risks

Does this intent deprecate or change behavior of existing APIs, such that it has potentially high risk for Android WebView-based applications?

No.

Debuggability

In DevTools console, try:

performance.getEntriesByType('navigation')[0].notRestoredReasons;

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.

NotRestoredReasons API is available on all platforms including WebView, but back/forward cache is not enabled on WebView. So on WebView, NotRestoredReasons API should always say that the page is blocked from being restored from bfcache with the reason being something like “not supported”.

(Currently it reports null due to a bug)

Is this feature fully tested by web-platform-tests?

Yes. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/performance-timeline/not-restored-reasons/

DevTrial instructions

https://github.com/rubberyuzu/bfcache-not-retored-reason/blob/main/HowToTest.md

Flag name

blink RunTimeEnabledFeature: BackForwardCacheSendNotRestoredReasons

Requires code in //chrome?

False

Tracking bug

https://bugs.chromium.org/p/chromium/issues/detail?id=1326344

Launch bug

https://launch.corp.google.com/launch/4200848

Estimated milestones

Shipping on desktop	116
OriginTrial desktop last	114
OriginTrial desktop first	109
DevTrial on desktop	108

Shipping on Android	116
OriginTrial Android last	114
OriginTrial Android first	109
DevTrial on Android	108

Shipping on WebView	116
OriginTrial WebView last	114
OriginTrial WebView first	109
DevTrial on WebView	108

Anticipated spec changes

Open questions about a feature may be a source of future web compat or interop issues. Please list open issues.

https://github.com/whatwg/html/pull/9360

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/5684908759449600

Links to previous Intent discussions

Intent to prototype: https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP-nMoGAzjUjzv3WmxcRpUSBgnA-AHQ05kh9gXc%2BQB8pRM6%2BfA%40mail.gmail.com Intent to Experiment: https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP-nMoHe391sAB2PdbEVw9uiSPFxTB_EYsRizcPpZ7-pg16O0A%40mail.gmail.com

Intent to Extend Experiment: https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAA5e698QcKZSthm%3Dz_4pi8cOzi4kfbx-AXveC%2BAKimUh-tMycA%40mail.gmail.com

This intent message was generated by Chrome Platform Status.

[Yoav Weiss‏]

On Thu, Jul 6, 2023 at 7:28 AM 'Yuzu Saijo' via blink-dev <blin...@chromium.org> wrote:

Contact emails

yu...@google.com, yu...@chromium.org, fer...@chromium.org

Explainer

https://github.com/WICG/bfcache-not-restored-reason/blob/main/NotRestoredReason.md

Specification

https://github.com/whatwg/html/pull/9360

Design docs

https://github.com/WICG/bfcache-not-restored-reason/blob/main/NotRestoredReason.md

Summary

NotRestoredReason API will report the list of reasons why a page is not served from BFcache in a frame tree structure, via PerformanceNavigationTiming API.

Blink component

UI>Browser>Navigation>BFCache

TAG review

https://github.com/w3ctag/design-reviews/issues/739

TAG review status

Issues addressed

Risks

Interoperability and Compatibility

Gecko: Defer (https://github.com/mozilla/standards-positions/issues/766) Once issues (standardized reasons & unsalvageable documents), they would switch to positive.

It seems like the "standardized reasons" part is addressed in your PR. Is the same true for the second point?

 

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP-nMoHYpT3sxWV%2BEipL5NcNSWy8fOdDdAroucmNb%3DZvxJWRBA%40mail.gmail.com.

[Domenic Denicola‏]

I have some questions about how well the implementation here matches up with the spec.

First, there doesn't appear to be any NotRestoredReasons interface defined in Chromium? The relevant attribute on PerformanceNavigationTiming returns object?. That seems like a problematic mismatch...

Second, I can't find exactly where the list of script-exposed not restored reasons are. But, I'll note that Chromium seems to have ~50 such reasons, whereas you've only specified 4 (fetch, navigation-failure, parser-aborted, websocket). Can you confirm that you're only shipping the specified four?

Thanks!

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfXtkH6O82W%2BWm9ckCyYasSJt2cbs9VA4VZAmYhtivgj4g%40mail.gmail.com.

[Domenic Denicola‏]

Also, checking the tests, it seems like the currently-implemented reasons don't match the spec. E.g. this test requires the reason to be "WebSocket", but the specification says "websocket" (lowercase). I couldn't find tests for the other three reasons...

[Yuzu Saijo‏]

Sorry for the delayed response.

> there doesn't appear to be any NotRestoredReasons interface defined in Chromium?

Let me address this implementation and delay the shipping until the chromium implementation matches the proposed spec. Thanks for pointing it out!

Same for WPT. I will add tests for all the standardized reasons.

> Can you confirm that you're only shipping the specified four?

We do have ~50 not restored reasons, and in theory we will be able to remove most of them except for the standardized four reasons. 

However, in reality it will take time for us to support all the reasons and we need to keep blocking on them for a while.

In the meantime, our plan was to expose the non-standardized reasons too, but in a way that's distinguishable from standardized reasons as you suggested here.

I realized that we need to add browser specific reasons to the spec as well. Let me add that and send a review request again.

Thanks!

Yuzu

[Yuzu Saijo‏]

+bfcache-dev

I was talking to Fergal today and discussed this, and I am not sure about adding browser-specific reasons to the spec.

For example, some reasons like "speech synthesis API is used" / "unload handler" are completely specific to Chrome, and it doesn't really make sense to add them to the spec, even with the namespace (x-speechsysthesis / x-unloadhandler).

Maybe we can document the reasons somewhere in a shared list but not in the spec?

I think the API would be more useful if it can give as much information as possible, not limited to the specced reasons.

Please let me know what you think!

Yuzu

[Domenic Denicola‏]

I think specifying these reasons is important. As noted in the linked issue, I think the end goal should be:

• Every reason that a browser ever emits, is found in a specification somewhere. (It doesn't have to be the HTML spec, e.g. the speech synthesis reason could live in the speech synthesis spec.)
• If browsers prevent bfcache restoration for a reason not found in a spec, it is always translated to a standardized reason such as "unknown".

This avoids the usual interop problems with vendor-specific extensions to the web platform, such as: no clear specification for what strings to use; no clear point at which the reason is added to the document's reasons list; etc. Although you claim these reasons are idiosyncratic to Chrome, that won't necessarily be the case; e.g. Firefox has unload handler as a reason, and I suspect most user agents have memory limitations or similar.

We could have a discussion about allowing vendor-specific information in the API in addition to the standardized reasons. For example, we could have one of the standardized reasons be "user-agent-specific", and then add an additional field userAgentSpecificInfo. But I would like to see significantly more discussion with other vendors before going that route.

[Fergal Daly‏]

On Wed, 9 Aug 2023 at 12:01, Domenic Denicola <dom...@chromium.org> wrote:

I think specifying these reasons is important. As noted in the linked issue, I think the end goal should be:

• Every reason that a browser ever emits, is found in a specification somewhere. (It doesn't have to be the HTML spec, e.g. the speech synthesis reason could live in the speech synthesis spec.)

There's no intrinsic reason for speech synthesis to block BFCache. It just happens that Chrome blocked it. There's no spec reason for unload to block BFCache, in fact the spec says that it doesn't.

I think it's good for us to have agreed names, e.g. "unload-event-handler". Should we put into various specs "if an implementer chooses to block BFCache because X has been used, they should use the reason `Y`"?

• If browsers prevent bfcache restoration for a reason not found in a spec, it is always translated to a standardized reason such as "unknown".

This avoids the usual interop problems with vendor-specific extensions to the web platform, such as: no clear specification for what strings to use; no clear point at which the reason is added to the document's reasons list; etc. Although you claim these reasons are idiosyncratic to Chrome, that won't necessarily be the case; e.g. Firefox has unload handler as a reason, and I suspect most user agents have memory limitations or similar.

Chrome has over 100 reasons. I'd say at least 50 of them are actionable such that you wouldn't want to lump them into an opaque "unknown" category.

I do not relish the idea of updating 50 places in spec to insert a name to be used if you decide to block.

How about maintaining a central list of reasons with low friction to add new reasons even if they are browser-specific? The cases where you must block should still be inline in spec (and also on the list),

F

 

You received this message because you are subscribed to the Google Groups "bfcache-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bfcache-dev...@chromium.org.
To view this discussion on the web, visit https://groups.google.com/a/chromium.org/d/msgid/bfcache-dev/CAM0wra-P3NxELP28%3Dgh%3D3ROC35m8ijS_5RRcStyjFew1AXNyEg%40mail.gmail.com.

[Domenic Denicola‏]

On Wed, Aug 9, 2023 at 6:44 PM Fergal Daly <fer...@google.com> wrote:

On Wed, 9 Aug 2023 at 12:01, Domenic Denicola <dom...@chromium.org> wrote:

I think specifying these reasons is important. As noted in the linked issue, I think the end goal should be:

• Every reason that a browser ever emits, is found in a specification somewhere. (It doesn't have to be the HTML spec, e.g. the speech synthesis reason could live in the speech synthesis spec.)

There's no intrinsic reason for speech synthesis to block BFCache. It just happens that Chrome blocked it. There's no spec reason for unload to block BFCache, in fact the spec says that it doesn't.

I think it's good for us to have agreed names, e.g. "unload-event-handler". Should we put into various specs "if an implementer chooses to block BFCache because X has been used, they should use the reason `Y`"?

• If browsers prevent bfcache restoration for a reason not found in a spec, it is always translated to a standardized reason such as "unknown".

This avoids the usual interop problems with vendor-specific extensions to the web platform, such as: no clear specification for what strings to use; no clear point at which the reason is added to the document's reasons list; etc. Although you claim these reasons are idiosyncratic to Chrome, that won't necessarily be the case; e.g. Firefox has unload handler as a reason, and I suspect most user agents have memory limitations or similar.

Chrome has over 100 reasons. I'd say at least 50 of them are actionable such that you wouldn't want to lump them into an opaque "unknown" category.

I do not relish the idea of updating 50 places in spec to insert a name to be used if you decide to block.

How about maintaining a central list of reasons with low friction to add new reasons even if they are browser-specific? The cases where you must block should still be inline in spec (and also on the list),

That sounds great to me. We should probably make this separation clear in the spec, e.g. the "must" list will have cross-references you can follow, whereas the "may" list ends up only being cross-referenced from some generic location like https://html.spec.whatwg.org/multipage/browsing-the-web.html#note-bfcache:~:text=User%20agents%20may,keeping%20it%20cached. .

[Yuzu Saijo‏]

Sounds good, I will create a list on the explainer for the "may block" reasons then.

Re: exposing NotRestoredReasons interface instead of object in idl:

I'm working on the implementation in this CL.

This might be a basic question, but is there any difference on how to call the API from users' perspective, when the exposed attribute is an interface vs object?

[Chris Harrelson‏]

Please also make sure to complete all of the other shipping gate reviews.

Thanks!

Chris

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/43e32f0e-454e-4525-b317-cbe492e2f23bn%40chromium.org.

[Yuzu Saijo‏]

This is now ready to ship, now that we have all the approvals on the ChromeStatus and the spec draft is close to agreement.

Can you please take a look at this again?

Thanks!

[Domenic Denicola‏]

I am happy with the spec progress here and don't think it's a significant blocker for the Intent at this point.

On the tests and implementation:

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.
• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?
• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?
• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

[Fergal Daly‏]

On Tue, 6 Feb 2024 at 15:13, Domenic Denicola <dom...@chromium.org> wrote:

I am happy with the spec progress here and don't think it's a significant blocker for the Intent at this point.

On the tests and implementation:

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.
• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?
• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?
• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

I don't know specifically what is there right now but I would expect that we will ship others. E.g. BroadcastChannel blocks BFCache on Chrome and Mozilla but not WebKit and there is currently disagreement. Why would it be better to show "masked" for that case?

F

[Domenic Denicola‏]

On Tue, Feb 6, 2024 at 6:40 PM Fergal Daly <fer...@google.com> wrote:

On Tue, 6 Feb 2024 at 15:13, Domenic Denicola <dom...@chromium.org> wrote:

I am happy with the spec progress here and don't think it's a significant blocker for the Intent at this point.

On the tests and implementation:

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.
• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?
• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?
• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

I don't know specifically what is there right now but I would expect that we will ship others. E.g. BroadcastChannel blocks BFCache on Chrome and Mozilla but not WebKit and there is currently disagreement. Why would it be better to show "masked" for that case?

The idea is to follow the standards and not ship nonstandard behavior. The current spec PR actually only allows sending "masked" in the cross-origin case, and doesn't allow sending it for BroadcastChannel. If the intention is to send some value in the BroadcastChannel case (which is this part of the spec) then that needs to be specified in the spec PR before shipping such a value in Chromium.

[Fergal Daly‏]

On Wed, 7 Feb 2024 at 12:26, Domenic Denicola <dom...@chromium.org> wrote:

On Tue, Feb 6, 2024 at 6:40 PM Fergal Daly <fer...@google.com> wrote:

On Tue, 6 Feb 2024 at 15:13, Domenic Denicola <dom...@chromium.org> wrote:

I am happy with the spec progress here and don't think it's a significant blocker for the Intent at this point.

On the tests and implementation:

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.
• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?
• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?
• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

I don't know specifically what is there right now but I would expect that we will ship others. E.g. BroadcastChannel blocks BFCache on Chrome and Mozilla but not WebKit and there is currently disagreement. Why would it be better to show "masked" for that case?

The idea is to follow the standards and not ship nonstandard behavior. The current spec PR actually only allows sending "masked" in the cross-origin case, and doesn't allow sending it for BroadcastChannel. If the intention is to send some value in the BroadcastChannel case (which is this part of the spec) then that needs to be specified in the spec PR before shipping such a value in Chromium.

BFCaching is never required by spec. That means any browser can block BFCache at any time, for any reason and still be in spec.

I think what's missing is that said we would maintain a registry of reasons that were not in the spec so that when we block for unspecced reasons, we don't proliferate a bunch of undocumented names.

I'm not sure how to express that in the spec,

F

[Domenic Denicola‏]

On Wed, Feb 7, 2024 at 12:51 PM Fergal Daly <fer...@google.com> wrote:

On Wed, 7 Feb 2024 at 12:26, Domenic Denicola <dom...@chromium.org> wrote:

On Tue, Feb 6, 2024 at 6:40 PM Fergal Daly <fer...@google.com> wrote:

On Tue, 6 Feb 2024 at 15:13, Domenic Denicola <dom...@chromium.org> wrote:

I am happy with the spec progress here and don't think it's a significant blocker for the Intent at this point.

On the tests and implementation:

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.
• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?
• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?
• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

I don't know specifically what is there right now but I would expect that we will ship others. E.g. BroadcastChannel blocks BFCache on Chrome and Mozilla but not WebKit and there is currently disagreement. Why would it be better to show "masked" for that case?

The idea is to follow the standards and not ship nonstandard behavior. The current spec PR actually only allows sending "masked" in the cross-origin case, and doesn't allow sending it for BroadcastChannel. If the intention is to send some value in the BroadcastChannel case (which is this part of the spec) then that needs to be specified in the spec PR before shipping such a value in Chromium.

BFCaching is never required by spec. That means any browser can block BFCache at any time, for any reason and still be in spec.

Yes. But a browser cannot create values for the NotRestoredReasonDetails's reason property which are not in the spec, while staying spec-compliant. This is similar to how we cannot have, e.g., DOMException's name property returning arbitrary values; we instead document them all in the spec, and then document that some of them may be thrown in implementation-specific circumstances (example).

 

I think what's missing is that said we would maintain a registry of reasons that were not in the spec so that when we block for unspecced reasons, we don't proliferate a bunch of undocumented names.

I'm not sure how to express that in the spec,

We discussed how to do so upthread: https://groups.google.com/a/chromium.org/g/bfcache-dev/c/ufQx6r6su6U/m/vyQM9nGHAwAJ 

[Yuzu Saijo‏]

Thanks Domenic for bringing up the concerns!

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.

> Updated all the strings to match the spec-defined strings.

• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?

> Now the failing tests and the expected files are down to three.

1) parser-aborted

We currently block with different reason("loading"), as we haven't worked on differentiating loading vs parser getting aborted.

2) navigation-failure

We do report "navigation-failure" when the document errors(implementation), but somehow the test only reports "http-status-not-ok" which is the chrome internal reason.

I will look into this more.

3) weblock

Chrome currently blocks with another reason here (masked), so this failure will not go away. Maybe I should make WPTs to test if the expected reason exists in the list, instead of checking the complete list.

• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?

> Added this to the spec, thanks!

• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

> We plan to add user-agent specific reasons to the spec in the may-block section.

This is the draft PR (have't added the explanation for each reason yet).

Is it okay to ship while we work on the follow-up PR?

[Domenic Denicola‏]

On Thu, Feb 22, 2024 at 4:20 PM Yuzu Saijo <yu...@google.com> wrote:

Thanks Domenic for bringing up the concerns!

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.

> Updated all the strings to match the spec-defined strings.

• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?

> Now the failing tests and the expected files are down to three.

1) parser-aborted

We currently block with different reason("loading"), as we haven't worked on differentiating loading vs parser getting aborted.

Note that "loading" is a nonstandard reason, so it would be bad to ship in that state. It should either be the correct reason ("parser-aborted") or the generi "masked" reason.

 

2) navigation-failure

We do report "navigation-failure" when the document errors(implementation), but somehow the test only reports "http-status-not-ok" which is the chrome internal reason.

Similar to the above.

 

I will look into this more.

3) weblock

Chrome currently blocks with another reason here (masked), so this failure will not go away. Maybe I should make WPTs to test if the expected reason exists in the list, instead of checking the complete list.

Allowing an implementation to always do "masked" and pass the tests seems reasonable to me.

 

• I found a nonstandard toJSON() in NotRestoredReasonDetails in Chromium. Was the intent to specify that?

> Added this to the spec, thanks!

• Can you confirm that Chromium does not plan to ship any nonstandard not restored reason strings, beyond the specified "fetch", "navigation-failure", "parser-aborted", "websocket", "lock", and "masked"?

> We plan to add user-agent specific reasons to the spec in the may-block section.

This is the draft PR (have't added the explanation for each reason yet).

Is it okay to ship while we work on the follow-up PR?

You could ship the portion that is fully specified, but the portions in the draft PR would not be approved for shipment until they reach the usual bars (e.g., a fully reviewed spec, web platform tests, other-vendor positions, etc.).

[Fergal Daly‏]

On Thu, 22 Feb 2024 at 16:32, Domenic Denicola <dom...@chromium.org> wrote:

On Thu, Feb 22, 2024 at 4:20 PM Yuzu Saijo <yu...@google.com> wrote:

Thanks Domenic for bringing up the concerns!

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.

> Updated all the strings to match the spec-defined strings.

• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?

> Now the failing tests and the expected files are down to three.

1) parser-aborted

We currently block with different reason("loading"), as we haven't worked on differentiating loading vs parser getting aborted.

Note that "loading" is a nonstandard reason, so it would be bad to ship in that state. It should either be the correct reason ("parser-aborted") or the generi "masked" reason.

"parser-aborted" is a reason that Chrome doesn't currently emit (it doesn't exist in the code). I'm not sure how we ended up speccing a reason that doesn't exist but I don't think we punt the entire NRR feature for another milestone for that.

 

 

2) navigation-failure

We do report "navigation-failure" when the document errors(implementation), but somehow the test only reports "http-status-not-ok" which is the chrome internal reason.

Similar to the above.

I think this one we can just make change http-status-no-ok to navigation-failure.

 

 

I will look into this more.

3) weblock

Chrome currently blocks with another reason here (masked), so this failure will not go away. Maybe I should make WPTs to test if the expected reason exists in the list, instead of checking the complete list.

Allowing an implementation to always do "masked" and pass the tests seems reasonable to me.

I think this is a general issue with the testing. It should always be OK for a UA to have extra reasons, e.g. when we do add parser-aborted, loading will continue to show up. I guess we could add a hack to suppress loading if parser-aborted is present but really what we care about in these tests is that the specced reason is present in the specced case,

F

[Domenic Denicola‏]

On Thu, Feb 22, 2024 at 6:01 PM Fergal Daly <fer...@google.com> wrote:

On Thu, 22 Feb 2024 at 16:32, Domenic Denicola <dom...@chromium.org> wrote:

On Thu, Feb 22, 2024 at 4:20 PM Yuzu Saijo <yu...@google.com> wrote:

Thanks Domenic for bringing up the concerns!

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.

> Updated all the strings to match the spec-defined strings.

• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?

> Now the failing tests and the expected files are down to three.

1) parser-aborted

We currently block with different reason("loading"), as we haven't worked on differentiating loading vs parser getting aborted.

Note that "loading" is a nonstandard reason, so it would be bad to ship in that state. It should either be the correct reason ("parser-aborted") or the generi "masked" reason.

"parser-aborted" is a reason that Chrome doesn't currently emit (it doesn't exist in the code). I'm not sure how we ended up speccing a reason that doesn't exist but I don't think we punt the entire NRR feature for another milestone for that.

 

 

2) navigation-failure

We do report "navigation-failure" when the document errors(implementation), but somehow the test only reports "http-status-not-ok" which is the chrome internal reason.

Similar to the above.

I think this one we can just make change http-status-no-ok to navigation-failure.

 

 

I will look into this more.

3) weblock

Chrome currently blocks with another reason here (masked), so this failure will not go away. Maybe I should make WPTs to test if the expected reason exists in the list, instead of checking the complete list.

Allowing an implementation to always do "masked" and pass the tests seems reasonable to me.

I think this is a general issue with the testing. It should always be OK for a UA to have extra reasons, e.g. when we do add parser-aborted, loading will continue to show up. I guess we could add a hack to suppress loading if parser-aborted is present but really what we care about in these tests is that the specced reason is present in the specced case,

As we've discussed elsewhere on this thread, extra reasons are not OK; the APIs we ship need to be specified. "masked" is always OK though, per the spec.

[Fergal Daly‏]

On Thu, 22 Feb 2024 at 18:03, Domenic Denicola <dom...@chromium.org> wrote:

On Thu, Feb 22, 2024 at 6:01 PM Fergal Daly <fer...@google.com> wrote:

On Thu, 22 Feb 2024 at 16:32, Domenic Denicola <dom...@chromium.org> wrote:

On Thu, Feb 22, 2024 at 4:20 PM Yuzu Saijo <yu...@google.com> wrote:

Thanks Domenic for bringing up the concerns!

• I found performance-navigation-timing-navigation-failure.tentative.window.js which seems like it needs to be updated from "error-document" to "navigation-failure". That's worth looking into in case it means the implementation is also not yet updated.

> Updated all the strings to match the spec-defined strings.

• I also found that the Chromium test directory is full of -expected.txt files, which seem to match up with the failures on wpt.fyi. Will those be addressed before shipping?

> Now the failing tests and the expected files are down to three.

1) parser-aborted

We currently block with different reason("loading"), as we haven't worked on differentiating loading vs parser getting aborted.

Note that "loading" is a nonstandard reason, so it would be bad to ship in that state. It should either be the correct reason ("parser-aborted") or the generi "masked" reason.

"parser-aborted" is a reason that Chrome doesn't currently emit (it doesn't exist in the code). I'm not sure how we ended up speccing a reason that doesn't exist but I don't think we punt the entire NRR feature for another milestone for that.

 

 

2) navigation-failure

We do report "navigation-failure" when the document errors(implementation), but somehow the test only reports "http-status-not-ok" which is the chrome internal reason.

Similar to the above.

I think this one we can just make change http-status-no-ok to navigation-failure.

 

 

I will look into this more.

3) weblock

Chrome currently blocks with another reason here (masked), so this failure will not go away. Maybe I should make WPTs to test if the expected reason exists in the list, instead of checking the complete list.

Allowing an implementation to always do "masked" and pass the tests seems reasonable to me.

I think this is a general issue with the testing. It should always be OK for a UA to have extra reasons, e.g. when we do add parser-aborted, loading will continue to show up. I guess we could add a hack to suppress loading if parser-aborted is present but really what we care about in these tests is that the specced reason is present in the specced case,

As we've discussed elsewhere on this thread, extra reasons are not OK; the APIs we ship need to be specified. "masked" is always OK though, per the spec.

Any restore can be blocked for multiple reasons. That can be for a mix of common and UA-specific reasons (all of which appear in the spec). As long as the common reason appears in the test for the common reason, the test should pass. Otherwise are writing tests that assert that UA-specific reasons are absent in some cases. I don't think we should be doing that. UAs can block on UA-specific reasons whenever they want and be within spec,

F 

[Yuzu Saijo‏]

Now that M123 implementation is aligned with the spec, can we ship the feature to M123?

We're currently working on adding browser-specific blocking reasons (i.e. reasons that may block) to the spec in this PR, and while this is still under review and the details of the names can change, the overall direction will stay the same. Firefox folks are also engaged with speccing the names on the PR.

Please let me know what you think, thanks!

Yuzu

[Daniel Bratell‏]

LGTM1

/Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/2e913c96-1ebc-415c-9def-81695f5e74d4n%40chromium.org.

[Mike Taylor‏]

LGTM2

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/67ea5933-8efb-43d8-abe1-6638415411da%40gmail.com.

[Yoav Weiss (@Shopify)‏]

LGTM3

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/2e913c96-1ebc-415c-9def-81695f5e74d4n%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

[Yoav Weiss (@Shopify)‏]

It seems like there's a lot of developer confusion and documentation issues surfaced at https://github.com/WICG/bfcache-not-restored-reason/issues/2

Reading through that thread, I think it can be summed up as:

*  `preventedBackForwardCache` going away (compared to the previously-OTed API shape), making it hard for developers to know if BFCache was prevented from being used. Can you help me understand what developers should do here?

* Name-changes that are related to discussions on https://github.com/whatwg/html/pull/10154. Here better documentation can probably fix this.

  - One thing I didn't fully internalize (but should have) is that ongoing PR discussion can result in name changes after we ship. How likely do you consider that scenario?

* "masked" being used both for reasons that web developers can't do much about, as well as for cross-origin iframes, where we can't expose the reasons due to privacy and security considerations. Can developers distinguish those cases somehow? If so, how?

I'd like to ask y'all to hold off on shipping this until the above is clarified. I'd also like to apologize as I should've caught these points before LGTMing.

LGTM3

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/2e913c96-1ebc-415c-9def-81695f5e74d4n%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/67ea5933-8efb-43d8-abe1-6638415411da%40gmail.com.

[Barry Pollard‏]

I've been spending a bit of time on this over the last few days following that developers complaint as it seems it has already started rolling out to Chrome 123 users?

I think it's very close to shipping, but think we probably should have held off rolling it out to 123 until the following were addressed:

• The API has changed shape and we did not make the changes clear to developers who participated in the origin trial, nor did we update our own documentation.
• The "may block" reasons (and text) are not finalized yet. While these may seem less important than the "must block" reasons, they are some of the most common reasons of bfcache blocking (unload handlers, cache-control no-store...etc.). It is not ideal to change the text of these after shipping (e.g. "unload-listener" in the spec is current reporting as "unload-handler" in Chrome 123 as that spec change was not included yet).
• There are still some questions (see Yoav's one, and I've raised some myself).
• We do not seem to have rolled this out to the other channels (I cannot see it working in Beta and Canary for example), and I think that should be done first? Especially if we intended to complete the rollout in 123, then 124 and up should have this enabled now but do not.

On the documentation side, we have the updates ready now and should be pushing this out today or tomorrow. Perhaps with that, most of the confusion will have been addressed and can continue to roll this out, despite starting that a little earlier than ideal?

We also have work planned to add these to MDN, but I don't think that is necessary for it shipping given we have our own docs, and plan to update it shortly.

[Yuzu Saijo‏]

Thanks all for the replies.

I agree that I should have contacted Barry and his team to update the documentation first before starting the rollout process, sorry for the confusion.

Let me address Yoav's concerns/questions inline.

> `preventedBackForwardCache` going away (compared to the previously-OTed API shape), making it hard for developers to know if BFCache was prevented from being used. Can you help me understand what developers should do here?

The reason why `preventedBackForwardCache` was removed is that this bit of information can be inferred from `reasons`.

`preventedBackForwardCache` used to indicate whether or not the specific document has any blockers. Now you can check `reasons` instead. If `reasons` is empty, that document did not block bfcache, and if it contains any value, it blocked bfcache.

The only exception here is the “masked” case. The outermost main document will have “masked” in the `reasons` when cross-origin children are blocking. 

Regarding “whether or not bfcache was used”, i.e. whether the entire page had any blockers, you can check the NotRestoredReasons value. NotRestoredReasons will return null if the page is successfully restored from bfcache, and it will be non-null if not.

As a developer, you can write something like this.

if (the most recent navigation was history navigation) {

  if (NotRestoredReasons == null) {

    // The page was restored from bfcache.

  } else {

   // The page was not restored from bfcache.

  }

}

> Name-changes that are related to discussions on https://github.com/whatwg/html/pull/10154. Here better documentation can probably fix this.

  - One thing I didn't fully internalize (but should have) is that ongoing PR discussion can result in name changes after we ship. How likely do you consider that scenario?

Yes, the names are going to change but unlike e.g. function names, there is no backward-compatibility risk with the names changing - no site's code depends on the names, they may be logged for debugging purposes. Names will be added/deleted as time goes on.

Changing the names a couple of milestones after launch is not ideal but there is very little benefit to anyone from holding back the API until the names are finalized.

>  "masked" being used both for reasons that web developers can't do much about, as well as for cross-origin iframes, where we can't expose the reasons due to privacy and security considerations. Can developers distinguish those cases somehow? If so, how?

This reports the same value by design, in order to not expose cross-origin information at all, i.e. hide whether or not cross-origin iframes had any blockers. This is because the mere fact that cross-origin iframes could be privacy sensitive.

We discussed this point with Mozilla folks and they also preferred not to distinguish UA’s internal reasons vs cross-origin blockers. 

This is unfortunate because as you say, cross-origin information can still be actionable for developers. We are proposing a workaround: Opt-in header for exposing cross-origin iframes info

(We proposed selecting one random cross-origin iframe and exposing its blocking status, but that was turned down by Mozilla folks.)

[Mike Taylor‏]

On 3/25/24 12:45 PM, Yuzu Saijo wrote:

Thanks all for the replies.

I agree that I should have contacted Barry and his team to update the documentation first before starting the rollout process, sorry for the confusion.

Let me address Yoav's concerns/questions inline.

> `preventedBackForwardCache` going away (compared to the previously-OTed API shape), making it hard for developers to know if BFCache was prevented from being used. Can you help me understand what developers should do here?

The reason why `preventedBackForwardCache` was removed is that this bit of information can be inferred from `reasons`.

`preventedBackForwardCache` used to indicate whether or not the specific document has any blockers. Now you can check `reasons` instead. If `reasons` is empty, that document did not block bfcache, and if it contains any value, it blocked bfcache.

The only exception here is the “masked” case. The outermost main document will have “masked” in the `reasons` when cross-origin children are blocking. 

Regarding “whether or not bfcache was used”, i.e. whether the entire page had any blockers, you can check the NotRestoredReasons value. NotRestoredReasons will return null if the page is successfully restored from bfcache, and it will be non-null if not.

As a developer, you can write something like this.

if (the most recent navigation was history navigation) {

  if (NotRestoredReasons == null) {

    // The page was restored from bfcache.

  } else {

   // The page was not restored from bfcache.

  }

}

> Name-changes that are related to discussions on https://github.com/whatwg/html/pull/10154. Here better documentation can probably fix this.

  - One thing I didn't fully internalize (but should have) is that ongoing PR discussion can result in name changes after we ship. How likely do you consider that scenario?

Yes, the names are going to change but unlike e.g. function names, there is no backward-compatibility risk with the names changing - no site's code depends on the names, they may be logged for debugging purposes. Names will be added/deleted as time goes on.

FWIW, if something is exposed to script, there is always a backwards-compat risk to be considered, even if the intended usage is purely for debugging.

https://miketaylr.com/posts/2018/10/(native)-error-prototype-message.html is one example of this

[Fergal Daly‏]

On Tue, 26 Mar 2024 at 02:11, Mike Taylor <mike...@chromium.org> wrote:

On 3/25/24 12:45 PM, Yuzu Saijo wrote:

Thanks all for the replies.

I agree that I should have contacted Barry and his team to update the documentation first before starting the rollout process, sorry for the confusion.

Let me address Yoav's concerns/questions inline.

> `preventedBackForwardCache` going away (compared to the previously-OTed API shape), making it hard for developers to know if BFCache was prevented from being used. Can you help me understand what developers should do here?

The reason why `preventedBackForwardCache` was removed is that this bit of information can be inferred from `reasons`.

`preventedBackForwardCache` used to indicate whether or not the specific document has any blockers. Now you can check `reasons` instead. If `reasons` is empty, that document did not block bfcache, and if it contains any value, it blocked bfcache.

The only exception here is the “masked” case. The outermost main document will have “masked” in the `reasons` when cross-origin children are blocking. 

Regarding “whether or not bfcache was used”, i.e. whether the entire page had any blockers, you can check the NotRestoredReasons value. NotRestoredReasons will return null if the page is successfully restored from bfcache, and it will be non-null if not.

As a developer, you can write something like this.

if (the most recent navigation was history navigation) {

  if (NotRestoredReasons == null) {

    // The page was restored from bfcache.

  } else {

   // The page was not restored from bfcache.

  }

}

> Name-changes that are related to discussions on https://github.com/whatwg/html/pull/10154. Here better documentation can probably fix this.

  - One thing I didn't fully internalize (but should have) is that ongoing PR discussion can result in name changes after we ship. How likely do you consider that scenario?

Yes, the names are going to change but unlike e.g. function names, there is no backward-compatibility risk with the names changing - no site's code depends on the names, they may be logged for debugging purposes. Names will be added/deleted as time goes on.

FWIW, if something is exposed to script, there is always a backwards-compat risk to be considered, even if the intended usage is purely for debugging.

https://miketaylr.com/posts/2018/10/(native)-error-prototype-message.html is one example of this

I think there are some big differences between this and the example.

1 In this case, it is intended that reasons will be added AND removed as time goes on. Code has to be robust to seeing reasons it never saw before. Also there's no guarantee that any specific reason will ever appear, so code written expecting to find a specific reason is already broken.

2 I can understand how someone might rely on an exception's message (despite that being a bad idea), it carries information that is relevant to the current state of the world, it can be used to make decisions in the code at that time and I expect it was the *only* source of that information. Also it was stable and reproducible,. This API is not like that at all. These names carry information about something in the past that no longer exists. They're not actionable at runtime and they're often not reproducible. I am unable to think of any case where you would write code that was conditional on the presence or absence of one of these names, 

If it helps, you can think of these as adds and deletes, not renames.

It's the cost of pushing this API back by 1 or 2 milestones against the danger that I don't really believe in.

That said, there is one reason that is somewhat special - "session-restored" - it indicates that this isn't really a history navigation at all, it's a browser restart. If it occurs, the navigation should not be counted in the denominator for cache hit rate. It's name has not yet been agreed on, so I think that's a legit blocker. If that name doesn't change, I would argue we should just turn on the API knowing that some others will change,

F

[Yoav Weiss (@Shopify)‏]

Thanks Yuzu and Fergal!

My LGTM still stands.

The way that such reliance can manifest itself is in developer-visible dashboards being broken. E.g. RUM providers can collect (the common) "unload-handler" and highlight it, and throw "unload-listener" to the unknown bucket.

That breakage won't be visible to users, but it could diminish the benefits of the feature and may result in developer frustration. It might be worthwhile to be upfront about that in the feature's documentation.

Changing the names a couple of milestones after launch is not ideal but there is very little benefit to anyone from holding back the API until the names are finalized.

Talking to the developers that raised the confusion issues, they agree with you. 

>  "masked" being used both for reasons that web developers can't do much about, as well as for cross-origin iframes, where we can't expose the reasons due to privacy and security considerations. Can developers distinguish those cases somehow? If so, how?

This reports the same value by design, in order to not expose cross-origin information at all, i.e. hide whether or not cross-origin iframes had any blockers. This is because the mere fact that cross-origin iframes could be privacy sensitive.

We discussed this point with Mozilla folks and they also preferred not to distinguish UA’s internal reasons vs cross-origin blockers. 

This is unfortunate because as you say, cross-origin information can still be actionable for developers. We are proposing a workaround: Opt-in header for exposing cross-origin iframes info

(We proposed selecting one random cross-origin iframe and exposing its blocking status, but that was turned down by Mozilla folks.)

I see. An opt-in from the cross-origin iframe sounds like a reasonable approach.

[Barry Pollard‏]

Thanks all.

The documentation has been updated: https://developer.chrome.com/docs/web-platform/bfcache-notrestoredreasons

This includes a section noting differences from the origin trial (this may be removed in future once the origin trial becomes a distant memory).

The way that such reliance can manifest itself is in developer-visible dashboards being broken. E.g. RUM providers can collect (the common) "unload-handler" and highlight it, and throw "unload-listener" to the unknown bucket.
That breakage won't be visible to users, but it could diminish the benefits of the feature and may result in developer frustration. It might be worthwhile to be upfront about that in the feature's documentation.

I have added a note about avoiding depending on specific text wording.

I think the bigger risk is for a common reason like unload handlers to be split into two reasons in the short term thus making it look less common than it really is. It's a short term pain that likely won't affect most users of this API in a few releases, but not ideal to have to change this so soon after launch (especially as this example is a very common reason that we do want users to take action upon).

Looking forward to this API proving itself useful.

[Fergal Daly‏]

We have just discovered that this API seems to already be enabled in stable. The config here sets `copied_from_base_feature_if: "overridden"` so this should still be an "experimental" feature by my expecations BUT as it turns out that soon as we created a finch config to start rolling this out to dev/canary, finch also started sending a Default group to stable (and beta). This is counted as an override by the runtime-features code and so it became enabled. I think we should have a post-mortem for this, since accidentally enabling an API in stable when the intention was dev/canary could be a pretty serious problem. I'd like to separate that from this thread.

So the reality is that this API has been enabled in stable for several months now and at least one RUM provider has built on it. We can

- turn it off again until the names are finalized

- keep it on and try to make the update as painless as possible

- something else ???

F

[Barry Pollard‏]

While this is unfortunate, my strong, strong preference is to keep it on.

This API has been promised (and then delayed) for a long time and now we've communicated that it's released, to pull it again would cause even more confusion.

The API adds one extra property to the PerformanceNavigationTiming interface, which is a very specific API with many, many properties that we have continually added to over the years without issues. It is not something that a developer will encounter unless they specifically go looking for it. The only concern we had before we started informing people it was available in M123 was when it was NOT available. So the fact it has been released for some time without us realising has not caused issues.

Now it's true that those who did go looking for it in M123 were initially confused as what shipped was different to what was origin trialed and docs were not updated, but that has now been resolved. We're aware of two RUM providers who have built upon it in anticipation of it's availability in M123, both of whom have been informed of the changes now and one of whom has already adapted to the new format. I've also had a number of other developers looking into this now it's available. Again, to reiterate, to remove this now would cause much more confusion.

[Yoav Weiss (@Shopify)‏]

I agree. Turning it off now (after LGTMs) won't fix the fact it was shipped prior to them, and would harm developers that are expecting this.

While this definitely needs a postmortem (to ensure the right guardrails are in place to avoid such cases in the future), that's orthogonal.

[Yuzu Saijo‏]

There was a bug in RuntimeEnabledFeatures ( crbug.com/332151808 ), and because of this, the Default group in Finch had the feature enabled.

We were enabling the feature for 98% Stable users, even though we were supposed to be on 50% Canary/Dev/Beta.

This has been enabled since 15th Feb and we only realized this a week ago, giving developers a plenty of time to start using the API, without good documentation.

I wrote a Google-internal post-mortem here: link

(It turned out that the bug was affecting other features as well, launching them accidentally)

I will enable the feature everywhere 100% to avoid further confusion.

Thanks all for the input!

Yuzu

[Nic Jansma‏]

Thank you Barry for the updated documentation on developer.chrome.com, including the description of changes from the prior OT.

We are one of the RUM providers that were affected by differences from the OT, and are updating our library to adjust to the changes.  Once our customers upgrade our library version, they will start seeing the correct data.