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

content/browser/frame_host/render_frame_proxy_host.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 
 #include "base/lazy_instance.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 111 matching lines @@
     IPC_MESSAGE_HANDLER(FrameHostMsg_Detach, OnDetach)
     IPC_MESSAGE_HANDLER(FrameHostMsg_OpenURL, OnOpenURL)
     IPC_MESSAGE_HANDLER(FrameHostMsg_RouteMessageEvent, OnRouteMessageEvent)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 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())
+
+  // 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.  In that case, CreateRenderView will reinitialize the

[nasko, 2015/05/14 21:26:29]

As we are trying to kill off RenderView, having CreateRenderView be in charge of
initializing the process is a bit fragile. Let's ensure that we init the process
at the time we know we will be creating a RenderFrame in it. Namely, the callers
of CreateProxiesForSiteInstance (or actually CreateOpenerRenderViewsIfNeeded).

[alexmos, 2015/05/14 22:02:50]

Done.

+  // process, 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;
 
-  DCHECK(GetProcess()->HasConnection());
-
   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());
+    RenderFrameProxyHost* parent_proxy =
+        frame_tree_node_->parent()->render_manager()->GetRenderFrameProxyHost(

[alexmos, 2015/05/14 17:01:43]

Using GetRenderFrameProxyHost instead of GetRoutingIdForSiteInstance should be
safe, since FrameMsg_NewFrameProxy expects the parent to always be a
RenderFrameProxy.

[nasko, 2015/05/14 21:26:29]

This is a very good observation. Let's put that in a comment to explain why a
proxy will never have a RFH as a parent.

[alexmos, 2015/05/14 22:02:50]

Done.

+            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.
+    DCHECK(parent_proxy->is_render_frame_proxy_live());

[alexmos, 2015/05/14 17:01:42]

I don't think we need my previous recursively-initialize-the-parent fix with the
way this is currently called.  We can revisit that if we ever hit this (which
would also yield another repro for 476846).

[nasko, 2015/05/14 21:26:29]

Acknowledged.

[alexmos, 2015/05/14 22:02:50]

I also converted this DCHECK to a CHECK.  (This should only be reachable in
--site-per-process.)

+    parent_routing_id = parent_proxy->GetRoutingID();
     CHECK_NE(parent_routing_id, MSG_ROUTING_NONE);
   }
 
   Send(new FrameMsg_NewFrameProxy(routing_id_,
                                   parent_routing_id,
                                   frame_tree_node_->frame_tree()
                                       ->GetRenderViewHost(site_instance_.get())
                                       ->GetRoutingID(),
                                   frame_tree_node_
                                       ->current_replication_state()));
@@ skipping 100 matching lines @@
         base::Bind(&MessagePortMessageFilter::RouteMessageEventWithMessagePorts,
                    message_port_message_filter, target_rfh->GetRoutingID(),
                    new_params));
   } else {
     target_rfh->Send(
         new FrameMsg_PostMessageEvent(target_rfh->GetRoutingID(), new_params));
   }
 }
 
 }  // namespace content