New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Gecko Bug 1563587] Make history.back/forward/go asynchronous #18401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Aug 13, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already reviewed downstream.
The main part of the change is the change to ChildSHistory - make it possible to have Go() to be called asynchronously and also let one to cancel pending history navigations. History object (window.history) can then use either the sync or async Go(), depending on the dom.window.history.async pref. LoadDelegate, which is used by GeckoView, needs special handling, since it spins event loop nestedly. With session history loads and same-document loads we can just bypass it. To deal with same-document case, MaybeHandleSameDocumentNavigation is split to IsSameDocumentNavigation, which collects relevant information about the request and returns true if same-document navigation should happen, and then later HandleSameDocumentNavigation uses that information to trigger the navigation. SameDocumentNavigationState is used to pass the information around. referrer-policy-test-case.sub.js is buggy causing tests to pass only on Firefox with sync history API. nested-context-navigations-iframe.html.ini is added because of https://bugzilla.mozilla.org/show_bug.cgi?id=1572932 Differential Revision: https://phabricator.services.mozilla.com/D41199 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1563587 gecko-commit: 214fac6eb1c05177c7ef8eb29b87cceb09eb4110 gecko-integration-branch: autoland gecko-reviewers: farre
moz-wptsync-bot
force-pushed
the
gecko/1563587
branch
from
August 14, 2019 07:59
2c3ae1b
to
167b5f3
Compare
Failures here seem to be timeouts, so merging. |
Ms2ger
reviewed
Aug 16, 2019
next(); | ||
dump("next \n"); | ||
} else { | ||
dump("no next \n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crap, how did I leave that there. Will fix. |
removed dump in gecko tree |
chromium-wpt-export-bot
pushed a commit
that referenced
this pull request
Aug 20, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts #18401 8b46cbf and waits for popstate, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed
chromium-wpt-export-bot
pushed a commit
that referenced
this pull request
Aug 20, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in #18401 8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed
chromium-wpt-export-bot
pushed a commit
that referenced
this pull request
Aug 26, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in #18401 8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Aug 26, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#690280}
chromium-wpt-export-bot
pushed a commit
that referenced
this pull request
Aug 26, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in #18401 8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#690280}
foolip
pushed a commit
that referenced
this pull request
Aug 26, 2019
Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in #18401 8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#690280}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Aug 29, 2019
…istory.back(), a=testonly Automatic update from web-platform-tests [WPT/referrer-policy] Fix races around history.back() Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#690280} -- wpt-commits: edd8fe11e357990cf3dafff83504c4f4a605028c wpt-pr: 18055
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
Aug 30, 2019
…istory.back(), a=testonly Automatic update from web-platform-tests [WPT/referrer-policy] Fix races around history.back() Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#690280} -- wpt-commits: edd8fe11e357990cf3dafff83504c4f4a605028c wpt-pr: 18055
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 4, 2019
…istory.back(), a=testonly Automatic update from web-platform-tests [WPT/referrer-policy] Fix races around history.back() Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshigechromium.org> Reviewed-by: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/master{#690280} -- wpt-commits: edd8fe11e357990cf3dafff83504c4f4a605028c wpt-pr: 18055 UltraBlame original commit: 0f7f604a2c761df9d21daba23b9a131790c28702
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 4, 2019
…istory.back(), a=testonly Automatic update from web-platform-tests [WPT/referrer-policy] Fix races around history.back() Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshigechromium.org> Reviewed-by: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/master{#690280} -- wpt-commits: edd8fe11e357990cf3dafff83504c4f4a605028c wpt-pr: 18055 UltraBlame original commit: 0f7f604a2c761df9d21daba23b9a131790c28702
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 4, 2019
…istory.back(), a=testonly Automatic update from web-platform-tests [WPT/referrer-policy] Fix races around history.back() Because history.back() works asynchronously, there are races between history.back() and subsequent promise_test()s. For exmample: 1. The first 4K-length-referrer subtest finishes and history.back() is called. 2. The second subtest starts, and replaceState() is called. 3. The change to `location` caused by hisotory.back() in Step 1 comes into effect. 4. The second subtest finishes -- at this time, `location` is the original Document URL (not '.../AAAA....AAA' set by replaceState() in Step 2) and thus history.back() has no effects. This CL fixes the race by waiting for `popstate` events that should be fired at the end of async tasks of history.back(). This CL reverts the changes to `referrer-policy-test-case.sub.js` in web-platform-tests/wpt#18401 web-platform-tests/wpt@8b46cbf and waits for popstate instead, because single asyncResolve() isn't sufficient to fix the race condition described above. Bug: 906850 Change-Id: I21e7b8dd6001a13230fd8dca4d1693ae956958ed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1717268 Commit-Queue: Hiroshige Hayashizaki <hiroshigechromium.org> Reviewed-by: Mike West <mkwstchromium.org> Cr-Commit-Position: refs/heads/master{#690280} -- wpt-commits: edd8fe11e357990cf3dafff83504c4f4a605028c wpt-pr: 18055 UltraBlame original commit: 0f7f604a2c761df9d21daba23b9a131790c28702
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main part of the change is the change to ChildSHistory - make it possible to have Go() to be called asynchronously
and also let one to cancel pending history navigations. History object (window.history) can then use either the sync or
async Go(), depending on the dom.window.history.async pref.
LoadDelegate, which is used by GeckoView, needs special handling, since
it spins event loop nestedly. With session history loads and same-document loads we can just
bypass it.
To deal with same-document case, MaybeHandleSameDocumentNavigation is split to IsSameDocumentNavigation,
which collects relevant information about the request and returns true if same-document navigation should happen,
and then later HandleSameDocumentNavigation uses that information to trigger the navigation.
SameDocumentNavigationState is used to pass the information around.
referrer-policy-test-case.sub.js is buggy causing tests to pass only on Firefox with sync history API.
nested-context-navigations-iframe.html.ini is added because of https://bugzilla.mozilla.org/show_bug.cgi?id=1572932
Differential Revision: https://phabricator.services.mozilla.com/D41199
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1563587
gecko-commit: a365d3c4326127be0d85de3c7027cfd4174a4177
gecko-integration-branch: autoland
gecko-reviewers: farre