diff --git a/content/browser/isolated_origin_browsertest.cc b/content/browser/isolated_origin_browsertest.cc index 5fc0fdeb4c50d0e1e66e635a00b03ba302b48c06..c1523f5cc5d39fa3d8705658860af9968635b50e 100644 --- a/content/browser/isolated_origin_browsertest.cc +++ b/content/browser/isolated_origin_browsertest.cc @@ -4520,6 +4520,62 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, NavigateToBlobURL) { EXPECT_TRUE(load_observer.last_navigation_succeeded()); } +// Test that same-site cross-origin navigations keep user activation even when +// origin isolation is enabled. +IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, UserActivationSameSite) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(bar)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = web_contents()->GetPrimaryFrameTree().root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load cross-origin same-site page into iframe and verify there is still no + // sticky user activation. + GURL first_http_url( + embedded_test_server()->GetURL("isolated.bar.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, first_http_url)); + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform another cross-origin same-site navigation in the iframe. + GURL second_http_url( + embedded_test_server()->GetURL("bar.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, second_http_url)); + + // The cross-origin same-site navigation should keep the sticky user + // activation from the previous page. + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Ensure that top-level navigations can still happen. + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("window.open($1, $2)", first_http_url, "_top"), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + EXPECT_EQ(first_http_url, shell()->web_contents()->GetLastCommittedURL()); +} + // Ensure that --disable-site-isolation-trials disables origin isolation. class IsolatedOriginTrialOverrideTest : public IsolatedOriginFieldTrialTest { public: diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc index 0770dcfcdb8879f01045bf94c8b0f6d3dba6a777..23cfc295ff73fe2110022c82969273879b7ba2dd 100644 --- a/content/browser/renderer_host/frame_tree_node.cc +++ b/content/browser/renderer_host/frame_tree_node.cc @@ -330,19 +330,6 @@ bool FrameTreeNode::IsOutermostMainFrame() const { return !GetParentOrOuterDocument(); } -void FrameTreeNode::ResetForNavigation() { - // This frame has had its user activation bits cleared in the renderer before - // arriving here. We just need to clear them here and in the other renderer - // processes that may have a reference to this frame. - // - // We do not take user activation into account when calculating - // |ResetForNavigationResult|, as we are using it to determine bfcache - // eligibility and the page can get another user gesture after restore. - UpdateUserActivationState( - blink::mojom::UserActivationUpdateType::kClearActivation, - blink::mojom::UserActivationNotificationType::kNone); -} - RenderFrameHostImpl* FrameTreeNode::GetParentOrOuterDocument() const { return GetParentOrOuterDocumentHelper(/*escape_guest_view=*/false, /*include_prospective=*/true); @@ -732,15 +719,22 @@ void FrameTreeNode::BeforeUnloadCanceled() { } } +bool FrameTreeNode::NotifyUserActivationStickyOnly() { + return NotifyUserActivation( + blink::mojom::UserActivationNotificationType::kNone, + /*sticky_only=*/true); +} + bool FrameTreeNode::NotifyUserActivation( - blink::mojom::UserActivationNotificationType notification_type) { + blink::mojom::UserActivationNotificationType notification_type, + bool sticky_only) { // User Activation V2 requires activating all ancestor frames in addition to // the current frame. See // https://html.spec.whatwg.org/multipage/interaction.html#tracking-user-activation. for (RenderFrameHostImpl* rfh = current_frame_host(); rfh; rfh = rfh->GetParent()) { rfh->DidReceiveUserActivation(); - rfh->ActivateUserActivation(notification_type); + rfh->ActivateUserActivation(notification_type, sticky_only); } current_frame_host()->browsing_context_state()->set_has_active_user_gesture( @@ -755,7 +749,7 @@ bool FrameTreeNode::NotifyUserActivation( for (FrameTreeNode* node : frame_tree().Nodes()) { if (node->current_frame_host()->GetLastCommittedOrigin().IsSameOriginWith( current_origin)) { - node->current_frame_host()->ActivateUserActivation(notification_type); + node->current_frame_host()->ActivateUserActivation(notification_type, sticky_only); } } } @@ -823,6 +817,9 @@ bool FrameTreeNode::UpdateUserActivationState( return false; } } break; + case blink::mojom::UserActivationUpdateType::kNotifyActivationStickyOnly: + update_result = NotifyUserActivationStickyOnly(); + break; case blink::mojom::UserActivationUpdateType::kClearActivation: update_result = ClearUserActivation(); break; diff --git a/content/browser/renderer_host/frame_tree_node.h b/content/browser/renderer_host/frame_tree_node.h index 16f7e68c477621ae3aebd5f2fedc07d89bebc029..cc6eceeffed9fb59f3be697035177556c72a21a0 100644 --- a/content/browser/renderer_host/frame_tree_node.h +++ b/content/browser/renderer_host/frame_tree_node.h @@ -111,14 +111,6 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner { bool IsMainFrame() const; bool IsOutermostMainFrame() const; - // Clears any state in this node which was set by the document itself (CSP & - // UserActivationState) and notifies proxies as appropriate. Invoked after - // committing navigation to a new document (since the new document comes with - // a fresh set of CSP). - // TODO(arthursonzogni): Remove this function. The frame/document must not be - // left temporarily with lax state. - void ResetForNavigation(); - FrameTree& frame_tree() const { return frame_tree_.get(); } Navigator& navigator(); @@ -713,8 +705,13 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner { class OpenerDestroyedObserver; // The |notification_type| parameter is used for histograms only. + // |sticky_only| is set to true when propagating sticky user activation during + // cross-document navigations. The transient state remains unchanged. bool NotifyUserActivation( - blink::mojom::UserActivationNotificationType notification_type); + blink::mojom::UserActivationNotificationType notification_type, + bool sticky_only = false); + + bool NotifyUserActivationStickyOnly(); bool ConsumeTransientUserActivation(); diff --git a/content/browser/renderer_host/navigation_controller_impl.cc b/content/browser/renderer_host/navigation_controller_impl.cc index eccc0a69d74778c0cd6cb3f13880644a9ecc5362..f297d366e7a9ef1204c5dbbaf4979e1155a1fe8a 100644 --- a/content/browser/renderer_host/navigation_controller_impl.cc +++ b/content/browser/renderer_host/navigation_controller_impl.cc @@ -3924,7 +3924,9 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams( /*origin_agent_cluster_left_as_default=*/true, /*enabled_client_hints=*/ std::vector(), - /*is_cross_browsing_instance=*/false, /*old_page_info=*/nullptr, + /*is_cross_site_cross_browsing_context_group=*/false, + /*should_have_sticky_user_activation=*/false, + /*old_page_info=*/nullptr, /*http_response_code=*/-1, blink::mojom::NavigationApiHistoryEntryArrays::New(), /*early_hints_preloaded_resources=*/std::vector(), diff --git a/content/browser/renderer_host/navigation_entry_impl.cc b/content/browser/renderer_host/navigation_entry_impl.cc index 5919183ef7e8490a1f197a81800afda869cf574d..a46eb3844f88d21f00db8d4647bbb4d6cab1cd83 100644 --- a/content/browser/renderer_host/navigation_entry_impl.cc +++ b/content/browser/renderer_host/navigation_entry_impl.cc @@ -966,8 +966,9 @@ NavigationEntryImpl::ConstructCommitNavigationParams( true /* origin_agent_cluster_left_as_default */, std::vector< network::mojom::WebClientHintsType>() /* enabled_client_hints */, - false /* is_cross_browsing_instance */, nullptr /* old_page_info */, - -1 /* http_response_code */, + false /* is_cross_site_cross_browsing_context_group */, + false /* should_have_sticky_user_activation */, + nullptr /* old_page_info */, -1 /* http_response_code */, blink::mojom::NavigationApiHistoryEntryArrays::New(), std::vector() /* early_hints_preloaded_resources */, // This timestamp will be populated when the commit IPC is sent. diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index c44599dde59a2de9ac61b91c4f95bceb2b212679..a8eb6b1cb131409bd42418e46fce3434607870a4 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -1318,7 +1318,8 @@ std::unique_ptr NavigationRequest::CreateRendererInitiated( /*origin_agent_cluster_left_as_default=*/true, /*enabled_client_hints=*/ std::vector(), - /*is_cross_browsing_instance=*/false, + /*is_cross_site_cross_browsing_context_group=*/false, + /*should_have_sticky_user_activation=*/false, /*old_page_info=*/nullptr, /*http_response_code=*/-1, blink::mojom::NavigationApiHistoryEntryArrays::New(), /*early_hints_preloaded_resources=*/std::vector(), @@ -1458,7 +1459,8 @@ NavigationRequest::CreateForSynchronousRendererCommit( /*origin_agent_cluster_left_as_default=*/true, /*enabled_client_hints=*/ std::vector(), - /*is_cross_browsing_instance=*/false, + /*is_cross_site_cross_browsing_context_group=*/false, + /*should_have_sticky_user_activation=*/false, /*old_page_info=*/nullptr, http_response_code, blink::mojom::NavigationApiHistoryEntryArrays::New(), /*early_hints_preloaded_resources=*/std::vector(), @@ -5529,7 +5531,7 @@ void NavigationRequest::CommitNavigation() { // For uuid-in-package: resources served from WebBundles, use the Bundle's // origin. - url::Origin origin = + url::Origin origin_to_commit = (common_params_->url.SchemeIs(url::kUuidInPackageScheme) && GetWebBundleURL().is_valid()) ? url::Origin::Create(GetWebBundleURL()) @@ -5538,7 +5540,7 @@ void NavigationRequest::CommitNavigation() { // instead of creating one from a URL which lacks opacity information. isolation_info_for_subresources_ = GetRenderFrameHost()->ComputeIsolationInfoForSubresourcesForPendingCommit( - origin, is_credentialless(), ComputeFencedFrameNonce()); + origin_to_commit, is_credentialless(), ComputeFencedFrameNonce()); DCHECK(!isolation_info_for_subresources_.IsEmpty()); // TODO(https://crbug.com/888079): The storage key's origin is ignored at the @@ -5564,12 +5566,12 @@ void NavigationRequest::CommitNavigation() { browsing_topics::ApiCallerSource::kIframeAttribute); } } - + + RenderFrameHostImpl* old_frame_host = + frame_tree_node_->render_manager()->current_frame_host(); if (!NavigationTypeUtils::IsSameDocument(common_params_->navigation_type)) { // We want to record this for the frame that we are navigating away from. - frame_tree_node_->render_manager() - ->current_frame_host() - ->RecordNavigationSuddenTerminationHandlers(); + old_frame_host->RecordNavigationSuddenTerminationHandlers(); } if (IsServedFromBackForwardCache() || IsPrerenderedPageActivation()) { CommitPageActivation(); @@ -5589,8 +5591,7 @@ void NavigationRequest::CommitNavigation() { if (!weak_self) return; - DCHECK(GetRenderFrameHost() == - frame_tree_node_->render_manager()->current_frame_host() || + DCHECK(GetRenderFrameHost() == old_frame_host || GetRenderFrameHost() == frame_tree_node_->render_manager()->speculative_frame_host()); @@ -5631,7 +5632,7 @@ void NavigationRequest::CommitNavigation() { } // Navigation requests should use the new origin as the partition origin // except if embedded in an outer frame. - url::Origin partition_origin = origin; + url::Origin partition_origin = origin_to_commit; bool is_top_level = frame_tree_node()->GetParentOrOuterDocument() == nullptr; if (!is_top_level) { partition_origin = frame_tree_node() @@ -5640,7 +5641,7 @@ void NavigationRequest::CommitNavigation() { ->GetLastCommittedOrigin(); } - PersistOriginTrialsFromHeaders(origin, partition_origin, response(), + PersistOriginTrialsFromHeaders(origin_to_commit, partition_origin, response(), browser_context); // Update the reduced accept-language to commit if it's empty, and stop @@ -5668,6 +5669,20 @@ void NavigationRequest::CommitNavigation() { response(), frame_tree_node_); } + // Sticky user activation should only be preserved for same-site subframe + // navigations. This is done to prevent newly navigated documents from + // re-using the sticky user activation state from the previously navigated + // document in the frame. We persist user activation across same-site + // navigations for compatibility reasons, and this does not need to match the + // same-site checks used in the process model. See: crbug.com/736415. + // TODO(crbug.com/40228985): Remove this once we find a way to reset + // activation unconditionally without breaking sites in practice. + commit_params_->should_have_sticky_user_activation = + !frame_tree_node_->IsMainFrame() && + old_frame_host->HasStickyUserActivation() && + net::SchemefulSite(old_frame_host->GetLastCommittedOrigin()) == + net::SchemefulSite(origin_to_commit); + // Generate a UKM source and track it on NavigationRequest. This will be // passed down to the blink::Document to be created, if any, and used for UKM // source creation when navigation has successfully committed. diff --git a/content/browser/renderer_host/navigation_request_browsertest.cc b/content/browser/renderer_host/navigation_request_browsertest.cc index 58f3526fe0a60b499fd06943b0577a52210338fb..1a65d7b9b80cbb085eb21464224162919ee6100f 100644 --- a/content/browser/renderer_host/navigation_request_browsertest.cc +++ b/content/browser/renderer_host/navigation_request_browsertest.cc @@ -4465,4 +4465,150 @@ IN_PROC_BROWSER_TEST_F(NavigationRequestBrowserTest, EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), url_b1); } +// Test that same-site cross-origin navigations keep user activation even when +// site isolation is disabled. +IN_PROC_BROWSER_TEST_F(NavigationRequestNoSiteIsolationBrowserTest, + UserActivationSameSite) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(b)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load cross-origin same-site page into iframe and verify there is still no + // sticky user activation. + GURL first_http_url( + embedded_test_server()->GetURL("subdomain.b.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, first_http_url)); + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform another cross-origin same-site navigation in the iframe. + GURL second_http_url(embedded_test_server()->GetURL("b.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, second_http_url)); + + // The cross-origin same-site navigation should keep the sticky user + // activation from the previous page. + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Ensure that top-level navigations can still happen. + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("window.open($1, $2)", first_http_url, "_top"), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + EXPECT_EQ(first_http_url, shell()->web_contents()->GetLastCommittedURL()); +} + +// Test that navigating the outermost main frame to a javascript: url does not +// preserve user activation state. +IN_PROC_BROWSER_TEST_F(NavigationRequestNoSiteIsolationBrowserTest, + UserActivationJavascriptUrlMainFrame) { + GURL main_url(embedded_test_server()->GetURL("a.com", "/title1.html")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(root->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(root->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the root frame user activation. + EXPECT_TRUE(ExecJs(root, "// No-op script")); + EXPECT_TRUE(root->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(root->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform a javascript URL navigation. + GURL javascript_url("javascript:'foo'"); + EXPECT_TRUE(ExecJs(shell()->web_contents(), + JsReplace("location.href = $1", javascript_url), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_EQ("foo", EvalJs(shell()->web_contents(), "document.body.innerText", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // The navigation to the javascript: URL should not keep the sticky user + // activation from the previous page. + // TODO(crbug.com/328296079) The browser and renderer's sticky activation + // state should not be out of sync. Update this test when fixing that bug to + // check that the browser-side clears sticky user activation. + EXPECT_TRUE(root->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(root->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); +} + +// Test that navigating an iframe to a javascript: url preserves the user +// activation state. +IN_PROC_BROWSER_TEST_F(NavigationRequestNoSiteIsolationBrowserTest, + UserActivationJavascriptUrlChildFrame) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(b)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform a javascript URL navigation in the iframe. + GURL javascript_url("javascript:'foo'"); + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("location.href = $1", javascript_url), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_EQ("foo", + EvalJs(child->current_frame_host(), "document.body.innerText", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // The navigation to the javascript: URL should keep the sticky user + // activation from the previous page. + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); +} + } // namespace content diff --git a/content/browser/renderer_host/navigator.cc b/content/browser/renderer_host/navigator.cc index 0e23c0358cf11cdcfd555603aa2e416be3af850a..3696533d71b54811fa777c383c7f725416414499 100644 --- a/content/browser/renderer_host/navigator.cc +++ b/content/browser/renderer_host/navigator.cc @@ -47,6 +47,7 @@ #include "content/public/common/content_constants.h" #include "content/public/common/url_utils.h" #include "net/base/net_errors.h" +#include "net/base/schemeful_site.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "services/metrics/public/cpp/ukm_recorder.h" #include "services/metrics/public/cpp/ukm_source_id.h" @@ -467,7 +468,7 @@ void Navigator::DidNavigate( 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. + // in FrameTreeNode::UpdateUserActivationState. bool previous_document_was_activated = frame_tree.root()->HasStickyUserActivation(); @@ -493,6 +494,9 @@ void Navigator::DidNavigate( delegate_->DidNavigateMainFramePreCommit(frame_tree_node, was_within_same_document); } + + // Store this information before DidNavigateFrame() potentially swaps RFHs. + url::Origin old_frame_origin = old_frame_host->GetLastCommittedOrigin(); // DidNavigateFrame() must be called before replicating the new origin and // other properties to proxies. This is because it destroys the subframes of @@ -506,6 +510,29 @@ void Navigator::DidNavigate( .ShouldClearProxiesOnCommit(), navigation_request->commit_params().frame_policy); + // The main frame, same site, and cross-site navigation checks for user + // activation mirror the checks in DocumentLoader::CommitNavigation() (note: + // CommitNavigation() is not called for same-document navigations, which is + // why we have the !was_within_same_document check). This is done to prevent + // newly navigated pages from re-using the sticky user activation state from + // the previously navigated page in the frame. We persist user activation + // across same-site navigations for compatibility reasons with user + // activation, and does not need to match the same-site checks used in the + // process model. See: crbug.com/736415, and crbug.com/40228985 for the + // specific regression that resulted in this requirement. + if (!was_within_same_document) { + if (!navigation_request->commit_params() + .should_have_sticky_user_activation) { + frame_tree_node->UpdateUserActivationState( + blink::mojom::UserActivationUpdateType::kClearActivation, + blink::mojom::UserActivationNotificationType::kNone); + } else { + frame_tree_node->UpdateUserActivationState( + blink::mojom::UserActivationUpdateType::kNotifyActivationStickyOnly, + blink::mojom::UserActivationNotificationType::kNone); + } + } + // Save the new page's origin and other properties, and replicate them to // proxies, including the proxy created in DidNavigateFrame() to replace the // old frame in cross-process navigation cases. @@ -516,12 +543,6 @@ void Navigator::DidNavigate( render_frame_host->browsing_context_state()->SetInsecureNavigationsSet( params.insecure_navigations_set); - if (!was_within_same_document) { - // Navigating to a new location means a new, fresh set of http headers - // and/or elements - we need to reset Permissions Policy. - frame_tree_node->ResetForNavigation(); - } - // If the committing URL requires the SiteInstance's site to be assigned, // that site assignment should've already happened at ReadyToCommit time. We // should never get here with a SiteInstance that doesn't have a site diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc index 19001deee51321d74c19c6fed24e4fceed314535..b2182a087180cef08fada8cf6b5f52aa6da864ea 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -5617,9 +5617,14 @@ void RenderFrameHostImpl::ConsumeTransientUserActivation() { } void RenderFrameHostImpl::ActivateUserActivation( - blink::mojom::UserActivationNotificationType notification_type) { - user_activation_state_.Activate(notification_type); - history_user_activation_state_.Activate(); + blink::mojom::UserActivationNotificationType notification_type, + bool sticky_only) { + if (sticky_only) { + user_activation_state_.SetHasBeenActive(); + } else { + user_activation_state_.Activate(notification_type); + history_user_activation_state_.Activate(); + } } bool RenderFrameHostImpl::IsHistoryUserActivationActive() const { diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h index 660badeefd206b655720c106436636a5240fd776..d3355eaffa16498525b1f516390c34ec6a286630 100644 --- a/content/browser/renderer_host/render_frame_host_impl.h +++ b/content/browser/renderer_host/render_frame_host_impl.h @@ -2882,7 +2882,8 @@ class CONTENT_EXPORT RenderFrameHostImpl void ClearUserActivation(); void ConsumeTransientUserActivation(); void ActivateUserActivation( - blink::mojom::UserActivationNotificationType notification_type); + blink::mojom::UserActivationNotificationType notification_type, + bool sticky_only = false); // These are called only when RenderFrameHostOwner is iterating over all // frames, not directly from the renderer. diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index 41b6d9f3cd9e077ceb25c219dab424a7ef001d87..36ec149e0c270c983876e81c2f6d09d0228fdb0e 100644 --- a/content/browser/site_per_process_browsertest.cc +++ b/content/browser/site_per_process_browsertest.cc @@ -12944,6 +12944,165 @@ IN_PROC_BROWSER_TEST_P( EXPECT_NE(js_process, web_contents()->GetPrimaryMainFrame()->GetProcess()); } +// Test that cross-site navigations clear user activation. +IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, UserActivationCrossSite) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(b)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = web_contents()->GetPrimaryFrameTree().root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load cross-site page into iframe and verify there is still no sticky user + // activation. + GURL first_http_url(embedded_test_server()->GetURL("d.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, first_http_url)); + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform another cross-site navigation in the iframe. + GURL http_url(embedded_test_server()->GetURL("c.com", "/title1.html")); + EXPECT_TRUE(NavigateToURLFromRendererWithoutUserGesture(child, http_url)); + + // The cross-site navigation should have cleared the user activation. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Ensure that a top-level navigation cannot happen. + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("window.open($1, $2)", http_url, "_top"), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + EXPECT_NE(http_url, shell()->web_contents()->GetLastCommittedURL()); +} + +// Test that same-site cross-origin navigations keep user activation. +// TODO(crbug.com/40228985): Find a way to reset activation here without +// breaking sites in practice. +IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, UserActivationSameSite) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(b)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = web_contents()->GetPrimaryFrameTree().root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load cross-origin same-site page into iframe and verify there is still no + // sticky user activation. + GURL first_http_url( + embedded_test_server()->GetURL("subdomain.b.com", "/title1.html")); + EXPECT_TRUE( + NavigateToURLFromRendererWithoutUserGesture(child, first_http_url)); + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Perform another same-site navigation in the iframe. + GURL http_url(embedded_test_server()->GetURL("b.com", "/title1.html")); + EXPECT_TRUE(NavigateToURLFromRendererWithoutUserGesture(child, http_url)); + + // The cross-origin same-site navigation should keep the sticky user + // activation from the previous page. + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Ensure that top-level navigations can still happen. + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("window.open($1, $2)", http_url, "_top"), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + EXPECT_EQ(http_url, shell()->web_contents()->GetLastCommittedURL()); +} + +// Test that same-origin navigations keep user activation. +// TODO(crbug.com/40228985): Find a way to reset activation here without +// breaking sites in practice. +IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest, UserActivationSameOrigin) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/cross_site_iframe_factory.html?a(b)")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = web_contents()->GetPrimaryFrameTree().root(); + FrameTreeNode* child = root->child_at(0); + + // Sanity check that there is no sticky user activation at first. + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load cross-site page into iframe and verify there is still no sticky user + // activation. + GURL first_http_url(embedded_test_server()->GetURL("c.com", "/title1.html")); + EXPECT_TRUE(NavigateIframeToURL(web_contents(), "child-0", first_http_url)); + EXPECT_FALSE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(false, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Give the child iframe user activation. + EXPECT_TRUE(ExecJs(child, "// No-op script")); + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Load same-origin page into iframe. + GURL http_url(embedded_test_server()->GetURL("c.com", "/title2.html")); + EXPECT_TRUE(NavigateIframeToURL(web_contents(), "child-0", http_url)); + + // The same-origin navigation should keep the sticky user activation from the + // previous page. + EXPECT_TRUE(child->current_frame_host()->HasStickyUserActivation()); + EXPECT_EQ(true, EvalJs(child->current_frame_host(), + "navigator.userActivation.hasBeenActive", + EXECUTE_SCRIPT_NO_USER_GESTURE)); + + // Ensure that top-level navigations can still happen. + EXPECT_TRUE(ExecJs(child->current_frame_host(), + JsReplace("window.open($1, $2)", http_url, "_top"), + EXECUTE_SCRIPT_NO_USER_GESTURE)); + EXPECT_TRUE(WaitForLoadStop(shell()->web_contents())); + EXPECT_EQ(http_url, shell()->web_contents()->GetLastCommittedURL()); +} + // Tests that verify the feature disabling process reuse. class DisableProcessReusePolicyTest : public SitePerProcessBrowserTest { public: diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 0f116aab421d6b1a33082fa79752f7ce57ac0b9d..bc4f150248449ad21a307385378f42f89567a782 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -989,6 +989,9 @@ void FillMiscNavigationParams( navigation_params->is_cross_site_cross_browsing_context_group = commit_params.is_cross_site_cross_browsing_context_group; + navigation_params->should_have_sticky_user_activation = + commit_params.should_have_sticky_user_activation; + #if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_OHOS) // Only android webview uses this. navigation_params->grant_load_local_resources =