Introduce RenderFrameProxyHost object and use it in RFHM.

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 b9c5be36a75c1c6896c7a7e1ebe7793720f0eceb..fc30688085672501a9c9cf1d8154aec2c36c976b 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -20,6 +20,7 @@
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
@@ -89,11 +90,9 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   // We should always have a current RenderFrameHost except in some tests.
   render_frame_host_.reset();
 
-  // TODO(creis): Now that we aren't using Shutdown, make RenderFrameHostMap
-  // use scoped_ptrs.
   // Delete any swapped out RenderFrameHosts.
-  for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin();
-       iter != swapped_out_hosts_.end();
+  for (RenderFrameHostMap::iterator iter = proxy_hosts_.begin();
+       iter != proxy_hosts_.end();
        ++iter) {
     delete iter->second;
   }
@@ -109,9 +108,11 @@ void RenderFrameHostManager::Init(BrowserContext* browser_context,
   if (!site_instance)
     site_instance = SiteInstance::Create(browser_context);
 
-  render_frame_host_ = make_scoped_ptr(
-      CreateRenderFrameHost(site_instance, view_routing_id, frame_routing_id,
-                            false, delegate_->IsHidden()));
+  render_frame_host_ = CreateRenderFrameHost(site_instance,
+                                             view_routing_id,
+                                             frame_routing_id,
+                                             false,
+                                             delegate_->IsHidden());
 
   // Keep track of renderer processes as they start to shut down or are
   // crashed/killed.
@@ -447,10 +448,10 @@ void RenderFrameHostManager::DidNavigateFrame(
 // TODO(creis): Take in RenderFrameHost instead, since frames can have openers.
 void RenderFrameHostManager::DidDisownOpener(RenderViewHost* render_view_host) {
   // Notify all swapped out hosts, including the pending RVH.
-  for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin();
-       iter != swapped_out_hosts_.end();
+  for (RenderFrameHostMap::iterator iter = proxy_hosts_.begin();
+       iter != proxy_hosts_.end();
        ++iter) {
-    DCHECK_NE(iter->second->GetSiteInstance(),
+    DCHECK_NE(iter->second->render_frame_host()->GetSiteInstance(),
               current_frame_host()->GetSiteInstance());
     iter->second->render_view_host()->DisownOpener();
   }
@@ -462,8 +463,8 @@ void RenderFrameHostManager::RendererProcessClosing(
   // swap them back in while the process is exiting.  Start by finding them,
   // since there could be more than one.
   std::list<int> ids_to_remove;
-  for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin();
-       iter != swapped_out_hosts_.end();
+  for (RenderFrameHostMap::iterator iter = proxy_hosts_.begin();
+       iter != proxy_hosts_.end();
        ++iter) {
     if (iter->second->GetProcess() == render_process_host)
       ids_to_remove.push_back(iter->first);
@@ -471,8 +472,8 @@ void RenderFrameHostManager::RendererProcessClosing(
 
   // Now delete them.
   while (!ids_to_remove.empty()) {
-    delete swapped_out_hosts_[ids_to_remove.back()];
-    swapped_out_hosts_.erase(ids_to_remove.back());
+    delete proxy_hosts_[ids_to_remove.back()];
+    proxy_hosts_.erase(ids_to_remove.back());
     ids_to_remove.pop_back();
   }
 }
@@ -548,34 +549,36 @@ void RenderFrameHostManager::Observe(
   }
 }
 
-bool RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance(
+bool RenderFrameHostManager::ClearProxiesInSiteInstance(
     int32 site_instance_id,
     FrameTreeNode* node) {
   RenderFrameHostMap::iterator iter =
-      node->render_manager()->swapped_out_hosts_.find(site_instance_id);
-  if (iter != node->render_manager()->swapped_out_hosts_.end()) {
-    RenderFrameHostImpl* swapped_out_rfh = iter->second;
+      node->render_manager()->proxy_hosts_.find(site_instance_id);
+  if (iter != node->render_manager()->proxy_hosts_.end()) {
+    RenderFrameProxyHost* proxy = iter->second;
     // If the RVH is pending swap out, it needs to switch state to
     // pending shutdown. Otherwise it is deleted.
-    if (swapped_out_rfh->render_view_host()->rvh_state() ==
+    if (proxy->render_view_host()->rvh_state() ==
         RenderViewHostImpl::STATE_PENDING_SWAP_OUT) {
-      swapped_out_rfh->SetPendingShutdown(base::Bind(
+      scoped_ptr<RenderFrameHostImpl> rfh = proxy->PassFrameHost();
+
+      rfh->SetPendingShutdown(base::Bind(
           &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance,
           node->render_manager()->weak_factory_.GetWeakPtr(),
           site_instance_id,
-          swapped_out_rfh));
+          rfh.get()));
       RFHPendingDeleteMap::iterator pending_delete_iter =
           node->render_manager()->pending_delete_hosts_.find(site_instance_id);
       if (pending_delete_iter ==
               node->render_manager()->pending_delete_hosts_.end() ||
-          pending_delete_iter->second.get() != iter->second) {
+          pending_delete_iter->second.get() != rfh) {
         node->render_manager()->pending_delete_hosts_[site_instance_id] =
-            linked_ptr<RenderFrameHostImpl>(swapped_out_rfh);
+            linked_ptr<RenderFrameHostImpl>(rfh.release());
       }
     } else {
-      delete swapped_out_rfh;
+      delete proxy;
     }
-    node->render_manager()->swapped_out_hosts_.erase(site_instance_id);
+    node->render_manager()->proxy_hosts_.erase(site_instance_id);
   }
 
   return true;
@@ -833,7 +836,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
   return current_instance->GetRelatedSiteInstance(dest_url);
 }
 
-RenderFrameHostImpl* RenderFrameHostManager::CreateRenderFrameHost(
+scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
     SiteInstance* site_instance,
     int view_routing_id,
     int frame_routing_id,
@@ -862,16 +865,15 @@ RenderFrameHostImpl* RenderFrameHostManager::CreateRenderFrameHost(
     }
   }
 
-  // TODO(creis): Make render_frame_host a scoped_ptr.
   // TODO(creis): Pass hidden to RFH.
-  RenderFrameHostImpl* render_frame_host =
-      RenderFrameHostFactory::Create(render_view_host,
-                                     render_frame_delegate_,
-                                     frame_tree,
-                                     frame_tree_node_,
-                                     frame_routing_id,
-                                     swapped_out).release();
-  return render_frame_host;
+  scoped_ptr<RenderFrameHostImpl> render_frame_host =
+      make_scoped_ptr(RenderFrameHostFactory::Create(render_view_host,
+                                                     render_frame_delegate_,
+                                                     frame_tree,
+                                                     frame_tree_node_,
+                                                     frame_routing_id,
+                                                     swapped_out).release());
+  return render_frame_host.Pass();
 }
 
 int RenderFrameHostManager::CreateRenderFrame(
@@ -882,6 +884,9 @@ int RenderFrameHostManager::CreateRenderFrame(
   CHECK(instance);
   DCHECK(!swapped_out || hidden); // Swapped out views should always be hidden.
 
+  scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
+  int routing_id = MSG_ROUTING_NONE;
+
   // We are creating a pending or swapped out RFH here.  We should never create
   // it in the same SiteInstance as our current RFH.
   CHECK_NE(render_frame_host_->GetSiteInstance(), instance);
@@ -889,17 +894,22 @@ int RenderFrameHostManager::CreateRenderFrame(
   // Check if we've already created an RFH for this SiteInstance.  If so, try
   // to re-use the existing one, which has already been initialized.  We'll
   // remove it from the list of swapped out hosts if it commits.
-  RenderFrameHostImpl* new_render_frame_host =
-      GetSwappedOutRenderFrameHost(instance);
+  RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
 
   FrameTreeNode* parent_node = NULL;
   if (frame_tree_node_)
     parent_node = frame_tree_node_->parent();
 
-  if (new_render_frame_host) {
+  if (proxy) {
+    routing_id = proxy->render_view_host()->GetRoutingID();
+    // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
     // Prevent the process from exiting while we're trying to use it.
     if (!swapped_out) {
+      new_render_frame_host = proxy->PassFrameHost();
       new_render_frame_host->GetProcess()->AddPendingView();
+
+      proxy_hosts_.erase(instance->GetId());
+      delete proxy;
     } else {
       // Detect if this is a cross-process child frame that is navigating
       // back to the same SiteInstance as its parent.
@@ -912,21 +922,20 @@ int RenderFrameHostManager::CreateRenderFrame(
     }
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
-    // TODO(creis): Make new_render_frame_host a scoped_ptr.
-    new_render_frame_host = CreateRenderFrameHost(instance, MSG_ROUTING_NONE,
-                                                  MSG_ROUTING_NONE, swapped_out,
-                                                  hidden);
+    new_render_frame_host = CreateRenderFrameHost(
+        instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, swapped_out, hidden);
+    RenderViewHostImpl* render_view_host =
+        new_render_frame_host->render_view_host();
 
     // If the new RFH is swapped out already, store it.  Otherwise prevent the
     // process from exiting while we're trying to navigate in it.
-    if (swapped_out) {
-      swapped_out_hosts_[instance->GetId()] = new_render_frame_host;
-    } else {
+    if (!swapped_out) {
       new_render_frame_host->GetProcess()->AddPendingView();
+    } else {
+      proxy_hosts_[instance->GetId()] = new RenderFrameProxyHost(
+          new_render_frame_host.Pass());
     }
 
-    RenderViewHostImpl* render_view_host =
-        new_render_frame_host->render_view_host();
     bool success = InitRenderView(render_view_host, opener_route_id);
     if (success && frame_tree_node_->IsMainFrame()) {
       // Don't show the main frame's view until we get a DidNavigate from it.
@@ -934,13 +943,15 @@ int RenderFrameHostManager::CreateRenderFrame(
     } else if (!swapped_out && pending_render_frame_host_) {
       CancelPending();
     }
+    routing_id = render_view_host->GetRoutingID();
   }
 
+

[Charlie Reis, 2014/03/31 23:22:35]

nit: extra line

[nasko, 2014/04/09 17:52:01]

Done.

   // Use this as our new pending RFH if it isn't swapped out.
   if (!swapped_out)
-    pending_render_frame_host_.reset(new_render_frame_host);
+    pending_render_frame_host_ = new_render_frame_host.Pass();
 
-  return new_render_frame_host->render_view_host()->GetRoutingID();
+  return routing_id;
 }
 
 bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
@@ -1017,7 +1028,8 @@ void RenderFrameHostManager::CommitPending() {
 
   // Swap in the pending frame and make it active. Also ensure the FrameTree
   // stays in sync.
-  RenderFrameHostImpl* old_render_frame_host = render_frame_host_.release();
+  scoped_ptr<RenderFrameHostImpl> old_render_frame_host =
+      render_frame_host_.Pass();
   render_frame_host_ = pending_render_frame_host_.Pass();
   if (is_main_frame)
     render_frame_host_->render_view_host()->AttachToFrameTree();
@@ -1046,11 +1058,10 @@ void RenderFrameHostManager::CommitPending() {
           &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance,
           weak_factory_.GetWeakPtr(),
           old_site_instance_id,
-          old_render_frame_host));
-    } else {
-      // TODO(creis): We'll need to set this back to false if we navigate back.
-      old_render_frame_host->set_swapped_out(true);
+          old_render_frame_host.get()));
     }
+    // TODO(creis): We'll need to set this back to false if we navigate back.
+    old_render_frame_host->set_swapped_out(true);
   }
 
   // Make sure the size is up to date.  (Fix for bug 1079768.)
@@ -1075,7 +1086,7 @@ void RenderFrameHostManager::CommitPending() {
   }
 
   // If the pending frame was on the swapped out list, we can remove it.
-  swapped_out_hosts_.erase(render_frame_host_->GetSiteInstance()->GetId());
+  proxy_hosts_.erase(render_frame_host_->GetSiteInstance()->GetId());
 
   if (old_render_frame_host->render_view_host()->IsRenderViewLive()) {
     // If the old RFH is live, we are swapping it out and should keep track of
@@ -1090,55 +1101,56 @@ void RenderFrameHostManager::CommitPending() {
     // sure we don't get different rvh instances for the same site instance
     // in the same rvhmgr.
     // TODO(creis): Clean this up.
-    RenderFrameHostMap::iterator iter =
-        swapped_out_hosts_.find(old_site_instance_id);
-    if (iter != swapped_out_hosts_.end() &&
-        iter->second != old_render_frame_host) {
-      // Delete the RFH that will be replaced in the map to avoid a leak.
+    RenderFrameHostMap::iterator iter = proxy_hosts_.find(old_site_instance_id);
+    if (iter != proxy_hosts_.end() &&
+        iter->second->render_frame_host() != old_render_frame_host) {
+      // Delete the proxy that will be replaced in the map to avoid a leak.
       delete iter->second;
     }
+
+    SiteInstanceImpl* old_site_instance =
+      static_cast<SiteInstanceImpl*>(old_render_frame_host->GetSiteInstance());

[Charlie Reis, 2014/03/31 23:22:35]

nit: Wrong indent.

[nasko, 2014/04/09 17:52:01]

Done.

+
     // If the RenderViewHost backing the RenderFrameHost is pending shutdown,
     // the RenderFrameHost should be put in the map of RenderFrameHosts pending
-    // shutdown. Otherwise, it is stored in the map of swapped out
-    // RenderFrameHosts.
+    // shutdown. Otherwise, it is stored in the map of proxy hosts.
     if (old_render_frame_host->render_view_host()->rvh_state() ==
             RenderViewHostImpl::STATE_PENDING_SHUTDOWN) {
-      swapped_out_hosts_.erase(old_site_instance_id);
+      proxy_hosts_.erase(old_site_instance_id);
       RFHPendingDeleteMap::iterator pending_delete_iter =
           pending_delete_hosts_.find(old_site_instance_id);
       if (pending_delete_iter == pending_delete_hosts_.end() ||
           pending_delete_iter->second.get() != old_render_frame_host) {
         pending_delete_hosts_[old_site_instance_id] =
-            linked_ptr<RenderFrameHostImpl>(old_render_frame_host);
+            linked_ptr<RenderFrameHostImpl>(old_render_frame_host.release());
       }
     } else {
-      swapped_out_hosts_[old_site_instance_id] = old_render_frame_host;
-    }
-
-    // If there are no active views in this SiteInstance, it means that
-    // this RFH was the last active one in the SiteInstance. Now that we
-    // know that all RFHs are swapped out, we can delete all the RFHs and RVHs
-    // in this SiteInstance.  We do this after ensuring the RFH is on the
-    // swapped out list to simplify the deletion.
-    if (!static_cast<SiteInstanceImpl*>(
-            old_render_frame_host->GetSiteInstance())->active_view_count()) {
-      ShutdownRenderFrameHostsInSiteInstance(old_site_instance_id);
-      // This is deleted while cleaning up the SiteInstance's views.
-      old_render_frame_host = NULL;
+      // If there are no active views in this SiteInstance, it means that
+      // this RFH was the last active one in the SiteInstance. Now that we
+      // know that all RFHs are swapped out, we can delete all the RFHs and RVHs
+      // in this SiteInstance.  We do this after ensuring the RFH is on the
+      // swapped out list to simplify the deletion.
+      if (!old_site_instance->active_view_count()) {
+        old_render_frame_host.reset(NULL);
+        ShutdownRenderFrameHostsInSiteInstance(old_site_instance_id);
+      } else {
+        proxy_hosts_[old_site_instance_id] = new RenderFrameProxyHost(
+            old_render_frame_host.Pass());
+      }
     }
   } else {
-    delete old_render_frame_host;
+    old_render_frame_host.reset(NULL);
   }
 }
 
 void RenderFrameHostManager::ShutdownRenderFrameHostsInSiteInstance(
     int32 site_instance_id) {
   // First remove any swapped out RFH for this SiteInstance from our own list.
-  ClearSwappedOutRFHsInSiteInstance(site_instance_id, frame_tree_node_);
+  ClearProxiesInSiteInstance(site_instance_id, frame_tree_node_);
 
   // Use the safe RenderWidgetHost iterator for now to find all RenderViewHosts
   // in the SiteInstance, then tell their respective FrameTrees to remove all
-  // swapped out RenderFrameHosts corresponding to them.
+  // RenderFrameProxyHosts corresponding to them.
   // TODO(creis): Replace this with a RenderFrameHostIterator that protects
   // against use-after-frees if a later element is deleted before getting to it.
   scoped_ptr<RenderWidgetHostIterator> widgets(
@@ -1153,7 +1165,7 @@ void RenderFrameHostManager::ShutdownRenderFrameHostsInSiteInstance(
       // |rvh| to Shutdown.
       FrameTree* tree = rvh->GetDelegate()->GetFrameTree();
       tree->ForEach(base::Bind(
-          &RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance,
+          &RenderFrameHostManager::ClearProxiesInSiteInstance,
           site_instance_id));
     }
   }
@@ -1335,7 +1347,7 @@ void RenderFrameHostManager::CancelPending() {
   // swap it back in and then canceled.  If so, make sure it gets swapped out
   // again.  If it's not on the swapped out list (e.g., aborting a pending
   // load), then it's safe to shut down.
-  if (IsOnSwappedOutList(pending_render_frame_host)) {
+  if (!pending_render_frame_host->is_created_for_pending()) {
     // Any currently suspended navigations are no longer needed.
     pending_render_frame_host->render_view_host()->CancelSuspendedNavigations();
 
@@ -1369,11 +1381,11 @@ void RenderFrameHostManager::RenderViewDeleted(RenderViewHost* rvh) {
     return;
 
   // We can't look it up by SiteInstance ID, which may no longer be valid.
-  for (RenderFrameHostMap::iterator iter = swapped_out_hosts_.begin();
-       iter != swapped_out_hosts_.end();
+  for (RenderFrameHostMap::iterator iter = proxy_hosts_.begin();
+       iter != proxy_hosts_.end();
        ++iter) {
     if (iter->second->render_view_host() == rvh) {
-      swapped_out_hosts_.erase(iter);
+      proxy_hosts_.erase(iter);
       break;
     }
   }
@@ -1381,11 +1393,11 @@ void RenderFrameHostManager::RenderViewDeleted(RenderViewHost* rvh) {
 
 bool RenderFrameHostManager::IsRVHOnSwappedOutList(
     RenderViewHostImpl* rvh) const {
-  RenderFrameHostImpl* render_frame_host = GetSwappedOutRenderFrameHost(
+  RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(
       rvh->GetSiteInstance());
-  if (!render_frame_host)
+  if (!proxy)
     return false;
-  return IsOnSwappedOutList(render_frame_host);
+  return IsOnSwappedOutList(proxy->render_frame_host());
 }
 
 bool RenderFrameHostManager::IsOnSwappedOutList(
@@ -1393,28 +1405,27 @@ bool RenderFrameHostManager::IsOnSwappedOutList(
   if (!rfh->GetSiteInstance())
     return false;
 
-  RenderFrameHostMap::const_iterator iter = swapped_out_hosts_.find(
+  RenderFrameHostMap::const_iterator iter = proxy_hosts_.find(
       rfh->GetSiteInstance()->GetId());
-  if (iter == swapped_out_hosts_.end())
+  if (iter == proxy_hosts_.end())
     return false;
 
-  return iter->second == rfh;
+  return iter->second->render_frame_host() == rfh;
 }
 
 RenderViewHostImpl* RenderFrameHostManager::GetSwappedOutRenderViewHost(
    SiteInstance* instance) const {
-  RenderFrameHostImpl* render_frame_host =
-      GetSwappedOutRenderFrameHost(instance);
-  if (render_frame_host)
-    return render_frame_host->render_view_host();
+  RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
+  if (proxy)
+    return proxy->render_view_host();
   return NULL;
 }
 
-RenderFrameHostImpl* RenderFrameHostManager::GetSwappedOutRenderFrameHost(
+RenderFrameProxyHost* RenderFrameHostManager::GetRenderFrameProxyHost(
     SiteInstance* instance) const {
   RenderFrameHostMap::const_iterator iter =
-      swapped_out_hosts_.find(instance->GetId());
-  if (iter != swapped_out_hosts_.end())
+      proxy_hosts_.find(instance->GetId());
+  if (iter != proxy_hosts_.end())
     return iter->second;
 
   return NULL;