-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Clarify how to cancel a navigation #3447
Comments
Another place where navigation is canceled: https://html.spec.whatwg.org/multipage/window-object.html#dom-window-stop
(And soon that algorithm will be called in |
Currently, we have different behaviors for the "having a provisional document loader" state versus the "having a queued navigation" state. In the first case, we call FrameLoader::StopAllLoaders(), which cancels the ongoing navigation as well as fetches on the current page (e.g. XMLHttpRequest). In the second, we merely cancel the task to navigate, but do NOT cancel fetches. Indeed, it is recognized that the spec is currently unclear about canceling queued navigation vs. direct navigation (see [1]). However, it is worth noting that Chrome does not always follow the spec with this distinction in the first place (through location.href, for example, which queues a navigation task in Chrome but navigates directly in spec). Additionally, since even the current code cancels navigation in both circumstances (the only disagreement being if peripheral fetches are also canceled), we see no reason to have an inconsistency in this regard (see [2]). This CL now always calls FrameLoader::StopAllLoaders(), for both when we have a provisional loader and when we have a queued navigation, thus ridding ourselves of the inconsistency. By doing so, we implement the "ideal 2" plan laid out in [2], which recently became part of the HTML Standard in [3]. Tests for this new behavior (which this CL fully passes) are in [4], which was imported into our tree by the WPT Importer bot, whose expectations this CL now change. [1]: whatwg/html#3447 [2]: whatwg/html#3975 [3]: whatwg/html#3999 [4]: web-platform-tests/wpt#10789 Bug: 866274 Change-Id: I4e3ffac6b7c07bc8da812f6f210ab5d6933bdfd1 Reviewed-on: https://chromium-review.googlesource.com/1195837 Commit-Queue: Timothy Gu <timothygu@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/master@{#590011}
See also #3730 I was recently looking at this area of code in Firefox, and it looks like Firefox queues a task to navigate from "follow a hyperlink" but not from form submission or location href sets. #3730 suggests that some browsers do it for location href sets too. In any case, there's an interop problem here. This testcase:
ends up loading example.com in Firefox but the software.hixie.ch page in Chrome and Safari. Per spec as written it should load the example.com page, but that seems pretty weird to me. My preference for this stuff in a vacuum would be that neither following a hyperlink nor location sets queue a task. A task would still be queued for javascript: execution under navigation. But I'd really like to know what exactly Chrome and Safari do here. @domenic do you know what Chrome does or who would know? @rniwa same thing for Safari: do you know what the actual behavior is or who would know? |
Based on #3730 I'm going to guess that @zetafunction is the right person on Chrome to describe our navigation/task-queuing behavior. Indeed, this is quite an interop mess. |
Does anyone have a non-racy example of a use case for stopping navigations on The reason I ask is that this is looking like the only place where the page gets synchronous control over cross-document navigation (if you know of others, please tell me) and we might end up implementing that |
I suspect a common pattern that isn't racy in current browsers is location.href = "foo";
// ...later in the same task...
document.open(); |
I think |
…ncellationThrottle Renderer-initiated navigations can be cancelled from the JS task it was initiated from, e.g. if the script runs window.stop() after initiating the navigation. See also whatwg/html#3447 and https://crbug.com/763106 for more background. The renderer cancels navigation by triggering the disconnection of the NavigationClient interface used to start the navigation, eventually calling `NavigationRequest::OnRendererAbortedNavigation()`. Same-SiteInstanceGroup navigations used to use the same NavigationClient for starting and committing navigation. This means even if a CommitNavigation IPC is in-flight at the time of navigation cancellation, the navigation can still get cancelled. Since the same RenderFrame is reused, the CommitNavigation IPC also implicitly waits for the JS task that triggers the navigation to finish, as the commit can't be processed before then. However, with RenderDocument, the RenderFrame and NavigationClient won't be reused, which means navigation cancellations might only affect navigations that haven't entered READY_TO_COMMIT stage. This CL introduces RendererCancellationThrottle, which helps preserve the previous behavior by waiting for the JS task to finish, through deferring the navigations before it gets into the READY_TO_COMMIT stage until the renderer that started the navigation calls the `RendererCancellationWindowEnded` method on the per-navigation NavigationRendererCancellationListener interface (also added by this CL), signifying that the JS task that initiated the navigation had ended and no more renderer-initiated navigation cancellations can happen. See also: https://docs.google.com/document/d/1VNmvEVuaiNH3ypt6YfrYPsJJp8okCTYjooekarOiWN8/edit#heading=h.71sdg5clbek8 Bug: 936696 Change-Id: I07393142c3fa03c1b3937147f730cc4e6dca4eff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561214 Reviewed-by: Alexander Timin <altimin@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: John Delaney <johnidel@chromium.org> Cr-Commit-Position: refs/heads/main@{#1025993}
…ncellationThrottle Renderer-initiated navigations can be cancelled from the JS task it was initiated from, e.g. if the script runs window.stop() after initiating the navigation. See also whatwg/html#3447 and https://crbug.com/763106 for more background. The renderer cancels navigation by triggering the disconnection of the NavigationClient interface used to start the navigation, eventually calling `NavigationRequest::OnRendererAbortedNavigation()`. Same-SiteInstanceGroup navigations used to use the same NavigationClient for starting and committing navigation. This means even if a CommitNavigation IPC is in-flight at the time of navigation cancellation, the navigation can still get cancelled. Since the same RenderFrame is reused, the CommitNavigation IPC also implicitly waits for the JS task that triggers the navigation to finish, as the commit can't be processed before then. However, with RenderDocument, the RenderFrame and NavigationClient won't be reused, which means navigation cancellations might only affect navigations that haven't entered READY_TO_COMMIT stage. This CL introduces RendererCancellationThrottle, which helps preserve the previous behavior by waiting for the JS task to finish, through deferring the navigations before it gets into the READY_TO_COMMIT stage until the renderer that started the navigation calls the `RendererCancellationWindowEnded` method on the per-navigation NavigationRendererCancellationListener interface (also added by this CL), signifying that the JS task that initiated the navigation had ended and no more renderer-initiated navigation cancellations can happen. See also: https://docs.google.com/document/d/1VNmvEVuaiNH3ypt6YfrYPsJJp8okCTYjooekarOiWN8/edit#heading=h.71sdg5clbek8 Bug: 936696 Change-Id: I07393142c3fa03c1b3937147f730cc4e6dca4eff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561214 Reviewed-by: Alexander Timin <altimin@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: John Delaney <johnidel@chromium.org> Cr-Commit-Position: refs/heads/main@{#1025993} NOKEYCHECK=True GitOrigin-RevId: af55b5b6ebe03da36d40e71bd733617291c6c9c7
…ncellationThrottle Renderer-initiated navigations can be cancelled from the JS task it was initiated from, e.g. if the script runs window.stop() after initiating the navigation. See also whatwg/html#3447 and https://crbug.com/763106 for more background. The renderer cancels navigation by triggering the disconnection of the NavigationClient interface used to start the navigation, eventually calling `NavigationRequest::OnRendererAbortedNavigation()`. Same-SiteInstanceGroup navigations used to use the same NavigationClient for starting and committing navigation. This means even if a CommitNavigation IPC is in-flight at the time of navigation cancellation, the navigation can still get cancelled. Since the same RenderFrame is reused, the CommitNavigation IPC also implicitly waits for the JS task that triggers the navigation to finish, as the commit can't be processed before then. However, with RenderDocument, the RenderFrame and NavigationClient won't be reused, which means navigation cancellations might only affect navigations that haven't entered READY_TO_COMMIT stage. This CL introduces RendererCancellationThrottle, which helps preserve the previous behavior by waiting for the JS task to finish, through deferring the navigations before it gets into the READY_TO_COMMIT stage until the renderer that started the navigation calls the `RendererCancellationWindowEnded` method on the per-navigation NavigationRendererCancellationListener interface (also added by this CL), signifying that the JS task that initiated the navigation had ended and no more renderer-initiated navigation cancellations can happen. See also: https://docs.google.com/document/d/1VNmvEVuaiNH3ypt6YfrYPsJJp8okCTYjooekarOiWN8/edit#heading=h.71sdg5clbek8 Bug: 936696 Change-Id: I07393142c3fa03c1b3937147f730cc4e6dca4eff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561214 Reviewed-by: Alexander Timin <altimin@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: John Delaney <johnidel@chromium.org> Cr-Commit-Position: refs/heads/main@{#1025993} NOKEYCHECK=True GitOrigin-RevId: af55b5b6ebe03da36d40e71bd733617291c6c9c7
@rwaldron and I reasoned a bit around these tests web-platform-tests/wpt#8558 and found a possible problem with how HTML cancels navigation attempts. It won't remove any "navigate" tasks that still exist on the task queue. Is that intentional?
Consider this test
https://github.com/w3c/web-platform-tests/pull/8558/files#diff-cc01cb4065c5670fb1bf4b19c88fd562
click()
will dohttps://html.spec.whatwg.org/#the-a-element:following-hyperlinks-2-2
and step 14 in
https://html.spec.whatwg.org/#following-hyperlinks-2
Note "queue a task".
Then the test also has
https://html.spec.whatwg.org/#the-iframe-element:attr-iframe-src-4
https://html.spec.whatwg.org/#otherwise-steps-for-iframe-or-frame-elements
which step 4 says to navigate, without queueing a task:
So the iframe
src
navigation starts first (and it will queue another task for https://html.spec.whatwg.org/#javascript-protocol ). Then the hyperlink navigation starts, which cancels thesrc
navigation.If the above analysis is correct and the behavior is intentional, we should probably clarify in the spec that canceling navigations does not mean to remove tasks from the task queue. OTOH, if removing tasks is a thing browsers do, we should specify that. Maybe each browsing context can have an internal slot to track the most recent navigation to cancel, and have an abstract operation that handles queued task case as well as started case.
Places where the spec cancels navigations:
https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate
step 6:
https://html.spec.whatwg.org/#traverse-the-history-by-a-delta
step 5.1:
The text was updated successfully, but these errors were encountered: