Skip to content

Commit

Permalink
Fire the navigate event earlier for cross-document traversals
Browse files Browse the repository at this point in the history
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 WICG/navigation-api#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
  • Loading branch information
natechapin authored and chromium-wpt-export-bot committed Nov 20, 2023
1 parent cbc5171 commit 735468f
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 6 deletions.
24 changes: 24 additions & 0 deletions navigation-api/navigate-event/navigate-history-back-bfcache.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/utils.js"></script>
<script src="/common/dispatcher/dispatcher.js"></script>
<script src="/html/browsers/browsing-the-web/back-forward-cache/resources/helper.sub.js"></script>

<script>
runBfcacheTest({
targetOrigin: originSameOrigin,
funcBeforeBackNavigation: () => {
window.did_navigate = false;
navigation.onnavigate = () => window.did_navigate = true;
},
async funcAfterAssertion(pageA, pageB) {
// When `funcAfterAssertion` is called, we've already navigated to pageB,
// then gone back to pageA with bfcache. Now go forward to pageB so we can
// observe whether the navigate event fired.
await pageA.execute_script(() => history.forward());
await pageB.execute_script(waitForPageShow);
assert_true(await pageB.execute_script(() => window.did_navigate));
}
}, "navigate event should fire when traversing to a bfcache hit");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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`);
}
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe id="i" src="resources/notify-top-early.html"></iframe>
<script>
async_test(t => {
let events = [];
function finish() {
assert_array_equals(events, ["onnavigate", "onunload", "readystateinteractive", "domcontentloaded", "readystatecomplete", "onload", "onpageshow"]);
t.done();
};

window.onload = t.step_func(() => {
i.contentWindow.navigation.navigate("?1");
i.onload = t.step_func(() => {
window.childStarted = () => {
i.contentWindow.navigation.onnavigatesuccess = () => events.push("onnavigatesuccess");
i.contentWindow.navigation.onnavigateerror = () => events.push("onnavigateerror");
i.contentWindow.onpageshow = () => events.push("onpageshow");
i.contentWindow.onhashchange = () => events.push("onhashchange");
i.contentWindow.onpopstate = () => events.push("onpopstate");
i.onload = t.step_func(() => {
events.push("onload");
t.step_timeout(finish, 0);
});
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.back().committed.then(
() => events.push("promisefulfilled"), () => events.push("promiserejected"));
})
});
}, "back() event ordering for cross-document traversal");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};

Expand All @@ -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"));
Expand Down

0 comments on commit 735468f

Please sign in to comment.