Skip to content

Commit

Permalink
Properly handle non-current entries with no NavigationApi state
Browse files Browse the repository at this point in the history
1. The NavigationApiHistoryEntry mojom struct doesn't allow a
   null string representation of the the state object, which confuses
   deserialization when no state object is set.
2. When the state object is updated, it does not immediately get
   synced to the browser process. This is because it is either set
   shortly after commit for ordering reasons (and so misses the sync
   that happens during commit), or is updated by
   navigation.updateCurrentEntry() (which is not treated as a
   navigation, so there is no commit). This is problematic because
   the navigation api state object can be exposed on other same-origin
   documents, and that state is gathered in the browser process commit
   logic, which is too early for the renderer's "last chance" sync
   during unload. When that happens, the next document's entry for
   for the previous document has an out-of-date state object.
   Introduce a mechanism to force an immediate history sync outside
   of the commit/freeze/unload ones, and use it when navigation api
   state is updated.

Bug: 1326246
Change-Id: Ie62bf0791c0e366258a32a5e62f193d5597de5a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3662189
Reviewed-by: Charlie Reis <[email protected]>
Reviewed-by: Domenic Denicola <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Nate Chapin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1008064}
  • Loading branch information
natechapin authored and chromium-wpt-export-bot committed May 27, 2022
1 parent a1bd260 commit b28e5b1
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
17 changes: 17 additions & 0 deletions navigation-api/state/cross-document-getState-undefined.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe id="i" src="/common/blank.html"></iframe>
<script>
async_test(t => {
window.onload = t.step_func(() => {
assert_equals(i.contentWindow.navigation.entries().length, 1);
i.contentWindow.location.href = "?1";
i.onload = t.step_func_done(() => {
assert_equals(i.contentWindow.navigation.entries().length, 2);
assert_equals(i.contentWindow.navigation.currentEntry.index, 1);
assert_equals(i.contentWindow.navigation.entries()[0].getState(), undefined);
});
});
}, "Default behavior for entry.getState() for a non-current cross-document entry");
</script>
19 changes: 19 additions & 0 deletions navigation-api/state/cross-document-getState.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe id="i" src="/common/blank.html"></iframe>
<script>
async_test(t => {
window.onload = t.step_func(() => {
i.contentWindow.navigation.updateCurrentEntry({ state: { data: "value" } });
assert_equals(i.contentWindow.navigation.entries().length, 1);
i.contentWindow.location.href = "?1";
i.onload = t.step_func_done(() => {
assert_equals(i.contentWindow.navigation.entries().length, 2);
assert_equals(i.contentWindow.navigation.currentEntry.index, 1);
assert_not_equals(i.contentWindow.navigation.entries()[0].getState(), undefined);
assert_equals(i.contentWindow.navigation.entries()[0].getState().data, "value");
});
});
}, "entry.getState() still works for a non-current cross-document entry");
</script>

0 comments on commit b28e5b1

Please sign in to comment.