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

Conversation

moz-wptsync-bot
Copy link
Collaborator

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 wpt-pr-bot left a 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
@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
@moz-wptsync-bot moz-wptsync-bot deleted the gecko/1563587 branch August 15, 2019 15:56
next();
dump("next \n");
} else {
dump("no next \n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaug----
Copy link
Contributor

smaug---- commented Aug 16, 2019

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

@smaug----
Copy link
Contributor

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants