Create RenderFrameProxyHost at time of swap-out instead of commit.

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/debug/trace_event.h"
@@ skipping 157 matching lines @@
     return NULL;  // We weren't able to create a pending render frame host.
 
   // If the current render_frame_host_ isn't live, we should create it so
   // that we don't show a sad tab while the dest_render_frame_host fetches
   // its first page.  (Bug 1145340)
   if (dest_render_frame_host != render_frame_host_ &&
       !render_frame_host_->render_view_host()->IsRenderViewLive()) {
     // Note: we don't call InitRenderView here because we are navigating away
     // soon anyway, and we don't have the NavigationEntry for this host.
     delegate_->CreateRenderViewForRenderManager(
-        render_frame_host_->render_view_host(), MSG_ROUTING_NONE, NULL);
+        render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
+        MSG_ROUTING_NONE, NULL);
   }
 
   // If the renderer crashed, then try to create a new one to satisfy this
   // navigation request.
   if (!dest_render_frame_host->render_view_host()->IsRenderViewLive()) {
     // Recreate the opener chain.
     int opener_route_id = delegate_->CreateOpenerRenderViewsForRenderManager(
         dest_render_frame_host->GetSiteInstance());
     if (!InitRenderView(dest_render_frame_host->render_view_host(),
-                        opener_route_id))
+                        opener_route_id, MSG_ROUTING_NONE))
       return NULL;
 
     // Now that we've created a new renderer, be sure to hide it if it isn't
     // our primary one.  Otherwise, we might crash if we try to call Show()
     // on it later.
     if (dest_render_frame_host != render_frame_host_ &&
         dest_render_frame_host->render_view_host()->GetView()) {
       dest_render_frame_host->render_view_host()->GetView()->Hide();
     } else if (frame_tree_node_->IsMainFrame()) {
       // This is our primary renderer, notify here as we won't be calling
@@ skipping 300 matching lines @@
     // The same CrossProcessFrameConnector is used for subsequent cross-
     // process navigations, but it will be destroyed if the Frame is
     // navigated back to the same site instance as its parent.
     // TODO(kenrb): This will change when RenderFrameProxyHost is created.
     if (!cross_process_frame_connector_) {
       cross_process_frame_connector_ =
           new CrossProcessFrameConnector(render_frame_host_.get());
     }
   }
 
+  // Create the RenderFrameProxyHost that will replace the
+  // RenderFrameHost which is swapping out. If one exists, ensure it is deleted
+  // from the map and not leaked.
+  RenderFrameProxyHostMap::iterator iter = proxy_hosts_.find(
+      render_frame_host_->GetSiteInstance()->GetId());
+  if (iter != proxy_hosts_.end()) {
+    delete iter->second;
+    proxy_hosts_.erase(iter);
+  }
+
+  RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
+     render_frame_host_->GetSiteInstance(), frame_tree_node_);

[Charlie Reis, 2014/05/15 22:55:00]

nit: Wrong indent.

[nasko, 2014/05/15 23:36:49]

Done.

+  proxy_hosts_[render_frame_host_->GetSiteInstance()->GetId()] = proxy;
+
   // Tell the old frame it is being swapped out.  This will fire the unload
   // handler in the background (without firing the beforeunload handler a second
   // time).  When the navigation completes, we will send a message to the
   // ResourceDispatcherHost, allowing the pending RVH's response to resume.
-  render_frame_host_->SwapOut();
+  render_frame_host_->SwapOut(proxy);
 
   // ResourceDispatcherHost has told us to run the onunload handler, which
   // means it is not a download or unsafe page, and we are going to perform the
   // navigation.  Thus, we no longer need to remember that the RenderFrameHost
   // is part of a pending cross-site request.
   if (pending_render_frame_host_) {
     pending_render_frame_host_->render_view_host()->
         SetHasPendingCrossSiteRequest(false);
   }
 }
