fix: focus with OOPIF embedded inside <webview> (#21219)

Backports https://chromium-review.googlesource.com/c/chromium/src/+/1922650
This commit is contained in:
Robo 2019-11-20 09:06:09 -08:00 committed by GitHub
parent 4bc85f777f
commit 0111f6216c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 124 additions and 14 deletions

View file

@ -87,3 +87,4 @@ backport_fix_msstl_compat_in_ui_events.patch
build_win_fix_msstl_compatibility_for_pdf.patch
fix_missing_algorithm_include.patch
add_trustedauthclient_to_urlloaderfactory.patch
fix_focusowningwebcontents_to_handle_renderwidgethosts_for_oopifs.patch

View file

@ -0,0 +1,109 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alex Moshchuk <alexmos@chromium.org>
Date: Tue, 19 Nov 2019 22:41:28 +0000
Subject: Fix FocusOwningWebContents to handle RenderWidgetHosts for OOPIFs.
Previously, FocusOwningWebContents() would not focus anything when
called for an OOPIF's RenderWidgetHost. This is because
GetFocusedRenderWidgetHost() would always return that RWH back,
causing FocusOwningWebContents() to skip the call to
SetAsFocusedWebContentsIfNecessary() because the passed-in RWH matched
the focused RWH.
This is usually not a problem in Chrome, because inner WebContents
can't have OOPIFs and so an inner WebContents would only need to be
focused when this is called from a main frame's RenderWidgetHost, and
the outermost WebContents would probably already be focused via other
means. However, apparently inner WebContents could have OOPIFs in
embedders like Electron, and then this becomes problematic. This CL
fixes FocusOwningWebContents() to always pass in the main frame's
RenderWidgetHost to GetFocusedRenderWidgetHost(), since the latter was
never designed to take an OOPIF's RenderWidgetHost (it expects to take
an event arriving at a main frame's RenderWidgetHostView and then
target it to a subframe's RenderWidgetHost, if needed).
The setup in the added test is similar to ProcessSwapOnInnerContents,
which was also apparently added for an Electron-specific use case
(cross-process navigations inside a <webview>) which isn't currently
possible in regular Chrome.
Change-Id: If9559caf53274d415a360a976ebddfcc323d37dd
Bug: 1026056
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1922650
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716803}
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index a30192cdf4ddf857c9624c9492734e56a7c69ddb..d027c7f2a82daa092447fdb8cca9baf12a39731d 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -14049,6 +14049,52 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, ProcessSwapOnInnerContents) {
EXPECT_NE(a_view, b_view);
}
+// This test ensures that WebContentsImpl::FocusOwningWebContents() focuses an
+// inner WebContents when it is given an OOPIF's RenderWidgetHost inside that
+// inner WebContents. This setup isn't currently supported in Chrome
+// (requiring issue 614463), but it can happen in embedders. See
+// https://crbug.com/1026056.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, FocusInnerContentsFromOOPIF) {
+ GURL main_url(embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(a)"));
+ EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+ // Set up and attach an artificial inner WebContents.
+ FrameTreeNode* child_frame =
+ web_contents()->GetFrameTree()->root()->child_at(0);
+ WebContentsImpl* inner_contents =
+ static_cast<WebContentsImpl*>(CreateAndAttachInnerContents(
+ ToRenderFrameHost(child_frame).render_frame_host()));
+ FrameTreeNode* inner_contents_root = inner_contents->GetFrameTree()->root();
+
+ // Navigate inner WebContents to b.com, and then navigate a subframe on that
+ // page to c.com.
+ GURL b_url(embedded_test_server()->GetURL(
+ "b.com", "/cross_site_iframe_factory.html?b(b)"));
+ NavigateFrameToURL(inner_contents_root, b_url);
+ GURL c_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
+ FrameTreeNode* inner_child = inner_contents_root->child_at(0);
+ NavigateFrameToURL(inner_child, c_url);
+
+ // Because |inner_contents| was set up without kGuestScheme, it can actually
+ // have OOPIFs. Ensure that the subframe is in an OOPIF.
+ EXPECT_NE(inner_contents_root->current_frame_host()->GetSiteInstance(),
+ inner_child->current_frame_host()->GetSiteInstance());
+ EXPECT_TRUE(inner_child->current_frame_host()->IsCrossProcessSubframe());
+
+ // Make sure the outer WebContents is focused to start with.
+ web_contents()->Focus();
+ web_contents()->SetAsFocusedWebContentsIfNecessary();
+ EXPECT_EQ(web_contents(), web_contents()->GetFocusedWebContents());
+
+ // Focus the inner WebContents as if an event were received and dispatched
+ // directly on the |inner_child|'s RenderWidgetHost, and ensure that this
+ // took effect.
+ inner_contents->FocusOwningWebContents(
+ inner_child->current_frame_host()->GetRenderWidgetHost());
+ EXPECT_EQ(inner_contents, web_contents()->GetFocusedWebContents());
+}
+
// Check that a web frame can't navigate a remote subframe to a file: URL. The
// frame should stay at the old URL, and the navigation attempt should produce
// a console error message. See https://crbug.com/894399.
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index a5c4162b3c69b534f843e1b6737392c867fc88ea..cd3ded209dbc54f64d398c2adf4cec34aea87244 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -6420,8 +6420,10 @@ void WebContentsImpl::FocusOwningWebContents(
if (!GuestMode::IsCrossProcessFrameGuest(this) && browser_plugin_guest_)
return;
+ RenderWidgetHostImpl* main_frame_widget_host =
+ GetMainFrame()->GetRenderWidgetHost();
RenderWidgetHostImpl* focused_widget =
- GetFocusedRenderWidgetHost(render_widget_host);
+ GetFocusedRenderWidgetHost(main_frame_widget_host);
if (focused_widget != render_widget_host &&
(!focused_widget ||

View file

@ -9,7 +9,7 @@ failures, as it was unable to find max in the std namespace.
Upstreamed at https://chromium-review.googlesource.com/c/chromium/src/+/1904823.
diff --git a/media/base/byte_queue.cc b/media/base/byte_queue.cc
index 245fafa668568fd308e5a2806dafc1c5f0bf3dbd..f9cd0c214ac6210d4332bad47b56ef8130be67a2 100644
index 245fafa668568fd308e5a2806dafc1c5f0bf3dbd..c54ac79bdfbba4614b9944f73269f5f30ec9a80a 100644
--- a/media/base/byte_queue.cc
+++ b/media/base/byte_queue.cc
@@ -4,6 +4,8 @@

View file

@ -63,7 +63,7 @@ index ab32d5475a0e269d32f6ab71f7dc82aa9e02035a..9ada750416f02d23c2f9faae0c41e4a7
}
diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc
index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4eeed184e 100644
index 81cb954f02363c829947dde830f340a761c80a77..60ffae6ea08187e3ddf1d86ad48f2aeca3519d48 100644
--- a/chrome/browser/printing/print_view_manager_base.cc
+++ b/chrome/browser/printing/print_view_manager_base.cc
@@ -27,10 +27,7 @@
@ -129,7 +129,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
DisconnectFromCurrentPrintJob();
// Don't print / print preview interstitials or crashed tabs.
@@ -131,7 +136,14 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) {
@@ -131,7 +137,14 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) {
return false;
SetPrintingRFH(rfh);
@ -145,7 +145,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
return true;
}
@@ -246,9 +258,9 @@ void PrintViewManagerBase::StartLocalPrintJob(
@@ -246,9 +259,9 @@ void PrintViewManagerBase::StartLocalPrintJob(
void PrintViewManagerBase::UpdatePrintingEnabled() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// The Unretained() is safe because ForEachFrame() is synchronous.
@ -158,7 +158,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
}
void PrintViewManagerBase::NavigationStopped() {
@@ -351,7 +363,7 @@ void PrintViewManagerBase::OnPrintingFailed(int cookie) {
@@ -351,7 +364,7 @@ void PrintViewManagerBase::OnPrintingFailed(int cookie) {
PrintManager::OnPrintingFailed(cookie);
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
@ -167,7 +167,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
#endif
ReleasePrinterQuery();
@@ -449,9 +461,13 @@ void PrintViewManagerBase::OnNotifyPrintJobEvent(
@@ -449,9 +462,13 @@ void PrintViewManagerBase::OnNotifyPrintJobEvent(
content::NotificationService::NoDetails());
break;
}
@ -183,7 +183,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
NOTREACHED();
break;
}
@@ -546,8 +562,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(
@@ -546,8 +563,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(
DCHECK(!quit_inner_loop_);
DCHECK(query);
@ -192,7 +192,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
// We can't print if there is no renderer.
if (!web_contents()->GetRenderViewHost() ||
@@ -562,8 +576,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(
@@ -562,8 +577,6 @@ bool PrintViewManagerBase::CreateNewPrintJob(
print_job_->SetSource(PrintJob::Source::PRINT_PREVIEW, /*source_id=*/"");
#endif // defined(OS_CHROMEOS)
@ -201,7 +201,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
printing_succeeded_ = false;
return true;
}
@@ -612,14 +624,24 @@ void PrintViewManagerBase::ReleasePrintJob() {
@@ -612,14 +625,24 @@ void PrintViewManagerBase::ReleasePrintJob() {
content::RenderFrameHost* rfh = printing_rfh_;
printing_rfh_ = nullptr;
@ -229,7 +229,7 @@ index 81cb954f02363c829947dde830f340a761c80a77..98043d6ec0cb2c55493ef7446a6de0c4
print_job_ = nullptr;
}
diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h
index af49d3e2f8abaf7dc4d82dc3f9beccdf4fbd9f18..b99b5d0a4fce3b15484495948462ee2aa2e321c3 100644
index af49d3e2f8abaf7dc4d82dc3f9beccdf4fbd9f18..c5ef1a4c1577c509e5fbe0fcf06e6dfdba30c92c 100644
--- a/chrome/browser/printing/print_view_manager_base.h
+++ b/chrome/browser/printing/print_view_manager_base.h
@@ -33,6 +33,8 @@ class PrintJob;
@ -253,7 +253,7 @@ index af49d3e2f8abaf7dc4d82dc3f9beccdf4fbd9f18..b99b5d0a4fce3b15484495948462ee2a
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
// Prints the document in |print_data| with settings specified in
@@ -206,9 +210,15 @@ class PrintViewManagerBase : public content::NotificationObserver,
@@ -206,9 +211,15 @@ class PrintViewManagerBase : public content::NotificationObserver,
// The current RFH that is printing with a system printing dialog.
content::RenderFrameHost* printing_rfh_;
@ -270,7 +270,7 @@ index af49d3e2f8abaf7dc4d82dc3f9beccdf4fbd9f18..b99b5d0a4fce3b15484495948462ee2a
// This means we are _blocking_ until all the necessary pages have been
// rendered or the print settings are being loaded.
diff --git a/chrome/browser/printing/printing_message_filter.cc b/chrome/browser/printing/printing_message_filter.cc
index 40762a36024bc48dfe5259520161dc203197bfd0..8f2d0b808df1b68699bb946436b219315f1ea6d3 100644
index 40762a36024bc48dfe5259520161dc203197bfd0..e38aa442df858ce362645230f7642b2eb48262ce 100644
--- a/chrome/browser/printing/printing_message_filter.cc
+++ b/chrome/browser/printing/printing_message_filter.cc
@@ -22,6 +22,7 @@
@ -336,7 +336,7 @@ index 40762a36024bc48dfe5259520161dc203197bfd0..8f2d0b808df1b68699bb946436b21931
std::unique_ptr<PrinterQuery> printer_query =
queue_->PopPrinterQuery(document_cookie);
if (!printer_query) {
@@ -258,7 +267,9 @@ void PrintingMessageFilter::OnUpdatePrintSettingsReply(
@@ -258,7 +266,9 @@ void PrintingMessageFilter::OnUpdatePrintSettingsReply(
std::unique_ptr<PrinterQuery> printer_query,
IPC::Message* reply_msg) {
PrintMsg_PrintPages_Params params;
@ -347,7 +347,7 @@ index 40762a36024bc48dfe5259520161dc203197bfd0..8f2d0b808df1b68699bb946436b21931
params.Reset();
} else {
RenderParamsFromPrintSettings(printer_query->settings(), &params.params);
@@ -296,7 +307,7 @@ void PrintingMessageFilter::OnUpdatePrintSettingsReply(
@@ -296,7 +306,7 @@ void PrintingMessageFilter::OnUpdatePrintSettingsReply(
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
void PrintingMessageFilter::OnCheckForCancel(const PrintHostMsg_PreviewIds& ids,
bool* cancel) {