Skip to content
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
merged 1 commit into from Aug 15, 2019
Merged

Conversation

@moz-wptsync-bot
Copy link
Collaborator

moz-wptsync-bot commented Aug 13, 2019

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

Copy link
Collaborator

wpt-pr-bot left a comment

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 moz-wptsync-bot force-pushed the gecko/1563587 branch from 2c3ae1b to 167b5f3 Aug 14, 2019
@jgraham
Copy link
Contributor

jgraham commented Aug 15, 2019

Failures here seem to be timeouts, so merging.

@jgraham jgraham merged commit 8b46cbf into master Aug 15, 2019
10 of 14 checks passed
10 of 14 checks passed
website-build-and-publish
Details
manifest-build-and-tag
Details
update-pr-preview
Details
Taskcluster (pull_request) TaskGroup: failure
Details
upstream/gecko Landed on mozilla-central
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - safari[experimental] Safari results
Details
Azure Pipelines Build #20190814.22 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
wpt.fyi - firefox[experimental] Firefox results
Details
@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1563587 branch Aug 15, 2019
@smaug----
Copy link
Contributor

smaug---- commented Aug 16, 2019

crap, how did I leave that there. Will fix.

@smaug----
Copy link
Contributor

smaug---- commented Aug 16, 2019

removed dump in gecko tree

chromium-wpt-export-bot added 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 added 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 added 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 added 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 added 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.