Don't leave a RenderFrameProxyHost for the current RenderFrameHost.

This could happen in cases where we initiate a transfer but end up navigating
in the original SiteInstance.

BUG=402018
TEST=No proxy host for the current RFH after navigating an about:blank tab.

Review URL: https://codereview.chromium.org/452673002

Cr-Commit-Position: refs/heads/master@{#288406}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288406 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/frame_host/frame_tree_browsertest.cc

@@ -187,6 +187,10 @@ IN_PROC_BROWSER_TEST_F(CrossProcessFrameTreeBrowserTest,
   FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
                             ->GetFrameTree()->root();
 
+  // There should not be a proxy for the root's own SiteInstance.
+  SiteInstance* root_instance = root->current_frame_host()->GetSiteInstance();
+  EXPECT_FALSE(root->render_manager()->GetRenderFrameProxyHost(root_instance));
+
   // Load same-site page into iframe.
   GURL http_url(test_server()->GetURL("files/title1.html"));
   NavigateFrameToURL(root->child_at(0), http_url);
@@ -204,20 +208,24 @@ IN_PROC_BROWSER_TEST_F(CrossProcessFrameTreeBrowserTest,
   // Ensure that we have created a new process for the subframe.
   ASSERT_EQ(1U, root->child_count());
   FrameTreeNode* child = root->child_at(0);
-  SiteInstance* site_instance = child->current_frame_host()->GetSiteInstance();
+  SiteInstance* child_instance = child->current_frame_host()->GetSiteInstance();
   RenderViewHost* rvh = child->current_frame_host()->render_view_host();
   RenderProcessHost* rph = child->current_frame_host()->GetProcess();
 
   EXPECT_NE(shell()->web_contents()->GetRenderViewHost(), rvh);
-  EXPECT_NE(shell()->web_contents()->GetSiteInstance(), site_instance);
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(), child_instance);
   EXPECT_NE(shell()->web_contents()->GetRenderProcessHost(), rph);
 
   // Ensure that the root node has a proxy for the child node's SiteInstance.
-  EXPECT_TRUE(root->render_manager()->proxy_hosts_[site_instance->GetId()]);
+  EXPECT_TRUE(root->render_manager()->GetRenderFrameProxyHost(child_instance));
 
   // Also ensure that the child has a proxy for the root node's SiteInstance.
-  EXPECT_TRUE(child->render_manager()->proxy_hosts_[
-              root->current_frame_host()->GetSiteInstance()->GetId()]);
+  EXPECT_TRUE(child->render_manager()->GetRenderFrameProxyHost(root_instance));
+
+  // The nodes should not have proxies for their own SiteInstance.
+  EXPECT_FALSE(root->render_manager()->GetRenderFrameProxyHost(root_instance));
+  EXPECT_FALSE(
+      child->render_manager()->GetRenderFrameProxyHost(child_instance));
 }
 
 }  // namespace content

content/browser/frame_host/render_frame_host_manager.cc

@@ -529,12 +529,7 @@ void RenderFrameHostManager::SwapOutOldPage() {
   // 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);
-  }
+  DeleteRenderFrameProxyHost(render_frame_host_->GetSiteInstance());
 
   RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
       render_frame_host_->GetSiteInstance(), frame_tree_node_);
@@ -1457,6 +1452,14 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 
   // Otherwise the same SiteInstance can be used.  Navigate render_frame_host_.
   DCHECK(!cross_navigation_pending_);
+
+  // It's possible to swap out the current RFH and then decide to navigate in it
+  // anyway (e.g., a cross-process navigation that redirects back to the
+  // original site).  In that case, we have a proxy for the current RFH but
+  // haven't deleted it yet.  The new navigation will swap it back in, so we can
+  // delete the proxy.
+  DeleteRenderFrameProxyHost(new_instance);
+
   if (ShouldReuseWebUI(current_entry, &entry)) {
     pending_web_ui_.reset();
     pending_and_current_web_ui_ = web_ui_->AsWeakPtr();
@@ -1591,4 +1594,13 @@ RenderFrameProxyHost* RenderFrameHostManager::GetRenderFrameProxyHost(
   return NULL;
 }
 
+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

content/browser/frame_host/render_frame_host_manager.h

@@ -459,6 +459,10 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // schedule new navigations in its swapped out RenderFrameHosts after this.
   void RendererProcessClosing(RenderProcessHost* render_process_host);
 
+  // Helper method to delete a RenderFrameProxyHost from the list, if one exists
+  // for the given |instance|.
+  void DeleteRenderFrameProxyHost(SiteInstance* instance);
+
   // For use in creating RenderFrameHosts.
   FrameTreeNode* frame_tree_node_;