Enable swapping a frame back in to its parent process

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 913 matching lines @@
   // const GURL& current_url = current_instance->site();
   // For now, though, we're in a hybrid model where you only switch
   // SiteInstances if you type in a cross-site URL.  This means we have to
   // compare the entry's URL to the last committed entry's URL.
   NavigationEntry* current_entry = controller.GetLastCommittedEntry();
   if (interstitial_page_) {
     // The interstitial is currently the last committed entry, but we want to
     // compare against the last non-interstitial entry.
     current_entry = controller.GetEntryAtOffset(-1);
   }
-  // If there is no last non-interstitial entry (and current_instance already
-  // has a site), then we must have been opened from another tab.  We want
-  // to compare against the URL of the page that opened us, but we can't
-  // get to it directly.  The best we can do is check against the site of
-  // the SiteInstance.  This will be correct when we intercept links and
-  // script-based navigations, but for now, it could place some pages in a
-  // new process unnecessarily.  We should only hit this case if a page tries
-  // to open a new tab to an interstitial-inducing URL, and then navigates
-  // the page to a different same-site URL.  (This seems very unlikely in
-  // practice.)
-  const GURL& current_url = (current_entry) ? current_entry->GetURL() :
-      current_instance->GetSiteURL();
 
   // View-source URLs must use a new SiteInstance and BrowsingInstance.
   // We don't need a swap when going from view-source to a debug URL like
   // chrome://crash, however.
   // TODO(creis): Refactor this method so this duplicated code isn't needed.
   // See http://crbug.com/123007.
   if (current_entry &&
       current_entry->IsViewSourceMode() != dest_is_view_source_mode &&
       !IsRendererDebugURL(dest_url)) {
     return SiteInstance::CreateForURL(browser_context, dest_url);
   }
 
   // Use the current SiteInstance for same site navigations, as long as the
   // process type is correct.  (The URL may have been installed as an app since
   // the last time we visited it.)
+  const GURL& current_url =
+      GetCurrentURLForSiteInstance(current_instance, current_entry);
   if (SiteInstance::IsSameWebSite(browser_context, current_url, dest_url) &&
       !current_site_instance->HasWrongProcessForURL(dest_url)) {
     return current_instance;
   }
 
   // Start the new renderer in a new SiteInstance, but in the current
   // BrowsingInstance.  It is important to immediately give this new
   // SiteInstance to a RenderViewHost (if it is different than our current
   // SiteInstance), so that it is ref counted.  This will happen in
   // CreateRenderView.
   return current_instance->GetRelatedSiteInstance(dest_url);
 }
 
+const GURL& RenderFrameHostManager::GetCurrentURLForSiteInstance(

[Charlie Reis, 2014/10/30 19:36:06]

Returning a const ref here seems risky if anyone tries to later return a GURL
allocated in this method.  That said, I think we can get rid of this function
once we have FrameNavigationEntry, so I'm ok with it for now.

[Nate Chapin, 2014/10/30 23:12:29]

Yeah, I was just matching the types used in the code previously, I agree that
it's not ideal long term, but it shouldn't be anyway.

+    SiteInstance* current_instance, NavigationEntry* current_entry) {
+  // If this is a subframe that is potentially out of process from its parent,
+  // don't consider using current_entry's url for SiteInstance selection, since

[Charlie Reis, 2014/10/30 19:36:06]

nit: Change the second clause to "since current_entry's url is for the main
frame and may be from a different site than this frame."

Also add:
TODO(creis): Remove this when we can check the FrameNavigationEntry's URL.  See
http://crbug.com/369654.

[Nate Chapin, 2014/10/30 23:12:28]

Done.

+  // current_entry may be in a different site than this frame already.
+  if (!frame_tree_node_->IsMainFrame() &&
+      CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess))
+    return frame_tree_node_->current_url();
+
+  // If there is no last non-interstitial entry (and current_instance already
+  // has a site), then we must have been opened from another tab.  We want
+  // to compare against the URL of the page that opened us, but we can't
+  // get to it directly.  The best we can do is check against the site of
+  // the SiteInstance.  This will be correct when we intercept links and
+  // script-based navigations, but for now, it could place some pages in a
+  // new process unnecessarily.  We should only hit this case if a page tries
+  // to open a new tab to an interstitial-inducing URL, and then navigates
+  // the page to a different same-site URL.  (This seems very unlikely in
+  // practice.)
+  if (current_entry)
+    return current_entry->GetURL();
+  return current_instance->GetSiteURL();
+}
+
 void RenderFrameHostManager::CreateRenderFrameHostForNewSiteInstance(
     SiteInstance* old_instance,
     SiteInstance* new_instance,
     bool is_main_frame) {
   // Ensure that we have created RFHs for the new RFH's opener chain if
   // we are staying in the same BrowsingInstance. This allows the new RFH
   // to send cross-process script calls to its opener(s).
   int opener_route_id = MSG_ROUTING_NONE;
   if (new_instance->IsRelatedSiteInstance(old_instance)) {
     opener_route_id =
@@ skipping 197 matching lines @@
                                                      proxy_routing_id,
                                                      for_main_frame_navigation);
 }
 
 bool RenderFrameHostManager::InitRenderFrame(
     RenderFrameHostImpl* render_frame_host) {
   if (render_frame_host->IsRenderFrameLive())
     return true;
 
   int parent_routing_id = MSG_ROUTING_NONE;
+  int proxy_routing_id = MSG_ROUTING_NONE;
   if (frame_tree_node_->parent()) {
     parent_routing_id = frame_tree_node_->parent()->render_manager()->
         GetRoutingIdForSiteInstance(render_frame_host->GetSiteInstance());
     CHECK_NE(parent_routing_id, MSG_ROUTING_NONE);
   }
+  // Check whether there is an existing proxy for this frame in this
+  // SiteInstance. If there is, the new RenderFrame needs to be able to find
+  // the proxy it is replacing, so that it can fully initialize itself.
+  // NOTE: This is the only time that a RenderFrameProxyHost can be in the same
+  // SiteInstance as its RenderFrameHost. This is only the case until the
+  // RenderFrameHost commits, at which point it will replace and delete the
+  // RenderFrameProxyHost.
+  RenderFrameProxyHost* existing_proxy =
+      GetRenderFrameProxyHost(render_frame_host->GetSiteInstance());
+  if (existing_proxy) {
+    proxy_routing_id = existing_proxy->GetRoutingID();
+    CHECK_NE(proxy_routing_id, MSG_ROUTING_NONE);
+  }
   return delegate_->CreateRenderFrameForRenderManager(render_frame_host,
-                                                      parent_routing_id);
+                                                      parent_routing_id,
+                                                      proxy_routing_id);
 }
 
 int RenderFrameHostManager::GetRoutingIdForSiteInstance(
     SiteInstance* site_instance) {
   if (render_frame_host_->GetSiteInstance() == site_instance)
     return render_frame_host_->GetRoutingID();
 
   RenderFrameProxyHostMap::iterator iter =
       proxy_hosts_.find(site_instance->GetId());
   if (iter != proxy_hosts_.end())
@@ skipping 408 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