From 0bf88817dbad9a1645b315887c1a5600e2df6025 Mon Sep 17 00:00:00 2001 From: Rakina Zata Amni Date: Tue, 19 Jul 2022 23:11:03 +0000 Subject: [PATCH] Wait for renderer-initiated navigation cancellations using RendererCancellationThrottle 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 https://github.com/whatwg/html/issues/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 Reviewed-by: Daniel Cheng Reviewed-by: David Bokan Commit-Queue: Rakina Zata Amni Reviewed-by: John Delaney Cr-Commit-Position: refs/heads/main@{#1025993} NOKEYCHECK=True GitOrigin-RevId: af55b5b6ebe03da36d40e71bd733617291c6c9c7 --- tracing/protos/chrome_track_event.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/tracing/protos/chrome_track_event.proto b/tracing/protos/chrome_track_event.proto index fbaf9f9fc..aa15ff1aa 100644 --- a/tracing/protos/chrome_track_event.proto +++ b/tracing/protos/chrome_track_event.proto @@ -672,6 +672,7 @@ message RendererMainThreadTaskExecution { TASK_TYPE_INTERNAL_INPUT_BLOCKING = 77; TASK_TYPE_WEB_GPU = 78; TASK_TYPE_INTERNAL_POST_MESSAGE_FORWARDING = 79; + TASK_TYPE_INTERNAL_NAVIGATION_CANCELLATION = 80; } enum FrameType {