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

content/browser/frame_host/render_frame_host_manager.cc

 // Copyright 2013 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_host_manager.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 1328 matching lines @@
     SiteInstance* old_instance,
     SiteInstance* new_instance,
     bool is_main_frame) {
   int create_render_frame_flags = 0;
   if (is_main_frame)
     create_render_frame_flags |= CREATE_RF_FOR_MAIN_FRAME_NAVIGATION;
 
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
 
+  // 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 old process crashed) have been initialized. Calling Init

[nasko, 2015/05/14 22:10:52]

nit: s/old/existing/

[alexmos, 2015/05/14 22:54:16]

Done.

+  // multiple times will be ignored, so this is safe.
+  new_instance->GetProcess()->Init();

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

Should I also return early here if this fails?  CreateRenderFrame/View also
check GetProcess()->Init() and will return early, but not sure if things like
CancelPending() should still happen even if process creation failed.

[nasko, 2015/05/14 22:10:52]

Returning early is probably desired. Looking at the callers for that method, it
looks best if we cancel the pending RFH. I don't know if there is a specific
reason to the ordering of those calls.

[alexmos, 2015/05/14 22:54:15]

I added the early return and moved CancelPending to be done before process init.
 I double-checked with Charlie, and he didn't see any particular reason why
CancelPending should be done after CreateOpenerRenderViewsIfNeeded.  Charlie
pointed out that it was introduced by carlosk in
https://codereview.chromium.org/753173002/diff/80001/content/browser/frame_ho...,
who was probably just trying to put it above CreateRenderFrame.

+
   int opener_route_id = CreateOpenerRenderViewsIfNeeded(
       old_instance, new_instance, &create_render_frame_flags);
 
   if (pending_render_frame_host_)
     CancelPending();
 
   // Create a non-swapped-out RFH with the given opener.
   pending_render_frame_host_ =
       CreateRenderFrame(new_instance, pending_web_ui(), opener_route_id,
                         create_render_frame_flags, nullptr);
@@ skipping 69 matching lines @@
     int bindings) {
   CHECK(new_instance);
   CHECK_NE(old_instance, new_instance);
   CHECK(!should_reuse_web_ui_);
 
   // Note: |speculative_web_ui_| must be initialized before starting the
   // |speculative_render_frame_host_| creation steps otherwise the WebUI
   // 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 old 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,
                                       &create_render_frame_flags);
 
   if (frame_tree_node_->IsMainFrame())
     create_render_frame_flags |= CREATE_RF_FOR_MAIN_FRAME_NAVIGATION;
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
   speculative_render_frame_host_ =
@@ skipping 670 matching lines @@
 void RenderFrameHostManager::DeleteRenderFrameProxyHost(
     SiteInstance* instance) {
   RenderFrameProxyHostMap::iterator iter = proxy_hosts_.find(instance->GetId());
   if (iter != proxy_hosts_.end()) {
     delete iter->second;
     proxy_hosts_.erase(iter);
   }
 }
 
 }  // namespace content