0

Fix the permanent back trapping using same document entries

This fix enhances the history manipulation intervention such that
same document entries cannot be used to achieve permanent back trapping.
The original intervention stops working when there is a user activation
which does not get cleared after the user goes back and forth between
same document entries. So creating new same document entries becomes a
way to hijack the back button. This enhancement fixes the following
scenario:
1. History contains [new-tab-page, a*]
2. A gets a click and adds a same-document entry a1 =>
   [new-tab-page, a, a1*]
3. User goes back to a via the back button
   [new-tab-page, a*, a1]
4. Without any new user activation, a adds another new same document
   entry a2. Page a should now be marked as skippable.
   [new-tab-page, a(skip), a2*]
5. Now when the user goes back, they should land on new-tab-page.

Bug: 1248529
Change-Id: I6de52ab75aef151abd65d4a02de56bd7f791eb19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3182862
Commit-Queue: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1248715}
This commit is contained in:
Shivani Sharma
2024-01-18 13:03:56 +00:00
committed by Chromium LUCI CQ
parent 3f3ff10664
commit eef521b43a
9 changed files with 679 additions and 35 deletions

@ -200,6 +200,8 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
// if it does another redirect without user activation after the user has come
// back to that document again. This implies that a single user activation does
// not mean that the user can be infintely trapped.
// For a same-document version of this test, see
// OnUserGestureResetSameDocumentEntriesSkipFlag (https://crbug.com/1248529).
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
NoUserActivationAfterReturningSetsSkippable) {
GURL non_skippable_url(
@ -467,6 +469,511 @@ class CanGoBackNavigationStateChangedDelegate : public WebContentsDelegate {
};
} // namespace
// Tests the value of honor_sticky_activation_for_history_intervention_ starts
// at true, becomes false after same-document history navigations and gets
// properly reset to true again.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationForHistoryIntervention) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry using the pushState API with a user activation,
// and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
// `honor_sticky_activation_for_history_intervention_` should still be false
// if there is a same-document navigation.
GURL push_state_url2(embedded_test_server()->GetURL("/title1.html"));
script = ("history.pushState('', '','" + push_state_url2.spec() + "');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script,
content::EXECUTE_SCRIPT_NO_USER_GESTURE));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
// Cross-document navigation will reset
// `honor_sticky_activation_for_history_intervention_`.
GURL url1(embedded_test_server()->GetURL(
"/navigation_controller/simple_page_1.html"));
EXPECT_TRUE(NavigateToURL(shell(), url1));
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh1 = root->current_frame_host();
EXPECT_TRUE(rfh1->honor_sticky_activation_for_history_intervention_);
// Go to previous index.
TestNavigationObserver load_observer(shell()->web_contents());
controller.GoToOffset(-1);
load_observer.Wait();
RenderFrameHostImpl* rfh2 = root->current_frame_host();
EXPECT_TRUE(rfh2->honor_sticky_activation_for_history_intervention_);
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh2->HasStickyUserActivationForHistoryIntervention());
}
// Same as above but for forward button instead of back button to ensure the
// intervention works in both directions.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationForHistoryInterventionForward) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry and then do history.back followed by the forward
// button.`honor_sticky_activation_for_history_intervention_` should now be
// false.
// Use the pushState API to add another entry with user gesture.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Go back to the last entry using history.back so that we still honor the
// activation.
EXPECT_TRUE(
ExecJs(shell(), "history.back();", EXECUTE_SCRIPT_NO_USER_GESTURE));
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated forward navigation so the activation is no longer
// honored.
TestNavigationObserver forward_load_observer(shell()->web_contents());
controller.GoForward();
forward_load_observer.Wait();
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
// `honor_sticky_activation_for_history_intervention_` should still be false
// if there is a same-document navigation.
GURL push_state_url2(embedded_test_server()->GetURL("/title1.html"));
script = ("history.pushState('', '','" + push_state_url2.spec() + "');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script,
content::EXECUTE_SCRIPT_NO_USER_GESTURE));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
// Cross-document navigation will reset
// `honor_sticky_activation_for_history_intervention_`.
GURL url1(embedded_test_server()->GetURL(
"/navigation_controller/simple_page_1.html"));
EXPECT_TRUE(NavigateToURL(shell(), url1));
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh1 = root->current_frame_host();
EXPECT_TRUE(rfh1->honor_sticky_activation_for_history_intervention_);
// Go to previous index.
TestNavigationObserver load_observer(shell()->web_contents());
controller.GoToOffset(-1);
load_observer.Wait();
RenderFrameHostImpl* rfh2 = root->current_frame_host();
EXPECT_TRUE(rfh2->honor_sticky_activation_for_history_intervention_);
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh2->HasStickyUserActivationForHistoryIntervention());
}
// Same as above but expects honor_sticky_activation_for_history_intervention_
// to be true when a user activation is received.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
HonorStickyActivationForHistoryInterventionReset) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
// Use the pushState API to add another entry with user gesture.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
// `honor_sticky_activation_for_history_intervention_` should be set as true
// if there is a same-document navigation along with user activation (default
// for ExecJs).
GURL push_state_url2(embedded_test_server()->GetURL("/title1.html"));
script = ("history.pushState('', '','" + push_state_url2.spec() + "');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(rfh->HasStickyUserActivationForHistoryIntervention());
}
// Same as above but expects honor_sticky_activation_for_history_intervention_
// to not be reset with a replaceState call.
IN_PROC_BROWSER_TEST_P(
NavigationControllerHistoryInterventionBrowserTest,
HonorStickyActivationForHistoryInterventionNotResetOnReplaceState) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
// Use the pushState API to add another entry with user gesture.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
// `honor_sticky_activation_for_history_intervention_` should not be set to
// true if there is a replaceState same-document navigation without user
// activation.
GURL replace_state_url2(embedded_test_server()->GetURL("/title1.html"));
script =
("history.replaceState('', '','" + replace_state_url2.spec() + "');");
EXPECT_TRUE(
ExecJs(shell()->web_contents(), script, EXECUTE_SCRIPT_NO_USER_GESTURE));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
EXPECT_FALSE(rfh->HasStickyUserActivationForHistoryIntervention());
}
// Tests that both sticky user activation and
// `honor_sticky_activation_for_history_intervention_`are reset on a reload
// since reload is considered a cross-document navigation.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestStickyActivationOnReload) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
// Use the pushState API to add another entry with user gesture.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
// Reload should cause the sticky activation and
// `honor_sticky_activation_for_history_intervention_` to be reset.
ReloadBlockUntilNavigationsComplete(shell(), 1);
rfh = root->current_frame_host();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
}
// Tests that when `honor_sticky_activation_for_history_intervention_` is
// false, a cross-document navigation should mark the entry as skippable.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationCrossDocument) {
GURL url(embedded_test_server()->GetURL("/frame_tree/top.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* rfh = root->current_frame_host();
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
// Use the pushState API to add another entry with user gesture.
GURL push_state_url1(embedded_test_server()->GetURL("/title1.html"));
std::string script("history.pushState('', '','" + push_state_url1.spec() +
"');");
EXPECT_TRUE(ExecJs(shell()->web_contents(), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(rfh->honor_sticky_activation_for_history_intervention_);
// A cross-document navigation should mark the entry as skippable.
// Navigate to a new same-site document from the renderer without a user
// gesture.
GURL redirected_url(embedded_test_server()->GetURL(
"/navigation_controller/simple_page_1.html"));
EXPECT_TRUE(
NavigateToURLFromRendererWithoutUserGesture(shell(), redirected_url));
EXPECT_EQ(1, controller.GetCurrentEntryIndex());
EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
// Previous entry should have been marked as skippable.
EXPECT_TRUE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
EXPECT_FALSE(
controller.GetLastCommittedEntry()->should_skip_on_back_forward_ui());
}
// Tests that `honor_sticky_activation_for_history_intervention_` should
// stay false and a cross-document navigation should mark the entry as
// skippable, even if the back navigation that set the
// `honor_sticky_activation_for_history_intervention_` to false was for a
// child frame that did a same-document navigation. The next test tests the
// same behavior after the child frame does a cross-document navigation.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationWithChildFrame) {
GURL url(
embedded_test_server()->GetURL("/frame_tree/page_with_one_frame.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* main_rfh = root->current_frame_host();
EXPECT_TRUE(main_rfh->honor_sticky_activation_for_history_intervention_);
// Also check that the honor bit is always true for subframes, since it is
// only consulted for the main frame.
EXPECT_TRUE(root->child_at(0)
->current_frame_host()
->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a same-document entry to a child frame with a user activation and then
// go back.`honor_sticky_activation_for_history_intervention_` should now be
// false.
// Use the pushState API to add another entry with user gesture.
std::string script = "history.pushState({}, 'page 1', 'simple_page_1.html')";
EXPECT_TRUE(ExecJs(root->child_at(0), script));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(main_rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(main_rfh->honor_sticky_activation_for_history_intervention_);
// The honor bit is not consulted for subframes and never changes from its
// default value of true.
EXPECT_TRUE(root->child_at(0)
->current_frame_host()
->honor_sticky_activation_for_history_intervention_);
// A navigation (cross-document or same-document) should mark the entry as
// skippable.
// Navigate from the renderer without a user gesture.
GURL redirected_url(embedded_test_server()->GetURL(
"/navigation_controller/simple_page_1.html"));
EXPECT_TRUE(
NavigateToURLFromRendererWithoutUserGesture(shell(), redirected_url));
EXPECT_EQ(1, controller.GetCurrentEntryIndex());
EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
// Previous entry should have been marked as skippable.
EXPECT_TRUE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
EXPECT_FALSE(
controller.GetLastCommittedEntry()->should_skip_on_back_forward_ui());
}
// Same as the above test except the child frame does a cross-document
// navigation in this test.
IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationWithChildFrameCrossDocument) {
GURL url(
embedded_test_server()->GetURL("/frame_tree/page_with_one_frame.html"));
EXPECT_TRUE(NavigateToURL(shell(), url));
// It is safe to obtain the root frame tree node here, as it doesn't change.
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetPrimaryFrameTree()
.root();
EXPECT_FALSE(root->HasStickyUserActivation());
EXPECT_FALSE(root->HasTransientUserActivation());
RenderFrameHostImpl* main_rfh = root->current_frame_host();
EXPECT_TRUE(main_rfh->honor_sticky_activation_for_history_intervention_);
// Also check that the honor bit is always true for subframes, since it is
// only consulted for the main frame.
EXPECT_TRUE(root->child_at(0)
->current_frame_host()
->honor_sticky_activation_for_history_intervention_);
NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
shell()->web_contents()->GetController());
// Add a cross-document entry to a child frame and then go back.
// `honor_sticky_activation_for_history_intervention_` should now be false.
GURL next_url(embedded_test_server()->GetURL("a.test", "/title1.html"));
EXPECT_TRUE(NavigateToURLFromRenderer(root->child_at(0), next_url));
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_TRUE(main_rfh->honor_sticky_activation_for_history_intervention_);
// Do a browser-initiated back navigation so the activation is no longer
// honored.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
EXPECT_TRUE(root->HasStickyUserActivation());
EXPECT_TRUE(root->HasTransientUserActivation());
EXPECT_FALSE(main_rfh->honor_sticky_activation_for_history_intervention_);
// The honor bit is not consulted for subframes and never changes from its
// default value of true.
EXPECT_TRUE(root->child_at(0)
->current_frame_host()
->honor_sticky_activation_for_history_intervention_);
// A navigation (cross-document or same-document) should mark the entry as
// skippable.
// Navigate from the renderer without a user gesture.
GURL redirected_url(embedded_test_server()->GetURL(
"/navigation_controller/simple_page_1.html"));
EXPECT_TRUE(
NavigateToURLFromRendererWithoutUserGesture(shell(), redirected_url));
EXPECT_EQ(1, controller.GetCurrentEntryIndex());
EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
// Previous entry should have been marked as skippable.
EXPECT_TRUE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
EXPECT_FALSE(
controller.GetLastCommittedEntry()->should_skip_on_back_forward_ui());
}
// Tests that if a navigation entry is marked as skippable due to pushState then
// the flag should be reset if there is a user gesture on this document. All of
// the adjacent entries belonging to the same document will have their skippable
@ -536,7 +1043,9 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
// Go to index 2.
TestNavigationObserver load_observer(shell()->web_contents());
controller.GoToIndex(2);
script = "history.go(-2);";
EXPECT_TRUE(
ExecJs(shell()->web_contents(), script, EXECUTE_SCRIPT_NO_USER_GESTURE));
load_observer.Wait();
EXPECT_EQ(push_state_url1, controller.GetLastCommittedEntry()->GetURL());
@ -574,6 +1083,9 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
EXPECT_TRUE(controller.GetEntryAtIndex(0)->should_skip_on_back_forward_ui());
// goBack should now navigate to entry at index 1.
// This should also reset the page's ability to create non-skippable entries,
// because it is a browser-initiated history navigation.
// See https://crbug.com/1248529.
TestNavigationObserver back_load_observer(shell()->web_contents());
controller.GoBack();
back_load_observer.Wait();
@ -585,14 +1097,29 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerHistoryInterventionBrowserTest,
EXPECT_TRUE(
ExecJs(shell()->web_contents(), script, EXECUTE_SCRIPT_NO_USER_GESTURE));
// We now have
// [skippable_url(skip), redirected_url, push_state_url4*]
// [skippable_url(skip), redirected_url(skip), push_state_url4(skip)*]
// The skippable flag for [1] will now be set since it added an entry without
// any new user activation after user went back to it.
EXPECT_EQ(3, controller.GetEntryCount());
EXPECT_EQ(skippable_url, controller.GetEntryAtIndex(0)->GetURL());
EXPECT_EQ(redirected_url, controller.GetEntryAtIndex(1)->GetURL());
EXPECT_EQ(push_state_url4, controller.GetEntryAtIndex(2)->GetURL());
// The skippable flag will still be unset since this page has seen a user
// gesture once.
EXPECT_TRUE(controller.GetEntryAtIndex(1)->should_skip_on_back_forward_ui());
// Go back to [1] again.
TestNavigationObserver load_observer1(shell()->web_contents());
controller.GoToIndex(1);
load_observer1.Wait();
EXPECT_EQ(redirected_url, controller.GetLastCommittedEntry()->GetURL());
// It should still be skippable.
EXPECT_TRUE(controller.GetEntryAtIndex(1)->should_skip_on_back_forward_ui());
// Simulate a user gesture. ExecuteScript internally also sends a user
// gesture.
EXPECT_TRUE(content::ExecJs(shell()->web_contents(), script));
// The skippable flag for [1] will now be unset.
EXPECT_FALSE(controller.GetEntryAtIndex(1)->should_skip_on_back_forward_ui());
EXPECT_FALSE(controller.GetEntryAtIndex(2)->should_skip_on_back_forward_ui());
}
// Tests that if a navigation entry is marked as skippable due to redirect to a

@ -1349,7 +1349,7 @@ bool NavigationControllerImpl::RendererDidNavigate(
LoadCommittedDetails* details,
bool is_same_document_navigation,
bool was_on_initial_empty_document,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
NavigationRequest* navigation_request) {
DCHECK(navigation_request);
@ -1532,7 +1532,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
case NAVIGATION_TYPE_MAIN_FRAME_NEW_ENTRY:
RendererDidNavigateToNewEntry(
rfh, params, details->is_same_document, details->did_replace_entry,
previous_document_was_activated, navigation_request, details);
previous_document_had_history_intervention_activation,
navigation_request, details);
break;
case NAVIGATION_TYPE_MAIN_FRAME_EXISTING_ENTRY:
RendererDidNavigateToExistingEntry(rfh, params, details->is_same_document,
@ -1542,7 +1543,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
case NAVIGATION_TYPE_NEW_SUBFRAME:
RendererDidNavigateNewSubframe(
rfh, params, details->is_same_document, details->did_replace_entry,
previous_document_was_activated, navigation_request, details);
previous_document_had_history_intervention_activation,
navigation_request, details);
break;
case NAVIGATION_TYPE_AUTO_SUBFRAME:
if (!RendererDidNavigateAutoSubframe(
@ -1919,7 +1921,7 @@ void NavigationControllerImpl::RendererDidNavigateToNewEntry(
const mojom::DidCommitProvisionalLoadParams& params,
bool is_same_document,
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
NavigationRequest* request,
LoadCommittedDetails* commit_details) {
std::unique_ptr<NavigationEntryImpl> new_entry;
@ -2065,7 +2067,7 @@ void NavigationControllerImpl::RendererDidNavigateToNewEntry(
}
SetShouldSkipOnBackForwardUIIfNeeded(
replace_entry, previous_document_was_activated,
replace_entry, previous_document_had_history_intervention_activation,
request->IsRendererInitiated(), request->GetPreviousPageUkmSourceId());
// If this is a history navigation and the old entry has an existing
@ -2228,7 +2230,7 @@ void NavigationControllerImpl::RendererDidNavigateNewSubframe(
const mojom::DidCommitProvisionalLoadParams& params,
bool is_same_document,
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
NavigationRequest* request,
LoadCommittedDetails* commit_details) {
DCHECK(ui::PageTransitionCoreTypeIs(params.transition,
@ -2292,7 +2294,7 @@ void NavigationControllerImpl::RendererDidNavigateNewSubframe(
frame_tree_->root());
SetShouldSkipOnBackForwardUIIfNeeded(
replace_entry, previous_document_was_activated,
replace_entry, previous_document_had_history_intervention_activation,
request->IsRendererInitiated(), request->GetPreviousPageUkmSourceId());
// TODO(creis): Update this to add the frame_entry if we can't find the one
@ -4332,13 +4334,14 @@ void NavigationControllerImpl::SetGetTimestampCallbackForTest(
// History manipulation intervention:
void NavigationControllerImpl::SetShouldSkipOnBackForwardUIIfNeeded(
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
bool is_renderer_initiated,
ukm::SourceId previous_page_load_ukm_source_id) {
// Note that for a subframe, previous_document_was_activated is true if the
// Note that for a subframe,
// previous_document_had_history_intervention_activation is true if the
// gesture happened in any subframe (propagated to main frame) or in the main
// frame itself.
if (replace_entry || previous_document_was_activated ||
if (replace_entry || previous_document_had_history_intervention_activation ||
!is_renderer_initiated) {
return;
}

@ -337,17 +337,21 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
// |was_on_initial_empty_document| indicates whether the document being
// navigated away from was an initial empty document.
//
// |previous_document_was_activated| is true if the previous document had user
// interaction. This is used for a new renderer-initiated navigation to decide
// if the page that initiated the navigation should be skipped on
// back/forward button.
bool RendererDidNavigate(RenderFrameHostImpl* rfh,
const mojom::DidCommitProvisionalLoadParams& params,
LoadCommittedDetails* details,
bool is_same_document_navigation,
bool was_on_initial_empty_document,
bool previous_document_was_activated,
NavigationRequest* navigation_request);
// |previous_document_had_history_intervention_activation| is true if the
// previous document had a user activation that is being honored for the
// history manipulation intervention (i.e., a new user activation is needed
// after same-document back/forward navigations).
// See RFHI::honor_sticky_activation_for_history_intervention_ for details.
// This is used for a new renderer-initiated navigation to decide if the page
// that initiated the navigation should be skipped on back/forward button.
bool RendererDidNavigate(
RenderFrameHostImpl* rfh,
const mojom::DidCommitProvisionalLoadParams& params,
LoadCommittedDetails* details,
bool is_same_document_navigation,
bool was_on_initial_empty_document,
bool previous_document_had_history_intervention_activation,
NavigationRequest* navigation_request);
// Notifies us that we just became active. This is used by the WebContentsImpl
// so that we know to load URLs that were pending as "lazy" loads.
@ -727,7 +731,7 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
const mojom::DidCommitProvisionalLoadParams& params,
bool is_same_document,
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
NavigationRequest* request,
LoadCommittedDetails* details);
void RendererDidNavigateToExistingEntry(
@ -743,7 +747,7 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
const mojom::DidCommitProvisionalLoadParams& params,
bool is_same_document,
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
NavigationRequest* request,
LoadCommittedDetails* details);
bool RendererDidNavigateAutoSubframe(
@ -815,7 +819,7 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
// adding the entry.
void SetShouldSkipOnBackForwardUIIfNeeded(
bool replace_entry,
bool previous_document_was_activated,
bool previous_document_had_history_intervention_activation,
bool is_renderer_initiated,
ukm::SourceId previous_page_load_ukm_source_id);

@ -33,6 +33,7 @@
#include "content/browser/web_package/prefetched_signed_exchange_cache.h"
#include "content/browser/webui/web_ui_controller_factory_registry.h"
#include "content/browser/webui/web_ui_impl.h"
#include "content/common/features.h"
#include "content/common/navigation_params_utils.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/content_browser_client.h"
@ -464,13 +465,17 @@ void Navigator::DidNavigate(
FrameTreeNode* frame_tree_node = render_frame_host->frame_tree_node();
FrameTree& frame_tree = frame_tree_node->frame_tree();
DCHECK_EQ(&frame_tree, &controller_.frame_tree());
base::WeakPtr<RenderFrameHostImpl> old_frame_host =
frame_tree_node->render_manager()->current_frame_host()->GetWeakPtr();
// Save the activation status of the previous page here before it gets reset
// in FrameTreeNode::ResetForNavigation.
bool previous_document_was_activated =
frame_tree.root()->HasStickyUserActivation();
// in FrameTreeNode::ResetForNavigation. Look at the root since the
// activation status for all frames on the page is aggregated in the main
// frame of the current frame tree (but not across nested frame trees).
bool previous_document_history_intervention_activation =
old_frame_host->GetMainFrame()
->HasStickyUserActivationForHistoryIntervention();
if (auto& old_page_info = navigation_request->commit_params().old_page_info) {
// This is a same-site main-frame navigation where we did a proactive
@ -587,7 +592,8 @@ void Navigator::DidNavigate(
base::TimeTicks start = base::TimeTicks::Now();
bool did_navigate = controller_.RendererDidNavigate(
render_frame_host, params, &details, was_within_same_document,
was_on_initial_empty_document, previous_document_was_activated,
was_on_initial_empty_document,
previous_document_history_intervention_activation,
navigation_request.get());
if (!was_within_same_document) {
base::UmaHistogramTimes(

@ -4046,6 +4046,35 @@ void RenderFrameHostImpl::DidNavigate(
// owner element which contains child's required document policy, so there
// is no need to store required document policy in proxies.
}
// Set `honor_sticky_activation_for_history_intervention_` to false if
// it's a browser-initiated same-document back/forward navigation.
// Note that `honor_sticky_activation_for_history_intervention_` is only
// tracked on the main frame so that same-document navigations on a child
// frame cannot be used as a work-around to the intervention.
// navigation_request->GetPageTransition only returns the bit set for
// PAGE_TRANSITION_FORWARD_BACK for back/forward transitions on the main
// frame so retrieve it from the pending entry instead of the
// navigation_request. Also navigation_request->IsSameDocument
// won't be true for the subframe's cross-document back/forward navigation
// case so instead check GetMainFrameDocumentSequenceNumber().
// TODO(creis): Add NavigationRequest::IsSessionHistory() and
// NavigationRequest::IsSamePage() to avoid needing to check either the
// pending NavigationEntry or the PageTransition.
NavigationControllerImpl& controller = frame_tree()->controller();
NavigationEntryImpl* pending_entry = controller.GetPendingEntry();
// pending_entry should be non-nullptr for a back/forward navigation.
if (pending_entry) {
ui::PageTransition transition = pending_entry->GetTransitionType();
if (transition & ui::PAGE_TRANSITION_FORWARD_BACK &&
navigation_request->browser_initiated() &&
pending_entry->GetMainFrameDocumentSequenceNumber() ==
controller.GetLastCommittedEntry()
->GetMainFrameDocumentSequenceNumber()) {
GetMainFrame()->honor_sticky_activation_for_history_intervention_ = false;
}
}
}
void RenderFrameHostImpl::SetLastCommittedOrigin(const url::Origin& origin) {
@ -5920,6 +5949,7 @@ bool RenderFrameHostImpl::IsActiveUserActivation() const {
void RenderFrameHostImpl::ClearUserActivation() {
user_activation_state_.Clear();
history_user_activation_state_.Clear();
GetMainFrame()->honor_sticky_activation_for_history_intervention_ = true;
}
void RenderFrameHostImpl::ConsumeTransientUserActivation() {
@ -5930,6 +5960,7 @@ void RenderFrameHostImpl::ActivateUserActivation(
blink::mojom::UserActivationNotificationType notification_type) {
user_activation_state_.Activate(notification_type);
history_user_activation_state_.Activate();
GetMainFrame()->honor_sticky_activation_for_history_intervention_ = true;
}
void RenderFrameHostImpl::ActivateFocusSourceUserActivation() {
@ -5948,6 +5979,17 @@ void RenderFrameHostImpl::DeactivateFocusSourceUserActivation() {
focus_source_user_activation_state_.Deactivate();
}
bool RenderFrameHostImpl::HasStickyUserActivationForHistoryIntervention()
const {
DCHECK(is_main_frame());
if (!base::FeatureList::IsEnabled(
features::kHistoryInterventionSameDocumentFix) ||
honor_sticky_activation_for_history_intervention_) {
return HasStickyUserActivation();
}
return false;
}
void RenderFrameHostImpl::ClosePage(ClosePageSource source) {
// This path is taken when tab/window close is initiated by either the
// browser process or via a window.close() call through a proxy. In both

@ -2916,6 +2916,13 @@ class CONTENT_EXPORT RenderFrameHostImpl
void ActivateFocusSourceUserActivation();
void DeactivateFocusSourceUserActivation();
// Computes user activation status for history intervention based on the
// actual sticky activation and
// `honor_sticky_activation_for_history_intervention_`. Note that this should
// be invoked on the root RFH and not a subframe's RFH since sticky activation
// from any frame is sufficient and is aggregated at the root.
bool HasStickyUserActivationForHistoryIntervention() const;
// Parameter for `ClosePage()` that indicates whether the request comes from
// the browser or the renderer. This is used to determine whether navigation
// should prevent the page from closing, since navigations should not prevent
@ -3247,6 +3254,25 @@ class CONTENT_EXPORT RenderFrameHostImpl
DetachedIframePagehideHandlerABCB);
FRIEND_TEST_ALL_PREFIXES(BackForwardCacheStillLoadingBrowserTest,
DoesNotCacheIfFrameStillLoading);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationForHistoryIntervention);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerHistoryInterventionBrowserTest,
HonorStickyActivationForHistoryInterventionReset);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerHistoryInterventionBrowserTest,
TestStickyActivationOnReload);
FRIEND_TEST_ALL_PREFIXES(
NavigationControllerHistoryInterventionBrowserTest,
HonorStickyActivationForHistoryInterventionNotResetOnReplaceState);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationCrossDocument);
FRIEND_TEST_ALL_PREFIXES(
NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationForHistoryInterventionForward);
FRIEND_TEST_ALL_PREFIXES(NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationWithChildFrame);
FRIEND_TEST_ALL_PREFIXES(
NavigationControllerHistoryInterventionBrowserTest,
TestHonorStickyActivationWithChildFrameCrossDocument);
class SubresourceLoaderFactoriesConfig;
@ -5001,7 +5027,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
// the Navigation API). Activated when `user_activation_state_` is activated,
// but consumed separately when a page interrupts browser-initiated history
// navigations (e.g., after canceled history navigations or uses of
// CloseWatcher).
// CloseWatcher). Note that this is used for the navigation API and not for
// tracking whether NavigationEntries are skippable (which uses sticky user
// activation and the honor_sticky_activation_for_history_intervention_ bit
// below).
blink::HistoryUserActivationState history_user_activation_state_;
// Manages a transient state tracking mechanism for this frame to verify focus
@ -5011,6 +5040,25 @@ class CONTENT_EXPORT RenderFrameHostImpl
// single focus event if a frame doesn't have any focusable elements.
TransientFocusSourceUserActivation focus_source_user_activation_state_;
// For the history manipulation intervention, main frames track whether to
// honor the sticky user activation for creating non-skippable
// NavigationEntries. The activation can be ignored if a same-document
// history navigation occurs, so that documents in the page cannot create
// more non-skippable entries after a back navigation even though activations
// are not consumed after same-document navigations. See crbug.com/1248529
// for more details.
// More specifically, this field is only updated for main frames, and:
// 1) Starts at true for every new document.
// 2) Is set to false after a same-document back/forward navigation in any
// frame of the page.
// 3) Is set back to true if another user activation is received or a cross
// document commit occurs (including a BFCache activation).
// The value is manually reset on cross-document navigations in
// ClearUserActivation via ResetForNavigation, and not via
// DocumentAssociatedData (which does not get reset for BFCache activations).
bool honor_sticky_activation_for_history_intervention_ = true;
// Used to avoid sending AXTreeData to the renderer if the renderer has not
// been told root ID yet. See UpdateAXTreeData() for more details.
bool needs_ax_root_id_ = true;

@ -53,6 +53,12 @@ BASE_FEATURE(kBlockInsecurePrivateNetworkRequestsFromUnknown,
"BlockInsecurePrivateNetworkRequestsFromUnknown",
base::FEATURE_DISABLED_BY_DEFAULT);
// The fix to crbug.com/1248529 will be behind this default-enabled flag, in
// case it breaks any applications in the wild.
BASE_FEATURE(kHistoryInterventionSameDocumentFix,
"HistoryInterventionSameDocumentFix",
base::FEATURE_ENABLED_BY_DEFAULT);
// When enabled, keyboard user activation will be verified by the browser side.
BASE_FEATURE(kBrowserVerifiedUserActivationKeyboard,
"BrowserVerifiedUserActivationKeyboard",

@ -61,6 +61,9 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kGroupNIKByJoiningOrigin);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kHandleChildThreadTypeChangesInBrowser);
#endif
CONTENT_EXPORT BASE_DECLARE_FEATURE(kHighPriorityBeforeUnload);
BASE_DECLARE_FEATURE(kHistoryInterventionSameDocumentFix);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kInMemoryCodeCache);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kInnerFrameCompositorSurfaceEviction);
#if BUILDFLAG(IS_MAC)

@ -29,7 +29,9 @@ experiences in all browsers. That work is tracked at
https://github.com/whatwg/html/issues/7832
## Invariants
The intervention guarantees the following invariants:
At a high level, the intervention ensures the back/forward buttons always
navigate to a page the user either navigated to or interacted with. It
guarantees the following invariants:
1. Only back/forward navigations triggered by the back/forward buttons will ever
skip history entries. This ensures that the history API's behavior is
unaffected.
@ -38,7 +40,10 @@ The intervention guarantees the following invariants:
3. If a document receives a user activation (before or after creating history
entries), its history entry is not skippable. With an activation, the
document can create many unskippable same-document history entries, until
either a cross-document navigation or a back/forward occurs.
either a cross-document navigation or a back/forward occurs. Note that
same-document back/forwards do not normally reset any prior user activation,
but the intervention stops honoring such activations for creating new
entries until a new activation is received, per https://crbug.com/1248529.
4. All same-document history entries will have the same skippable state. When
marking an entry unskippable after a user activation, this ensures that the
rest of the document's entries work as well. When marking an entry as