Don't crash while detaching a pending child frame under --site-per-process.

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 807a450985156f6c5d4c97e5f611d53bb71f983c..c395b76d43e886f558e0da4230207a268477e76f 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -67,7 +67,7 @@ RenderFrameHostManager::RenderFrameHostManager(
 
 RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
-    CancelPending();
+    UnsetPendingRenderFrameHost();
 
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
@@ -564,30 +564,6 @@ void RenderFrameHostManager::SwapOutOldFrame(
   }
 }
 
-void RenderFrameHostManager::DiscardUnusedFrame(

[nasko, 2014/12/12 21:54:49]

In a parallel CL (https://codereview.chromium.org/701953006/) carlosk@ is
working on, he is using this method in another place. It will be nice to keep
this method around instead of inlining it back into CancelPending (from which it
was factored out).

[ncarter (slow), 2014/12/12 22:33:24]

Done. I inlined it in an attempt to control the proliferation of "delete frame"
private methods. Thanks for catching this.

Incidentally, the CL you linked to would introduce a new instance of this bug
(it adds a new call to DiscardUnusedFrame in the dtor). I'll add a comment
there.

-    scoped_ptr<RenderFrameHostImpl> render_frame_host) {
-  // TODO(carlosk): this code is very similar to what can be found in
-  // SwapOutOldFrame and we should see that these are unified at some point.
-
-  // If the SiteInstance for the pending RFH is being used by others don't
-  // delete the RFH. Just swap it out and it can be reused at a later point.
-  SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
-  if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
-    // Any currently suspended navigations are no longer needed.
-    render_frame_host->CancelSuspendedNavigations();
-
-    RenderFrameProxyHost* proxy =
-        new RenderFrameProxyHost(site_instance, frame_tree_node_);
-    proxy_hosts_[site_instance->GetId()] = proxy;
-    render_frame_host->SwapOut(proxy);
-    if (frame_tree_node_->IsMainFrame())
-      proxy->TakeFrameHostOwnership(render_frame_host.Pass());
-  } else {
-    // We won't be coming back, so delete this one.
-    render_frame_host.reset();
-  }
-}
-
 void RenderFrameHostManager::MoveToPendingDeleteHosts(
     scoped_ptr<RenderFrameHostImpl> render_frame_host) {
   // |render_frame_host| will be deleted when its SwapOut ACK is received, or
@@ -1591,6 +1567,34 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 void RenderFrameHostManager::CancelPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CancelPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
+
+  scoped_ptr<RenderFrameHostImpl> render_frame_host =
+      UnsetPendingRenderFrameHost();
+
+  // TODO(carlosk): this code is very similar to what can be found in
+  // SwapOutOldFrame and we should see that these are unified at some point.
+
+  // If the SiteInstance for the pending RFH is being used by others don't
+  // delete the RFH. Just swap it out and it can be reused at a later point.
+  SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
+  if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
+    // Any currently suspended navigations are no longer needed.
+    render_frame_host->CancelSuspendedNavigations();
+
+    RenderFrameProxyHost* proxy =
+        new RenderFrameProxyHost(site_instance, frame_tree_node_);
+    proxy_hosts_[site_instance->GetId()] = proxy;
+    render_frame_host->SwapOut(proxy);
+    if (frame_tree_node_->IsMainFrame())
+      proxy->TakeFrameHostOwnership(render_frame_host.Pass());
+  } else {
+    // We won't be coming back, so delete this one.
+    render_frame_host.reset();
+  }
+}
+
+scoped_ptr<RenderFrameHostImpl>
+RenderFrameHostManager::UnsetPendingRenderFrameHost() {
   scoped_ptr<RenderFrameHostImpl> pending_render_frame_host =
       pending_render_frame_host_.Pass();
 
@@ -1601,10 +1605,10 @@ void RenderFrameHostManager::CancelPending() {
   // We no longer need to prevent the process from exiting.
   pending_render_frame_host->GetProcess()->RemovePendingView();
 
-  DiscardUnusedFrame(pending_render_frame_host.Pass());
-
   pending_web_ui_.reset();
   pending_and_current_web_ui_.reset();
+
+  return pending_render_frame_host.Pass();
 }
 
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost(