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 <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937981}
NOKEYCHECK=True
GitOrigin-RevId: 87021c0d80a703df61dd68a34eec72c1517b9c17
  • Loading branch information
natechapin authored and copybara-github committed Nov 3, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 07aa982 commit d8518a9
Showing 10 changed files with 94 additions and 26 deletions.
21 changes: 1 addition & 20 deletions blink/renderer/core/app_history/app_history.cc
Original file line number Diff line number Diff line change
@@ -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()
@@ -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) &&
15 changes: 15 additions & 0 deletions blink/renderer/core/loader/frame_loader.cc
Original file line number Diff line number Diff line change
@@ -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_);

Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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);
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
@@ -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 d8518a9

Please sign in to comment.