Skip to content

Commit

Permalink
Update AppHistory navigate event behavior for traversals
Browse files Browse the repository at this point in the history
Follows WICG/navigation-api#178

* Always fire navigate event for traversals (currently, we fire it for
  all same-document traversals and for cross-document appHistory.goTo(),
  but not cross-document traversals from the legacy history API, or
  from the UI).
* The AppHistory navigate event should never be cancelable for
  traversals. Previously, it was cancelable for renderer-initiated
  traversals, but this has the potential to cause problems in the case
  where multiple frames are navigating and one frame (but not all)
  cancels. That frame would be out of sync with the authoritative
  history state in the browser process.

Change-Id: I92a3ee0f908acc04c31dc9b8ec57569bd66b4bc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255177
Reviewed-by: Domenic Denicola <[email protected]>
Commit-Queue: Nate Chapin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#937981}
  • Loading branch information
natechapin authored and Chromium LUCI CQ committed Nov 3, 2021
1 parent a969490 commit 87021c0
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 26 deletions.
21 changes: 1 addition & 20 deletions third_party/blink/renderer/core/app_history/app_history.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,24 +492,6 @@ AppHistoryResult* AppHistory::goTo(ScriptState* script_state,
MakeGarbageCollected<AppHistoryApiNavigation>(script_state, this, options,
key);
upcoming_traversals_.insert(key, ongoing_navigation);

AppHistoryEntry* destination = entries_[keys_to_indices_.at(key)];

// TODO(japhet): We will fire the navigate event for same-document navigations
// at commit time, but not cross-document. This should probably move to a more
// central location if we want to fire the navigate event for cross-document
// back-forward navigations in general.
if (!destination->sameDocument()) {
if (DispatchNavigateEvent(
destination->url(), nullptr, NavigateEventType::kCrossDocument,
WebFrameLoadType::kBackForward, UserNavigationInvolvement::kNone,
nullptr,
destination->GetItem()) != AppHistory::DispatchResult::kContinue) {
return EarlyErrorResult(script_state, DOMExceptionCode::kAbortError,
"Navigation was aborted");
}
}

GetSupplementable()
->GetFrame()
->GetLocalFrameHostRemote()
Expand Down Expand Up @@ -658,8 +640,7 @@ AppHistory::DispatchResult AppHistory::DispatchNavigateEvent(
}
init->setDestination(destination);

init->setCancelable(involvement != UserNavigationInvolvement::kBrowserUI ||
type != WebFrameLoadType::kBackForward);
init->setCancelable(type != WebFrameLoadType::kBackForward);
init->setCanTransition(
CanChangeToUrlForHistoryApi(url, GetSupplementable()->GetSecurityOrigin(),
current_url) &&
Expand Down
15 changes: 15 additions & 0 deletions third_party/blink/renderer/core/loader/frame_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,21 @@ void FrameLoader::CommitNavigation(
if (!CancelProvisionalLoaderForNewNavigation())
return;

if (auto* app_history = AppHistory::appHistory(*frame_->DomWindow())) {
if (navigation_params->frame_load_type == WebFrameLoadType::kBackForward) {
auto result = app_history->DispatchNavigateEvent(
navigation_params->url, nullptr, NavigateEventType::kCrossDocument,
WebFrameLoadType::kBackForward,
navigation_params->is_browser_initiated
? UserNavigationInvolvement::kBrowserUI
: UserNavigationInvolvement::kNone,
nullptr, navigation_params->history_item);
DCHECK_EQ(result, AppHistory::DispatchResult::kContinue);
if (!document_loader_)
return;
}
}

FillStaticResponseIfNeeded(navigation_params.get(), frame_);
AssertCanNavigate(navigation_params.get(), frame_);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
i.onload = t.step_func(() => {
i.contentWindow.appHistory.onnavigate = t.step_func_done(e => {
assert_equals(e.navigationType, "traverse");
assert_true(e.cancelable);
assert_false(e.cancelable);
assert_false(e.canTransition);
assert_false(e.userInitiated);
assert_false(e.hashChange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
appHistory.navigate("#foo").committed.then(t.step_func(() => {
appHistory.onnavigate = t.step_func_done(e => {
assert_equals(e.navigationType, "traverse");
assert_true(e.cancelable);
assert_false(e.cancelable);
assert_true(e.canTransition);
assert_false(e.userInitiated);
assert_true(e.hashChange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

appHistory.onnavigate = t.step_func_done(e => {
assert_equals(e.navigationType, "traverse");
assert_true(e.cancelable);
assert_false(e.cancelable);
assert_true(e.canTransition);
assert_false(e.userInitiated);
assert_true(e.hashChange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

appHistory.onnavigate = t.step_func_done(e => {
assert_equals(e.navigationType, "traverse");
assert_true(e.cancelable);
assert_false(e.cancelable);
assert_true(e.canTransition);
assert_false(e.userInitiated);
assert_false(e.hashChange);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<!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(() => {
let target_key = i.contentWindow.appHistory.current.key;
let target_id = i.contentWindow.appHistory.current.id;
i.contentWindow.appHistory.navigate("?foo");
i.onload = t.step_func(() => {
i.contentWindow.appHistory.onnavigate = t.step_func_done(e => {
assert_equals(e.navigationType, "traverse");
assert_false(e.cancelable);
assert_false(e.canTransition);
assert_false(e.userInitiated);
assert_false(e.hashChange);
assert_equals(new URL(e.destination.url).pathname, "/common/blank.html");
assert_false(e.destination.sameDocument);
assert_equals(e.destination.key, target_key);
assert_equals(e.destination.id, target_id);
assert_equals(e.destination.index, 0);
assert_equals(e.formData, null);
assert_equals(e.info, undefined);
});
assert_true(i.contentWindow.appHistory.canGoBack);
i.contentWindow.history.back();
})
});
}, "navigate event for history.back() - cross-document");
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
assert_equals(i.contentWindow.appHistory.entries().length, 2);
assert_equals(i.contentWindow.appHistory.current, i.contentWindow.appHistory.entries()[1]);

// This will be a noop, because navigate events are uncancelable for traversals.
i.contentWindow.appHistory.onnavigate = e => e.preventDefault();

await assertBothRejectDOM(t, i.contentWindow.appHistory.goTo(key), "AbortError", i.contentWindow);
}, "goTo() promise rejection when preventDefault()ing the navigate event (cross-document)");
assertNeverSettles(t, i.contentWindow.appHistory.goTo(key), i.contentWindow);
await new Promise(resolve => i.onload = () => t.step_timeout(resolve, 0));
}, "goTo() promise never settle when preventDefault()ing the navigate event (cross-document)");
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PASS if no errors are reported.
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!doctype html>
PASS if no errors are reported.
<iframe id="i" src="resources/success.html"></iframe>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.queueLoadingScript("i.contentWindow.appHistory.navigate('about:blank');");
testRunner.queueBackNavigation(1);
}

function assert_equals(actual, expected) {
if (!Object.is(actual, expected))
throw Error(message + ': expected: ' + expected + ', actual: ' + actual);
}
function assert_true(expected) { assert_equals(true, expected); }
function assert_false(expected) { assert_equals(false, expected); }

i.onload = () => {
let target_key = i.contentWindow.appHistory.current.key;
let target_id = i.contentWindow.appHistory.current.id;
i.onload = () => {
i.contentWindow.appHistory.onnavigate = e => {
assert_equals(e.navigationType, "traverse");
assert_false(e.cancelable);
assert_false(e.canTransition);
assert_true(e.userInitiated);
assert_false(e.hashChange);
assert_equals(new URL(e.destination.url).pathname, "/misc/resources/success.html");
assert_false(e.destination.sameDocument);
assert_equals(e.destination.key, target_key);
assert_equals(e.destination.id, target_id);
assert_equals(e.destination.index, 0);
assert_equals(e.formData, null);
assert_equals(e.info, undefined);
};
};
};
</script>

0 comments on commit 87021c0

Please sign in to comment.