@@ skipping 27 matching lines @@
     int32 site_instance_id,
     FrameTreeNode* node) {
   RenderFrameProxyHostMap::iterator iter =
       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 (proxy->render_view_host()->rvh_state() ==
         RenderViewHostImpl::STATE_PENDING_SWAP_OUT) {
-      scoped_ptr<RenderFrameHostImpl> swapped_out_rfh = proxy->PassFrameHost();
+      scoped_ptr<RenderFrameHostImpl> swapped_out_rfh =
+          proxy->PassFrameHostOwnership();
 
       swapped_out_rfh->SetPendingShutdown(base::Bind(
           &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance,
           node->render_manager()->weak_factory_.GetWeakPtr(),
           site_instance_id,
           swapped_out_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() ||
@@ skipping 322 matching lines @@
   // remove it from the list of swapped out hosts if it commits.
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
 
   FrameTreeNode* parent_node = frame_tree_node_->parent();
 
   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 = proxy->PassFrameHostOwnership();
       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.
       if (parent_node && cross_process_frame_connector_ &&
           render_frame_host_->GetSiteInstance() == parent_node->
               render_manager()->current_frame_host()->GetSiteInstance()) {
         delete cross_process_frame_connector_;
         cross_process_frame_connector_ = NULL;
       }
     }
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
     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();
+    int proxy_routing_id = MSG_ROUTING_NONE;
 
     // Prevent the process from exiting while we're trying to navigate in it.
     // Otherwise, if the new RFH is swapped out already, store it.
     if (!swapped_out) {
       new_render_frame_host->GetProcess()->AddPendingView();
     } else {
-      proxy_hosts_[instance->GetId()] = new RenderFrameProxyHost(
-          new_render_frame_host.Pass());
+      proxy = new RenderFrameProxyHost(
+          new_render_frame_host->GetSiteInstance(), frame_tree_node_);
+      proxy_hosts_[instance->GetId()] = proxy;
+      proxy->TakeFrameHostOwnership(new_render_frame_host.Pass());
+      proxy_routing_id = proxy->GetRoutingID();
     }
 
-    bool success = InitRenderView(render_view_host, opener_route_id);
+    bool success = InitRenderView(
+        render_view_host, opener_route_id, proxy_routing_id);
     if (success && frame_tree_node_->IsMainFrame()) {
       // Don't show the main frame's view until we get a DidNavigate from it.
       render_view_host->GetView()->Hide();
     } else if (!swapped_out && pending_render_frame_host_) {
       CancelPending();
     }
     routing_id = render_view_host->GetRoutingID();
   }
 
   // Use this as our new pending RFH if it isn't swapped out.
   if (!swapped_out)
     pending_render_frame_host_ = new_render_frame_host.Pass();
 
   return routing_id;
 }
 
 bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
-                                            int opener_route_id) {
+                                            int opener_route_id,
+                                            int proxy_routing_id) {
   // We may have initialized this RenderViewHost for another RenderFrameHost.
   if (render_view_host->IsRenderViewLive())
     return true;
 
   // If the pending navigation is to a WebUI and the RenderView is not in a
   // guest process, tell the RenderViewHost about any bindings it will need
   // enabled.
   if (pending_web_ui() && !render_view_host->GetProcess()->IsGuest()) {
     render_view_host->AllowBindings(pending_web_ui()->GetBindings());
   } else {
     // Ensure that we don't create an unprivileged RenderView in a WebUI-enabled
     // process unless it's swapped out.
     RenderViewHostImpl* rvh_impl =
         static_cast<RenderViewHostImpl*>(render_view_host);
     if (!rvh_impl->IsSwappedOut()) {
       CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
                 render_view_host->GetProcess()->GetID()));
     }
   }
 
   return delegate_->CreateRenderViewForRenderManager(
-      render_view_host, opener_route_id, cross_process_frame_connector_);
+      render_view_host, opener_route_id, proxy_routing_id,
+      cross_process_frame_connector_);
 }
 
 void RenderFrameHostManager::CommitPending() {
   // First check whether we're going to want to focus the location bar after
   // this commit.  We do this now because the navigation hasn't formally
   // committed yet, so if we've already cleared |pending_web_ui_| the call chain
   // this triggers won't be able to figure out what's going on.
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
   // We expect SwapOutOldPage to have canceled any modal dialogs and told the
@@ skipping 100 matching lines @@
   }
 
   // If the old RFH is live, we are swapping it out and should keep track of
   // it in case we navigate back to it, or it is waiting for the unload event
   // to execute in the background.
   // TODO(creis): Swap out the subframe in --site-per-process.
   if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess))
     DCHECK(old_render_frame_host->is_swapped_out() ||
            !RenderViewHostImpl::IsRVHStateActive(
                old_render_frame_host->render_view_host()->rvh_state()));
