Commit 77beeff9 authored by Alex Moshchuk's avatar Alex Moshchuk Committed by Commit Bot

Fix renderer kills when a process dies in the middle of a cross-site navigation.

This CL fixes a process model bug which resulted in an attempt to
reuse the current process for cross-site navigations if
the current frame's process died after starting the navigation but
before receiving the response.  The problem was in an
IsRenderFrameLive() check in GetSiteInstanceForNavigationRequest(),
which incorrectly led to picking the current SiteInstance in this
case.

This check was introduced in r377920 but it's not clear if there was a
reason for adding it.  This CL removes it.

Bug: 968259
Change-Id: Ibf0dd6a6b6bf0c3a8c9192a2225ec2f5aa87c256
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633591
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664497}
parent ac83aabf
......@@ -2153,7 +2153,6 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
// mode, it is possible for renderer-intiated navigations to be allowed to
// go cross-process. Check it first.
bool can_renderer_initiate_transfer =
render_frame_host_->IsRenderFrameLive() &&
IsURLHandledByNetworkStack(request.common_params().url) &&
IsRendererTransferNeededForNavigation(render_frame_host_.get(),
request.common_params().url);
......
......@@ -14475,6 +14475,42 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
actual_scroll_delta);
}
// Check that if a frame starts a navigation, and the frame's current process
// dies before the response for the navigation comes back, the response will
// not trigger a process kill and will be allowed to commit in a new process.
// See https://crbug.com/968259.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
ProcessDiesBeforeCrossSiteNavigationCompletes) {
GURL first_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), first_url));
scoped_refptr<SiteInstanceImpl> first_site_instance(
web_contents()->GetMainFrame()->GetSiteInstance());
// Start a cross-site navigation and proceed only up to the request start.
GURL second_url(embedded_test_server()->GetURL("b.com", "/title1.html"));
TestNavigationManager delayer(web_contents(), second_url);
EXPECT_TRUE(ExecuteScript(shell(), JsReplace("location = $1", second_url)));
EXPECT_TRUE(delayer.WaitForRequestStart());
// Terminate the current a.com process.
RenderProcessHost* first_process =
web_contents()->GetMainFrame()->GetProcess();
RenderProcessHostWatcher crash_observer(
first_process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
EXPECT_TRUE(first_process->Shutdown(0));
crash_observer.Wait();
EXPECT_FALSE(web_contents()->GetMainFrame()->IsRenderFrameLive());
// Resume the cross-site navigation and ensure it commits in a new
// SiteInstance and process.
delayer.WaitForNavigationFinished();
EXPECT_TRUE(web_contents()->GetMainFrame()->IsRenderFrameLive());
EXPECT_NE(web_contents()->GetMainFrame()->GetProcess(), first_process);
EXPECT_NE(web_contents()->GetMainFrame()->GetSiteInstance(),
first_site_instance);
EXPECT_EQ(second_url, web_contents()->GetMainFrame()->GetLastCommittedURL());
}
class FeaturePolicyPropagationToAuxiliaryBrowsingContextTest
: public SitePerProcessFeaturePolicyJavaScriptBrowserTest,
public testing::WithParamInterface<std::tuple<
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment