[wpt-sync] Sync PR 32504 - App history: tweak and test event/promise ordering
Categories
(Testing :: web-platform-tests, task, P4)
Tracking
(firefox99 fixed)
| Tracking | Status | |
|---|---|---|
| firefox99 | --- | fixed |
People
(Reporter: wpt-sync, Unassigned)
References
()
Details
(Whiteboard: [wptsync downstream])
Sync web-platform-tests PR 32504 into mozilla-central (this bug is closed when the sync is complete).
PR: https://github.com/web-platform-tests/wpt/pull/32504
Details from upstream follow.
Domenic Denicola <domenic@chromium.org> wrote:
App history: tweak and test event/promise ordering
By adding new exhaustive tests under ordering/, it was revealed that the ordering between navigatesuccess/navigateerror and the committed/finished promises was not always consistent:
Simply adding a currentchange event handler would cause microtasks to run during commit, which changed some ordering.
Calling transitionWhile() would take us from the zero-promise case to the 1+-promise case in ScriptPromise::All(). As the new comment explains, both the spec and implementation have an observably-different fast path for the 0-promise case which caused changes in ordering.
In the course of fixing this, I found out that the did_finish_before_commit_ code in app_history_api_navigation.{h,cc} was actually not a mitigation for the case it stated, where promises passed to transitionWhile() would settle faster than the browser-process roundtrip for same-document traversals. That is in fact impossible, since we only fire the navigate event after the browser-process roundtrip has completed. Instead, they were a mitigation for (1).
This commit then ensures consistent ordering, tested with new rather-exhaustive tests, in the following manner:
We move the firing of currentchange to before resolving the committed promise. This eliminates (1) and allows us to delete the did_finish_before_commit_ tracking.
We always ensure we pass 1+ promises to ScriptPromise::All(). This eliminates (2).
A consequence of this is that we are now more likely to get rejected finished promises, in cases like
await appHistory.navigate("#1").committed; await appHistory.navigate("#2").committed;Before, the finished promise for the #1 navigation would go through the fast path per (2), and fulfill before the navigation to #2 canceled it. Now that does not happen, so code like the above will give an unhandled promise rejection for #1's finished promise.
To avoid this, we unconditionally mark finished promises as handled. This follows some web platform precedent, e.g. stream closed promises, where the promise is one of several information channels (in this case the developer might also find out via the AbortSignal or the navigateerror event). We do not do this for the committed promise though, as if a commit fails, that's probably something more deeply wrong, and cannot be ignored.
All of these changes will require spec updates.
For the tests, we introduce a new ordering/ directory which contains cross-cutting ordering tests, and we consolidate a few tests into the newly-introduced variant-driven exhaustive ones. A couple of other tests were affected by these changes too or fixed as a drive-by.
Change-Id: I8a50ca28d009e0a8a2c94331cd17f29b0a8dc463
Reviewed-on: https://chromium-review.googlesource.com/3405377
WPT-Export-Revision: 6050a1624dc5d23e6bb2355625b7500068bf3544
|
Assignee | |
Comment 1•4 years ago
|
||
|
|
Assignee | |
Comment 2•4 years ago
|
||
CI Results
Ran 0 Firefox configurations based on mozilla-central, and Firefox, and Chrome on GitHub CI
Total 19 tests and 1 subtests
Status Summary
Firefox
OK : 16
FAIL : 16
ERROR: 3
Chrome
OK : 15
PASS : 11
FAIL : 5
ERROR: 4
Links
Details
New Tests That Don't Pass
/app-history/currentchange-event/currentchange-app-history-back-forward-same-doc.html
AppHistoryCurrentChangeEvent fires for appHistory.back() and appHistory.forward(): FAIL (Chrome: FAIL)
/app-history/navigate-event/transitionWhile-and-navigate.html
Using transitionWhile() then navigate() in the ensuing currentchange should abort the finished promise (but not the committed promise): FAIL (Chrome: FAIL)
/app-history/ordering/back-same-document.html?currentchange
event and promise ordering for same-document appHistory.back(): FAIL (Chrome: FAIL)
/app-history/ordering/back-same-document.html?transitionWhile¤tChange
event and promise ordering for same-document appHistory.back(): FAIL (Chrome: PASS)
/app-history/ordering/back-same-document.html?: ERROR (Chrome: ERROR)
/app-history/ordering/back-same-document.html?transitionWhile
event and promise ordering for same-document appHistory.back(): FAIL (Chrome: PASS)
/app-history/ordering/currentchange-dispose-ordering.html
Ordering between AppHistoryCurrentChangeEvent and AppHistoryEntry dispose events: FAIL (Chrome: FAIL)
/app-history/ordering/navigate-cross-document-event-order.html
navigate() event ordering for cross-document navigation: FAIL (Chrome: PASS)
/app-history/ordering/navigate-same-document-transitionWhile-reject.html?currentchange
event and promise ordering for appHistory.navigate() with a rejected promise passed to transitionWhile(): FAIL (Chrome: PASS)
/app-history/ordering/navigate-same-document-transitionWhile-reject.html?: ERROR (Chrome: ERROR)
/app-history/ordering/navigate-same-document.html?transitionWhile
event and promise ordering for same-document appHistory.navigate(): FAIL (Chrome: PASS)
/app-history/ordering/navigate-same-document.html?currentchange
event and promise ordering for same-document appHistory.navigate(): FAIL (Chrome: FAIL)
/app-history/ordering/navigate-same-document.html?transitionWhile¤tChange
event and promise ordering for same-document appHistory.navigate(): FAIL (Chrome: PASS)
/app-history/ordering/navigate-same-document.html?: ERROR (Chrome: ERROR)
/app-history/ordering/navigateerror-ordering-location-api-reentrant.html
navigateerror ordering when navigating via the location API reentrantly: FAIL (Chrome: PASS)
/app-history/ordering/navigateerror-ordering-location-api.html
navigateerror ordering when navigating via the location API: FAIL (Chrome: PASS)
/app-history/ordering/navigateerror-ordering-navigate-cross-document.html
navigateerror ordering when navigate() is called repeatedly (cross-document): FAIL (Chrome: PASS)
/app-history/ordering/navigateerror-ordering-transitionWhile-reentrant.html
navigateerror ordering when navigate() is called reentrantly and handled by transitionWhile(): FAIL (Chrome: PASS)
/app-history/ordering/navigateerror-ordering-transitionWhile.html
navigateerror ordering when navigate() is called repeatedly and handled by transitionWhile(): FAIL (Chrome: PASS)
|
||
Comment 5•3 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/c122b330a600
https://hg.mozilla.org/mozilla-central/rev/bc7823dfe663
Description
•