|
@@ -0,0 +1,162 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Daniel Cheng <[email protected]>
|
|
|
+Date: Wed, 11 Nov 2020 00:54:41 +0000
|
|
|
+Subject: Ignore RenderFrameHostImpl::Detach() for speculative RFHs.
|
|
|
+
|
|
|
+Currently, this all happens to work by chance, because the speculative
|
|
|
+RFH or the entire FTN happens to be torn down before the browser process
|
|
|
+ever processes a Detach() IPC for a speculative RFH.
|
|
|
+
|
|
|
+However, there are a number of followup CLs that restructure how
|
|
|
+provisional RenderFrames are managed and owned in the renderer process.
|
|
|
+To simplify those CLs, explicitly branch in Detach() based on whether or
|
|
|
+not the RFH is speculative. In the future, additional logic may be added
|
|
|
+to the speculative branch (e.g. cancelling the navigation, if
|
|
|
+appropriate).
|
|
|
+
|
|
|
+(cherry picked from commit cf054220a2e1570a9149220494de8826c2e9d4db)
|
|
|
+
|
|
|
+Bug: 1146709
|
|
|
+Change-Id: I6490a90f7b447422d698676665b52f6f3a6f8ffd
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524280
|
|
|
+Commit-Queue: Daniel Cheng <[email protected]>
|
|
|
+Reviewed-by: Nasko Oskov <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/master@{#825903}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530189
|
|
|
+Reviewed-by: Adrian Taylor <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4240@{#1430}
|
|
|
+Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
|
|
|
+
|
|
|
+diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
|
|
|
+index 8a3afc59f96e0f29997d0e239726217d490189d8..0c71cf19a2d8587e3e341d963da72c03a092b453 100644
|
|
|
+--- a/content/browser/frame_host/render_frame_host_impl.cc
|
|
|
++++ b/content/browser/frame_host/render_frame_host_impl.cc
|
|
|
+@@ -2403,6 +2403,9 @@ void RenderFrameHostImpl::UpdateRenderProcessHostFramePriorities() {
|
|
|
+ }
|
|
|
+
|
|
|
+ void RenderFrameHostImpl::OnDetach() {
|
|
|
++ if (frame_tree_node_->render_manager()->speculative_frame_host() == this)
|
|
|
++ return;
|
|
|
++
|
|
|
+ if (!parent_) {
|
|
|
+ bad_message::ReceivedBadMessage(GetProcess(),
|
|
|
+ bad_message::RFH_DETACH_MAIN_FRAME);
|
|
|
+diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
|
|
|
+index 1e8f9b19e4bdeb0b6a371e384e30e10b7986137e..7770582317986869ff38658e2abfd4ba836c5bca 100644
|
|
|
+--- a/content/browser/site_per_process_browsertest.cc
|
|
|
++++ b/content/browser/site_per_process_browsertest.cc
|
|
|
+@@ -10362,6 +10362,36 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
|
|
|
+ EXPECT_EQ("opener-ping-reply", response);
|
|
|
+ }
|
|
|
+
|
|
|
++IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
|
|
|
++ DetachSpeculativeRenderFrameHost) {
|
|
|
++ // Commit a page with one iframe.
|
|
|
++ GURL main_url(embedded_test_server()->GetURL(
|
|
|
++ "a.com", "/cross_site_iframe_factory.html?a(a)"));
|
|
|
++ EXPECT_TRUE(NavigateToURL(shell(), main_url));
|
|
|
++
|
|
|
++ // Start a cross-site navigation.
|
|
|
++ GURL cross_site_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
|
|
|
++ TestNavigationManager nav_manager(shell()->web_contents(), cross_site_url);
|
|
|
++ BeginNavigateIframeToURL(web_contents(), "child-0", cross_site_url);
|
|
|
++
|
|
|
++ // Wait for the request, but don't commit it yet. This should create a
|
|
|
++ // speculative RenderFrameHost.
|
|
|
++ ASSERT_TRUE(nav_manager.WaitForRequestStart());
|
|
|
++ FrameTreeNode* root = web_contents()->GetFrameTree()->root();
|
|
|
++ RenderFrameHostImpl* speculative_rfh = root->current_frame_host()
|
|
|
++ ->child_at(0)
|
|
|
++ ->render_manager()
|
|
|
++ ->speculative_frame_host();
|
|
|
++ EXPECT_TRUE(speculative_rfh);
|
|
|
++
|
|
|
++ // Currently, the browser process never handles an explicit Detach() for a
|
|
|
++ // speculative RFH, since the speculative RFH or the entire FTN is always
|
|
|
++ // destroyed before the renderer sends this IPC.
|
|
|
++ speculative_rfh->Detach();
|
|
|
++
|
|
|
++ // Passes if there is no crash.
|
|
|
++}
|
|
|
++
|
|
|
+ #if defined(OS_ANDROID)
|
|
|
+
|
|
|
+ namespace {
|
|
|
+diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc
|
|
|
+index d7f88a819505305d690e35b3b78862c1b9fd8f5e..9b2f0bb265293549b84d37279ba1e2e7c314a737 100644
|
|
|
+--- a/content/public/test/browser_test_utils.cc
|
|
|
++++ b/content/public/test/browser_test_utils.cc
|
|
|
+@@ -624,15 +624,21 @@ bool NavigateToURL(WebContents* web_contents,
|
|
|
+ bool NavigateIframeToURL(WebContents* web_contents,
|
|
|
+ const std::string& iframe_id,
|
|
|
+ const GURL& url) {
|
|
|
++ TestNavigationObserver load_observer(web_contents);
|
|
|
++ bool result = BeginNavigateIframeToURL(web_contents, iframe_id, url);
|
|
|
++ load_observer.Wait();
|
|
|
++ return result;
|
|
|
++}
|
|
|
++
|
|
|
++bool BeginNavigateIframeToURL(WebContents* web_contents,
|
|
|
++ const std::string& iframe_id,
|
|
|
++ const GURL& url) {
|
|
|
+ std::string script = base::StringPrintf(
|
|
|
+ "setTimeout(\""
|
|
|
+ "var iframes = document.getElementById('%s');iframes.src='%s';"
|
|
|
+ "\",0)",
|
|
|
+ iframe_id.c_str(), url.spec().c_str());
|
|
|
+- TestNavigationObserver load_observer(web_contents);
|
|
|
+- bool result = ExecuteScript(web_contents, script);
|
|
|
+- load_observer.Wait();
|
|
|
+- return result;
|
|
|
++ return ExecuteScript(web_contents, script);
|
|
|
+ }
|
|
|
+
|
|
|
+ void NavigateToURLBlockUntilNavigationsComplete(WebContents* web_contents,
|
|
|
+diff --git a/content/public/test/browser_test_utils.h b/content/public/test/browser_test_utils.h
|
|
|
+index 8c2e904462c655c9a587e5243572e60b8327ad1c..d4b16a127f92559469f386b734ffa1e6d20eb51e 100644
|
|
|
+--- a/content/public/test/browser_test_utils.h
|
|
|
++++ b/content/public/test/browser_test_utils.h
|
|
|
+@@ -137,6 +137,12 @@ bool NavigateIframeToURL(WebContents* web_contents,
|
|
|
+ const std::string& iframe_id,
|
|
|
+ const GURL& url);
|
|
|
+
|
|
|
++// Similar to |NavigateIframeToURL()| but returns as soon as the navigation is
|
|
|
++// initiated.
|
|
|
++bool BeginNavigateIframeToURL(WebContents* web_contents,
|
|
|
++ const std::string& iframe_id,
|
|
|
++ const GURL& url);
|
|
|
++
|
|
|
+ // Generate a URL for a file path including a query string.
|
|
|
+ GURL GetFileUrlWithQuery(const base::FilePath& path,
|
|
|
+ const std::string& query_string);
|
|
|
+diff --git a/content/test/data/cross_site_iframe_factory.html b/content/test/data/cross_site_iframe_factory.html
|
|
|
+index 959f45a6be7f233082e364f90d6875d125ae6fe6..e4807d1ad3f7526d7b21843ba8f49e50f7ed8d7e 100644
|
|
|
+--- a/content/test/data/cross_site_iframe_factory.html
|
|
|
++++ b/content/test/data/cross_site_iframe_factory.html
|
|
|
+@@ -10,12 +10,12 @@ Example usage in a browsertest, explained:
|
|
|
+ When you navigate to the above URL, the outer document (on a.com) will create a
|
|
|
+ single iframe:
|
|
|
+
|
|
|
+- <iframe src="http://b.com:1234/cross_site_iframe_factory.html?b(c(),d())">
|
|
|
++ <iframe id="child-0" src="http://b.com:1234/cross_site_iframe_factory.html?b(c(),d())">
|
|
|
+
|
|
|
+ Inside of which, then, are created the two leaf iframes:
|
|
|
+
|
|
|
+- <iframe src="http://c.com:1234/cross_site_iframe_factory.html?c()">
|
|
|
+- <iframe src="http://d.com:1234/cross_site_iframe_factory.html?d()">
|
|
|
++ <iframe id="child-0" src="http://c.com:1234/cross_site_iframe_factory.html?c()">
|
|
|
++ <iframe id="child-1" src="http://d.com:1234/cross_site_iframe_factory.html?d()">
|
|
|
+
|
|
|
+ Add iframe options by enclosing them in '{' and '}' characters after the
|
|
|
+ hostname (multiple options can be separated with commas):
|
|
|
+@@ -24,8 +24,8 @@ hostname (multiple options can be separated with commas):
|
|
|
+
|
|
|
+ Will create two iframes:
|
|
|
+
|
|
|
+- <iframe src="http://a.com:1234/cross_site_iframe_factory.html?b()" allowfullscreen>
|
|
|
+- <iframe src="http://c.com:1234/cross_site_iframe_factory.html?c{sandbox-allow-scripts}(d())" sandbox="allow-scripts">
|
|
|
++ <iframe id="child-0" src="http://a.com:1234/cross_site_iframe_factory.html?b()" allowfullscreen>
|
|
|
++ <iframe id="child-1" src="http://c.com:1234/cross_site_iframe_factory.html?c{sandbox-allow-scripts}(d())" sandbox="allow-scripts">
|
|
|
+
|
|
|
+ To specify the site for each iframe, you can use a simple identifier like "a"
|
|
|
+ or "b", and ".com" will be automatically appended. You can also specify a port
|