diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 471105181713e..9d6957850df11 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -177,3 +177,4 @@ cherry-pick-1a31e2110440.patch m100_change_ownership_of_blobbytesprovider.patch cherry-pick-5ff02e4d7368.patch cherry-pick-12ba78f3fa7a.patch +reland_fix_noopener_case_for_user_activation_consumption.patch diff --git a/patches/chromium/reland_fix_noopener_case_for_user_activation_consumption.patch b/patches/chromium/reland_fix_noopener_case_for_user_activation_consumption.patch new file mode 100644 index 0000000000000..6d635fb48f6fe --- /dev/null +++ b/patches/chromium/reland_fix_noopener_case_for_user_activation_consumption.patch @@ -0,0 +1,242 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Garrett Tanzer +Date: Wed, 2 Mar 2022 18:33:42 +0000 +Subject: Reland "Fix noopener case for user activation consumption" + +This is a reland of e9828a82b5c182dc9a7fb0ae7226c35ba1726e7d + +The MSAN error is from checking status before err in +content/renderer/render_view_impl.cc . +https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/b8821495655905086193/overview + +The fix is to split the check for err and kIgnore into two checks, +and put the err check before kBlocked. + +It is probably possible for the browser to consume user activation +but then eventually mojo returns an error and the renderer doesn't +consume activation, but that seems pretty marginal. + +Original change's description: +> Fix noopener case for user activation consumption +> +> +> The flow for user activation consumption in window.open was as follows: +> +> Renderer: ask the browser to create a new window +> Browser: consume transient user activation (in the browser, and via RPC +> to remote frames only) +> Browser: return success for opener, return ignore for noopener +> Renderer: consume transient user activation upon success +> +> So in the noopener case, the renderer with the local frame where the +> window.open originated didn't have its transient user activation +> consumed. +> +> +> The new behavior is to consume user activation in the calling renderer +> whenever it is consumed in the browser. We accomplish this by returning +> a distinct value kBlocked to represent failure before the browser +> consumes user activation. +> +> Bug: 1264543, 1291210 +> Change-Id: Iffb6e3fd772bef625d3d28e600e6fb73d70ab29f +> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3468171 +> Reviewed-by: Dominic Farolino +> Reviewed-by: Ken Buchanan +> Reviewed-by: Mustaq Ahmed +> Reviewed-by: Charles Reis +> Reviewed-by: Jonathan Ross +> Reviewed-by: Daniel Cheng +> Commit-Queue: Garrett Tanzer +> Cr-Commit-Position: refs/heads/main@{#973876} + +Bug: 1264543, 1291210 +Change-Id: Ie27c4d68db34dfd98adee7cc5c743953dad59834 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3481666 +Reviewed-by: Jonathan Ross +Reviewed-by: Daniel Cheng +Reviewed-by: Mustaq Ahmed +Reviewed-by: Ken Buchanan +Reviewed-by: Charles Reis +Commit-Queue: Garrett Tanzer +Cr-Commit-Position: refs/heads/main@{#976745} + +diff --git a/chrome/browser/site_isolation/chrome_site_per_process_browsertest.cc b/chrome/browser/site_isolation/chrome_site_per_process_browsertest.cc +index a6061a9c5bf1ebb37fb18ecabfb12af3c45ffdf5..f254061d1c118d6505eb1989f65b77b81839b752 100644 +--- a/chrome/browser/site_isolation/chrome_site_per_process_browsertest.cc ++++ b/chrome/browser/site_isolation/chrome_site_per_process_browsertest.cc +@@ -1189,6 +1189,52 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, + EXPECT_FALSE(frame_c_popup_opened); + } + ++// Test that opening a window with `noopener` consumes user activation. ++// crbug.com/1264543, crbug.com/1291210 ++IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, ++ UserActivationConsumptionNoopener) { ++ // Start on a page a.com. ++ GURL main_url(embedded_test_server()->GetURL( ++ "a.com", "/cross_site_iframe_factory.html?a")); ++ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), main_url)); ++ content::WebContents* web_contents = ++ browser()->tab_strip_model()->GetActiveWebContents(); ++ ++ // Activate the frame by executing a dummy script. ++ const std::string no_op_script = "// No-op script"; ++ EXPECT_TRUE(ExecuteScript(web_contents, no_op_script)); ++ ++ // Add a popup observer. ++ content::TestNavigationObserver popup_observer(nullptr); ++ popup_observer.StartWatchingNewWebContents(); ++ ++ // Open a popup from the frame, with `noopener`. This should consume ++ // transient user activation. ++ GURL popup_url(embedded_test_server()->GetURL("popup.com", "/")); ++ EXPECT_TRUE(ExecuteScriptWithoutUserGesture( ++ web_contents, ++ base::StringPrintf( ++ "window.w = window.open('%s'+'title1.html', '_blank', 'noopener');", ++ popup_url.spec().c_str()))); ++ ++ // Try to open another popup. ++ EXPECT_TRUE(ExecuteScriptWithoutUserGesture( ++ web_contents, ++ base::StringPrintf( ++ "window.w = window.open('%s'+'title2.html', '_blank', 'noopener');", ++ popup_url.spec().c_str()))); ++ ++ // Wait and check that only one popup was opened. ++ popup_observer.Wait(); ++ EXPECT_EQ(2, browser()->tab_strip_model()->count()); ++ ++ content::WebContents* popup = ++ browser()->tab_strip_model()->GetActiveWebContents(); ++ EXPECT_EQ(embedded_test_server()->GetURL("popup.com", "/title1.html"), ++ popup->GetLastCommittedURL()); ++ EXPECT_NE(popup, web_contents); ++} ++ + // TODO(crbug.com/1021895): Flaky. + // Tests that a cross-site iframe runs its beforeunload handler when closing a + // tab. See https://crbug.com/853021. +diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc +index 0652dca43b5b5a7b37d96817e5818ec3e9cc9824..bc44948de39c56cbd94305d7b73433da45237ddd 100644 +--- a/content/browser/renderer_host/render_frame_host_impl.cc ++++ b/content/browser/renderer_host/render_frame_host_impl.cc +@@ -6446,17 +6446,22 @@ void RenderFrameHostImpl::CreateNewWindow( + effective_transient_activation_state, params->opener_suppressed, + &no_javascript_access); + +- bool was_consumed = false; +- if (can_create_window) { +- // Consume activation even w/o User Activation v2, to sync other renderers +- // with calling renderer. +- was_consumed = frame_tree_node_->UpdateUserActivationState( +- blink::mojom::UserActivationUpdateType::kConsumeTransientActivation, +- blink::mojom::UserActivationNotificationType::kNone); +- } else { +- std::move(callback).Run(mojom::CreateNewWindowStatus::kIgnore, nullptr); +- return; +- } ++ // If this frame isn't allowed to create a window, return early (before we ++ // consume transient user activation). ++ if (!can_create_window) { ++ std::move(callback).Run(mojom::CreateNewWindowStatus::kBlocked, nullptr); ++ return; ++ } ++ ++ // Otherwise, consume user activation before we proceed. In particular, it is ++ // important to do this before we return from the |opener_suppressed| case ++ // below. ++ // NB: This call will consume activations in the browser and the remote frame ++ // proxies for this frame. The initiating renderer will consume its view of ++ // the activations after we return. ++ bool was_consumed = frame_tree_node_->UpdateUserActivationState( ++ blink::mojom::UserActivationUpdateType::kConsumeTransientActivation, ++ blink::mojom::UserActivationNotificationType::kNone); + + // For Android WebView, we support a pop-up like behavior for window.open() + // even if the embedding app doesn't support multiple windows. In this case, +diff --git a/content/common/frame.mojom b/content/common/frame.mojom +index 63f53ff39240ea59a03f1a6f9d032444b0e4214d..db8f3e3c9794163feea2c020fc100e7f6cc9c73b 100644 +--- a/content/common/frame.mojom ++++ b/content/common/frame.mojom +@@ -531,8 +531,10 @@ struct CreateNewWindowParams { + + // Operation result when the renderer asks the browser to create a new window. + enum CreateNewWindowStatus { +- // Ignore creation of the new window. This can happen because creation is +- // blocked or because the new window should have no opener relationship. ++ // Creation of the new window was blocked, e.g. because the source frame ++ // doesn't have user activation. ++ kBlocked, ++ // Ignore creation of the new window, e.g. because noopener is in effect. + kIgnore, + // Reuse the current window rather than creating a new window. + kReuse, +diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc +index 57f970d15bf03f817680982fa8605c333485804b..7244d1a7f2b4c706c8dc56fafaed2d4b7f7c1c04 100644 +--- a/content/renderer/render_view_impl.cc ++++ b/content/renderer/render_view_impl.cc +@@ -302,8 +302,27 @@ WebView* RenderViewImpl::CreateView( + mojom::CreateNewWindowStatus status; + mojom::CreateNewWindowReplyPtr reply; + auto* frame_host = creator_frame->GetFrameHost(); +- bool err = !frame_host->CreateNewWindow(std::move(params), &status, &reply); +- if (err || status == mojom::CreateNewWindowStatus::kIgnore) ++ if (!frame_host->CreateNewWindow(std::move(params), &status, &reply)) { ++ // The sync IPC failed, e.g. maybe the render process is in the middle of ++ // shutting down. Can't create a new window without the browser process, ++ // so just bail out. ++ return nullptr; ++ } ++ ++ // If creation of the window was blocked (e.g. because this frame doesn't ++ // have user activation), return before consuming user activation. A frame ++ // that isn't allowed to open a window shouldn't be able to consume the ++ // activation for the rest of the frame tree. ++ if (status == mojom::CreateNewWindowStatus::kBlocked) ++ return nullptr; ++ ++ // Consume the transient user activation in the current renderer. ++ consumed_user_gesture = creator->ConsumeTransientUserActivation( ++ blink::UserActivationUpdateSource::kBrowser); ++ ++ // If we should ignore the new window (e.g. because of `noopener`), return ++ // now that user activation was consumed. ++ if (status == mojom::CreateNewWindowStatus::kIgnore) + return nullptr; + + // For Android WebView, we support a pop-up like behavior for window.open() +@@ -323,11 +342,6 @@ WebView* RenderViewImpl::CreateView( + DCHECK_NE(MSG_ROUTING_NONE, reply->main_frame_route_id); + DCHECK_NE(MSG_ROUTING_NONE, reply->widget_params->routing_id); + +- // The browser allowed creation of a new window and consumed the user +- // activation. +- consumed_user_gesture = creator->ConsumeTransientUserActivation( +- blink::UserActivationUpdateSource::kBrowser); +- + // While this view may be a background extension page, it can spawn a visible + // render view. So we just assume that the new one is not another background + // page instead of passing on our own value. +diff --git a/third_party/blink/web_tests/wpt_internal/fenced_frame/restrict-size.https.html b/third_party/blink/web_tests/wpt_internal/fenced_frame/restrict-size.https.html +new file mode 100644 +index 0000000000000000000000000000000000000000..5668407d7e1e6ac7840fc47b76869787cb3f3d63 +--- /dev/null ++++ b/third_party/blink/web_tests/wpt_internal/fenced_frame/restrict-size.https.html +@@ -0,0 +1,15 @@ ++ ++Test fencedframe size restrictions in opaque ads mode. ++ ++ ++ ++ ++ ++ ++ ++ ++