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

Navigation API: ensure that 204/205/downloads never settle #33234

Merged
merged 1 commit into from Mar 23, 2022

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Mar 17, 2022

See WICG/navigation-api#137.

This requires distinguishing between "dropped" navigations and other
canceled navigations at the FrameLoader/navigation API interface.

This also makes cross-document ongoing navigations generally abortable,
as can be seen by the revamped navigate-cross-document-double.html test.
Previously, we would clean up the ongoing navigation for any
cross-document nav, but per spec we need to keep it around so that it
can be aborted if someone interrupts it. (This applies both to "normal"
cross-document navigations, and ones to 204s/etc.)

Bug: 1183545
Change-Id: I6f8f1771816ddf454d651f7147d3179fba88979b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3532324
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Auto-Submit: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984420}

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.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot changed the title Navigation API: test that 204/205/downloads never settle Navigation API: ensure that 204/205/downloads never settle Mar 21, 2022
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3532324 branch 5 times, most recently from b2aa133 to 6547faf Compare March 23, 2022 16:13
See WICG/navigation-api#137.

This requires distinguishing between "dropped" navigations and other
canceled navigations at the FrameLoader/navigation API interface.

This also makes cross-document ongoing navigations generally abortable,
as can be seen by the revamped navigate-cross-document-double.html test.
Previously, we would clean up the ongoing navigation for any
cross-document nav, but per spec we need to keep it around so that it
can be aborted if someone interrupts it. (This applies both to "normal"
cross-document navigations, and ones to 204s/etc.)

Bug: 1183545
Change-Id: I6f8f1771816ddf454d651f7147d3179fba88979b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3532324
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Auto-Submit: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984420}
@WeizhongX
Copy link
Contributor

WPT Command: python3 ./wpt run --channel=nightly --verify --verify-no-chaos-mode --verify-repeat-loop=0 --verify-repeat-restart=10 --github-checks-text-file=/home/test/artifacts/checkrun.md --affected base_head --log-mach-level=info --log-mach=- -y --no-pause --no-restart-on-unexpected --install-fonts --no-headless --verify-log-full --binary=/home/test/build/firefox/firefox firefox

Some affected tests had inconsistent (flaky) results:

Unstable results

Test Subtest Results Messages
/navigation-api/ordering-and-transition/navigate-cross-document-double.html   OK: 5/10, TIMEOUT: 5/10  
/navigation-api/ordering-and-transition/navigate-cross-document-double.html event and promise ordering when navigate() is called to a cross-document destination, interrupting another navigate() to a cross-document destination FAIL: 5/10, TIMEOUT: 5/10 Test timed out;promise_test: Unhandled rejection with value: object "TypeError: can't access property "currentEntry", i.contentWindow.navigation is undefined"

These may be pre-existing or new flakes. Please try to reproduce (see the above WPT command, though some flags may not be needed when running locally) and determine if your change introduced the flake. If you are unable to reproduce the problem, please tag @web-platform-tests/wpt-core-team in a comment for help.

@WeizhongX
Copy link
Contributor

Pinged CL owner at https://chromium-review.googlesource.com/c/chromium/src/+/3532324

@past, can you help admin merge this?

@WeizhongX
Copy link
Contributor

Response from owner: Since Firefox does not implement this API at all, any flakes in Firefox can be disregarded.

@past past merged commit 2a18935 into master Mar 23, 2022
@past past deleted the chromium-export-cl-3532324 branch March 23, 2022 21:17
@past
Copy link
Member

past commented Mar 23, 2022

Thanks!

DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this pull request Mar 25, 2022
…orm-tests#33234)

See WICG/navigation-api#137.

This requires distinguishing between "dropped" navigations and other
canceled navigations at the FrameLoader/navigation API interface.

This also makes cross-document ongoing navigations generally abortable,
as can be seen by the revamped navigate-cross-document-double.html test.
Previously, we would clean up the ongoing navigation for any
cross-document nav, but per spec we need to keep it around so that it
can be aborted if someone interrupts it. (This applies both to "normal"
cross-document navigations, and ones to 204s/etc.)

Bug: 1183545
Change-Id: I6f8f1771816ddf454d651f7147d3179fba88979b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3532324
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Auto-Submit: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984420}

Co-authored-by: Domenic Denicola <domenic@chromium.org>
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

6 participants