OOPIF: Don't resurrect a dead process just to create proxies.

This fixes another cause of crashing without a valid parent proxy in
RenderFrameProxy::CreateFrameProxy.  This occurred when a renderer
crashed, and another renderer added a child frame, which triggered a
new proxy for that frame to be created for the crashed process.  The
crashed process was recreated just to create the proxy, and the proxy
creation crashed because its parent proxy didn't exist.

This CL fixes InitRenderFrameProxy to not recreate a process just to
create proxies.  The process should only come back if it ever needs to
host a RenderFrame, and all the proxies should already be created
then.

BUG=476846

Review URL: https://codereview.chromium.org/1138413002

Cr-Commit-Position: refs/heads/master@{#329972}

content/browser/frame_host/render_frame_host_manager.cc

@@ -1347,12 +1347,19 @@ void RenderFrameHostManager::CreatePendingRenderFrameHost(
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
 
-  int opener_route_id = CreateOpenerRenderViewsIfNeeded(
-      old_instance, new_instance, &create_render_frame_flags);
-
   if (pending_render_frame_host_)
     CancelPending();
 
+  // The process for the new SiteInstance may (if we're sharing a process with
+  // another host that already initialized it) or may not (we have our own
+  // process or the existing process crashed) have been initialized. Calling
+  // Init multiple times will be ignored, so this is safe.
+  if (!new_instance->GetProcess()->Init())
+    return;
+
+  int opener_route_id = CreateOpenerRenderViewsIfNeeded(
+      old_instance, new_instance, &create_render_frame_flags);
+
   // Create a non-swapped-out RFH with the given opener.
   pending_render_frame_host_ =
       CreateRenderFrame(new_instance, pending_web_ui(), opener_route_id,
@@ -1436,6 +1443,13 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
   // won't be properly initialized.
   speculative_web_ui_ = CreateWebUI(url, bindings);
 
+  // The process for the new SiteInstance may (if we're sharing a process with
+  // another host that already initialized it) or may not (we have our own
+  // process or the existing process crashed) have been initialized. Calling
+  // Init multiple times will be ignored, so this is safe.
+  if (!new_instance->GetProcess()->Init())
+    return false;
+
   int create_render_frame_flags = 0;
   int opener_route_id =
       CreateOpenerRenderViewsIfNeeded(old_instance, new_instance,

content/browser/frame_host/render_frame_proxy_host.cc

@@ -130,20 +130,31 @@ bool RenderFrameProxyHost::OnMessageReceived(const IPC::Message& msg) {
 
 bool RenderFrameProxyHost::InitRenderFrameProxy() {
   DCHECK(!render_frame_proxy_created_);
-  // The process may (if we're sharing a process with another host that already
-  // initialized it) or may not (we have our own process or the old process
-  // crashed) have been initialized. Calling Init multiple times will be
-  // ignored, so this is safe.
-  if (!GetProcess()->Init())
-    return false;
 
-  DCHECK(GetProcess()->HasConnection());
+  // It is possible to reach this when the process is dead (in particular, when
+  // creating proxies from CreateProxiesForChildFrame).  In that case, don't
+  // create the proxy.  The process shouldn't be resurrected just to create
+  // RenderFrameProxies; it should be restored only if it needs to host a
+  // RenderFrame.  When that happens, the process will be reinitialized, and
+  // all necessary proxies, including any of the ones we skipped here, will be
+  // created by CreateProxiesForSiteInstance. See https://crbug.com/476846
+  if (!GetProcess()->HasConnection())
+    return false;
 
   int parent_routing_id = MSG_ROUTING_NONE;
   if (frame_tree_node_->parent()) {
-    parent_routing_id = frame_tree_node_->parent()
-                            ->render_manager()
-                            ->GetRoutingIdForSiteInstance(site_instance_.get());
+    // It is safe to use GetRenderFrameProxyHost to get the parent proxy, since
+    // new child frames always start out as local frames, so a new proxy should
+    // never have a RenderFrameHost as a parent.
+    RenderFrameProxyHost* parent_proxy =
+        frame_tree_node_->parent()->render_manager()->GetRenderFrameProxyHost(
+            site_instance_.get());
+    CHECK(parent_proxy);
+    // When this is called, the parent RenderFrameProxy should already exist.
+    // The FrameNew_NewFrameProxy will crash on the renderer side if there's no
+    // parent proxy.
+    CHECK(parent_proxy->is_render_frame_proxy_live());
+    parent_routing_id = parent_proxy->GetRoutingID();
     CHECK_NE(parent_routing_id, MSG_ROUTING_NONE);
   }
 

content/browser/site_per_process_browsertest.cc

@@ -889,6 +889,85 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                              "\"done-frame2\"");
 }
 
+// Verify that proxy creation doesn't recreate a crashed process if no frame
+// will be created in it.
+//
+//      1           A                    A          A
+//    / | \       / | \                / | \      / | \    .
+//   2  3  4 ->  B  A  A -> Kill B -> B* A  A -> B* A  A
+//                                                      \  .
+//                                                       A
+//
+// The test kills process B (node 2), creates a child frame of node 4 in
+// process A, and then checks that process B isn't resurrected to create a
+// proxy for the new child frame.  See https://crbug.com/476846.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       CreateChildFrameAfterKillingProcess) {
+  // Navigate to a page with three frames: one cross-site and two same-site.
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_tree/page_with_three_frames.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  EXPECT_EQ(
+      " Site A ------------ proxies for B\n"
+      "   |--Site B ------- proxies for A\n"
+      "   |--Site A ------- proxies for B\n"
+      "   +--Site A ------- proxies for B\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/",
+      DepictFrameTree(root));
+  SiteInstance* b_site_instance =
+      root->child_at(0)->current_frame_host()->GetSiteInstance();
+
+  // Kill the first subframe's renderer (B).
+  RenderProcessHost* child_process =
+      root->child_at(0)->current_frame_host()->GetProcess();
+  RenderProcessHostWatcher crash_observer(
+      child_process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  child_process->Shutdown(0, false);
+  crash_observer.Wait();
+
+  // Add a new child frame to the third subframe.
+  RenderFrameHostCreatedObserver frame_observer(shell()->web_contents(), 1);
+  EXPECT_TRUE(ExecuteScript(
+      root->child_at(2)->current_frame_host(),
+      "document.body.appendChild(document.createElement('iframe'));"));
+  frame_observer.Wait();
+
+  // The new frame should have a RenderFrameProxyHost for B, but it should not
+  // be alive, and B should still not have a process (verified by last line of
+  // expected DepictFrameTree output).
+  EXPECT_EQ(
+      " Site A ------------ proxies for B\n"
+      "   |--Site B ------- proxies for A\n"
+      "   |--Site A ------- proxies for B\n"
+      "   +--Site A ------- proxies for B\n"
+      "        +--Site A -- proxies for B\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/ (no process)",
+      DepictFrameTree(root));
+  FrameTreeNode* grandchild = root->child_at(2)->child_at(0);
+  RenderFrameProxyHost* grandchild_rfph =
+      grandchild->render_manager()->GetRenderFrameProxyHost(b_site_instance);
+  EXPECT_FALSE(grandchild_rfph->is_render_frame_proxy_live());
+
+  // Navigate the second subframe to b.com to recreate process B.
+  TestNavigationObserver observer(shell()->web_contents());
+  GURL b_url = embedded_test_server()->GetURL("b.com", "/title1.html");
+  NavigateFrameToURL(root->child_at(1), b_url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(b_url, observer.last_navigation_url());
+
+  // Ensure that the grandchild RenderFrameProxy in B was created when process
+  // B was restored.
+  EXPECT_TRUE(grandchild_rfph->is_render_frame_proxy_live());
+}
+
 // In A-embed-B-embed-C scenario, verify that killing process B clears proxies
 // of C from the tree.
 //