diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 1259cd0499d46..f66a85e393d3f 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -152,3 +152,4 @@ cherry-pick-1ed869ad4bb3.patch cherry-pick-8f24f935c903.patch crashpad-initialize-logging.patch make_macos_os_version_numbers_consistent.patch +ignore_renderframehostimpl_detach_for_speculative_rfhs.patch diff --git a/patches/chromium/ignore_renderframehostimpl_detach_for_speculative_rfhs.patch b/patches/chromium/ignore_renderframehostimpl_detach_for_speculative_rfhs.patch new file mode 100644 index 0000000000000..b128bc8ebc9f5 --- /dev/null +++ b/patches/chromium/ignore_renderframehostimpl_detach_for_speculative_rfhs.patch @@ -0,0 +1,162 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Daniel Cheng +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 +Reviewed-by: Nasko Oskov +Cr-Original-Commit-Position: refs/heads/master@{#825903} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530189 +Reviewed-by: Adrian Taylor +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: + +-