-  // Temp fix for http://crbug.com/90867 until we do a better cleanup to make
-  // sure we don't get different rvh instances for the same site instance
-  // in the same rvhmgr.
-  // TODO(creis): Clean this up.
-  RenderFrameProxyHostMap::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;
-  }
 
   // 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 proxy hosts.
   if (old_render_frame_host->render_view_host()->rvh_state() ==
           RenderViewHostImpl::STATE_PENDING_SHUTDOWN) {
-    proxy_hosts_.erase(old_site_instance_id);
+    // The proxy for this RenderFrameHost is created when sending the
+    // SwapOut message, so check if it already exists and delete it.
+    RenderFrameProxyHostMap::iterator iter =
+        proxy_hosts_.find(old_site_instance_id);
+    if (iter != proxy_hosts_.end()) {
+      delete iter->second;
+      proxy_hosts_.erase(iter);
+    }
     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.release());
     }
   } else {
+    // Capture the active view count on the old RFH SiteInstance, since the
+    // ownership will be passed into the proxy and the pointer will be invalid.
+    int active_view_count =
+        static_cast<SiteInstanceImpl*>(old_render_frame_host->GetSiteInstance())
+            ->active_view_count();
+
+    RenderFrameProxyHostMap::iterator iter =
+        proxy_hosts_.find(old_site_instance_id);
+    CHECK(iter != proxy_hosts_.end());
+    iter->second->TakeFrameHostOwnership(old_render_frame_host.Pass());
+
     // 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()) {
-      old_render_frame_host.reset();
+    // in this SiteInstance.
+    if (!active_view_count)
       ShutdownRenderFrameHostsInSiteInstance(old_site_instance_id);
-    } else {
-      proxy_hosts_[old_site_instance_id] = new RenderFrameProxyHost(
-          old_render_frame_host.Pass());
-    }
   }
 }
 
 void RenderFrameHostManager::ShutdownRenderFrameHostsInSiteInstance(
     int32 site_instance_id) {
   // First remove any swapped out RFH for this SiteInstance from our own list.
   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
@@ skipping 191 matching lines @@
   pending_render_frame_host->GetProcess()->RemovePendingView();
 
   // 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 = static_cast<SiteInstanceImpl*>(
       pending_render_frame_host->GetSiteInstance());
   if (site_instance->active_view_count() > 1) {
     // Any currently suspended navigations are no longer needed.
     pending_render_frame_host->render_view_host()->CancelSuspendedNavigations();
 
-    pending_render_frame_host->SwapOut();
-
-    proxy_hosts_[site_instance->GetId()] = new RenderFrameProxyHost(
-        pending_render_frame_host.Pass());
+    RenderFrameProxyHost* proxy =
+        new RenderFrameProxyHost(site_instance, frame_tree_node_);
+    proxy_hosts_[site_instance->GetId()] = proxy;
+    pending_render_frame_host->SwapOut(proxy);
+    proxy->TakeFrameHostOwnership(pending_render_frame_host.Pass());
   } else {
     // We won't be coming back, so delete this one.
     pending_render_frame_host.reset();
   }
 
   pending_web_ui_.reset();
   pending_and_current_web_ui_.reset();
 }
 
 bool RenderFrameHostManager::IsRVHOnSwappedOutList(
@@ skipping 30 matching lines @@
     SiteInstance* instance) const {
   RenderFrameProxyHostMap::const_iterator iter =
       proxy_hosts_.find(instance->GetId());
   if (iter != proxy_hosts_.end())
     return iter->second;
 
   return NULL;
 }
 
 }  // namespace content