From 47bf0dece8c7ff08707e1c8f7e54a81a1318e6a3 Mon Sep 17 00:00:00 2001 From: Nate Chapin Date: Mon, 20 Nov 2023 14:21:26 -0800 Subject: [PATCH] Fire the navigate event earlier for cross-document traversals This earlier timing ensures that we fire the navigate event in the old document when a traversal is served from the bfcache, and is more consistent with non-history cross-document navigate events. Currently, cross-document traversals fire the navigate event at the last possible time: during commit in the renderer, the navigate event is fired in the old document immediately before it is unloaded. The navigate event is not allowed to cancel or intercept a cross-document traversal, otherwise this timing would be too late. We did not reach a firm conclusion on when to fire the navigate event for cross-document traversals during the design of the Navigation API (see https://github.com/WICG/navigation-api/issues/207), and this was the latest of the options considered. This timing has two problems: 1. Traversals served by the back forward cache don't "commit". So the navigate event is erroneously omitted. 2. The navigate event fires after redirects, where for other cross-document navigations, it fires before redirects. This CL adds plumbing for the browser to trigger the navigate event to fire in the renderer in the cross-document traversal case, and moves the time of the navigate event earlier. It now fires after the browser process has decided to allow the traversal to start (i.e., after beforeunload has been fired in any relevant frames, and after start throttles). In the cross-document traversal case where the navigation is not served from bfcache, this will fire the navigate event in parallel with the network request (which is ok because the navigate event can't intercept or cancel the navigation, this timing would not be permissible for other navigation types where the navigate event has more power over the navigation). In the case where no network request is needed (bfcache, about:blank, etc.), the navigate event task gets sent to the renderer immediately before the commit/activation task. Bug: 1475907 Change-Id: I1ef7337e2d85f9cdbfc0110f9f4fe3bcd4dea75d --- .../navigate-history-back-bfcache.html | 24 +++++++++++++ .../return-value/back-204-205-download.html | 9 +++-- .../back-cross-document-event-order.html | 36 +++++++++++++++++++ .../navigate-cross-document-event-order.html | 3 +- 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 navigation-api/navigate-event/navigate-history-back-bfcache.html create mode 100644 navigation-api/ordering-and-transition/back-cross-document-event-order.html diff --git a/navigation-api/navigate-event/navigate-history-back-bfcache.html b/navigation-api/navigate-event/navigate-history-back-bfcache.html new file mode 100644 index 00000000000000..0336ff1c2fe0ed --- /dev/null +++ b/navigation-api/navigate-event/navigate-history-back-bfcache.html @@ -0,0 +1,24 @@ + + + + + + + + diff --git a/navigation-api/navigation-methods/return-value/back-204-205-download.html b/navigation-api/navigation-methods/return-value/back-204-205-download.html index 5bedbf21e86812..3e19bcb0f5017a 100644 --- a/navigation-api/navigation-methods/return-value/back-204-205-download.html +++ b/navigation-api/navigation-methods/return-value/back-204-205-download.html @@ -32,11 +32,9 @@ const indexBefore = i.contentWindow.navigation.currentEntry.index; - // One might be surprised that navigate does not fire. (It does fire for the - // corresponding tests of navigation.navigate(), i.e., this is - // traversal-specific behavior.) See https://github.com/WICG/navigation-api/issues/207 - // for some discussion. - i.contentWindow.navigation.onnavigate = t.unreached_func("onnavigate should not be called"); + let onnavigate_called = false; + i.contentWindow.navigation.onnavigate = () => onnavigate_called = true; + i.contentWindow.onunload = t.unreached_func("onunload should not be called"); i.contentWindow.navigation.onnavigatesuccess = t.unreached_func("onnavigatesuccess should not be called"); i.contentWindow.navigation.onnavigateerror = t.unreached_func("onnavigateerror should not be called"); @@ -47,6 +45,7 @@ await new Promise(resolve => t.step_timeout(resolve, 50)); assert_equals(i.contentWindow.navigation.currentEntry.index, indexBefore); assert_equals(i.contentWindow.navigation.transition, null); + assert_true(onnavigate_called); }, `back() promises to ${description} never settle`); } diff --git a/navigation-api/ordering-and-transition/back-cross-document-event-order.html b/navigation-api/ordering-and-transition/back-cross-document-event-order.html new file mode 100644 index 00000000000000..bde5697001c9ea --- /dev/null +++ b/navigation-api/ordering-and-transition/back-cross-document-event-order.html @@ -0,0 +1,36 @@ + + + + + diff --git a/navigation-api/ordering-and-transition/navigate-cross-document-event-order.html b/navigation-api/ordering-and-transition/navigate-cross-document-event-order.html index 34a9b79fb5f01e..7a8c179259551a 100644 --- a/navigation-api/ordering-and-transition/navigate-cross-document-event-order.html +++ b/navigation-api/ordering-and-transition/navigate-cross-document-event-order.html @@ -6,7 +6,7 @@ async_test(t => { let events = []; function finish() { - assert_array_equals(events, ["onnavigate", "readystateinteractive", "domcontentloaded", "readystatecomplete", "onload", "onpageshow"]); + assert_array_equals(events, ["onnavigate", "onunload", "readystateinteractive", "domcontentloaded", "readystatecomplete", "onload", "onpageshow"]); t.done(); }; @@ -24,6 +24,7 @@ i.contentDocument.addEventListener("DOMContentLoaded", () => events.push("domcontentloaded")); i.contentDocument.onreadystatechange = () => events.push("readystate" + i.contentDocument.readyState); }; + i.contentWindow.onunload = () => events.push("onunload"); i.contentWindow.navigation.onnavigate = () => events.push("onnavigate"); i.contentWindow.navigation.navigate("?1").committed.then( () => events.push("promisefulfilled"), () => events.push("promiserejected"));