PlzNavigate: Avoid duplicate SiteInstance creation during navigation.

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/logging.h"
@@ skipping 976 matching lines @@
       render_frame_host_->GetSiteInstance()->GetSiteURL();
   bool current_is_view_source_mode = current_entry ?
       current_entry->IsViewSourceMode() : dest_is_view_source_mode;
   bool force_swap = ShouldSwapBrowsingInstancesForNavigation(
       current_effective_url,
       current_is_view_source_mode,
       dest_instance,
       SiteInstanceImpl::GetEffectiveURL(browser_context, dest_url),
       dest_is_view_source_mode);
   if (ShouldTransitionCrossSite() || force_swap) {
-    new_instance = GetSiteInstanceForURL(
-        dest_url, source_instance, current_instance, dest_instance,
-        transition, dest_is_restore, dest_is_view_source_mode, force_swap);
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableBrowserSideNavigation) &&
+        speculative_render_frame_host_ &&
+        speculative_render_frame_host_->GetSiteInstance()->GetSiteURL() ==
+            SiteInstanceImpl::GetSiteForURL(browser_context, dest_url)) {

[Charlie Reis, 2015/03/05 04:27:11]

SiteInstanceImpl::GetSiteForURL is not equivalent to
RFHM::GetSiteInstanceForURL, so I think this will have problems.

For example, consider the source_instance case that we use for about:blank and
data: URLs.  GetSiteInstanceForURL puts those in the source SiteInstance, so
their site URL won't match GetSiteForURL(browser_context, "about:blank").

[carlosk, 2015/03/06 16:03:54]

Acknowledged.

+      // We already created a new SiteInstance for this specific URL during this
+      // navigation and used it to create the speculative RenderFrameHost, so
+      // there's no need to create yet another new one.
+      new_instance = speculative_render_frame_host_->GetSiteInstance();
+    } else {
+      new_instance = GetSiteInstanceForURL(
+          dest_url, source_instance, current_instance, dest_instance,
+          transition, dest_is_restore, dest_is_view_source_mode, force_swap);
+    }
   }
 
   // If force_swap is true, we must use a different SiteInstance.  If we didn't,
   // we would have two RenderFrameHosts in the same SiteInstance and the same
   // frame, resulting in page_id conflicts for their NavigationEntries.
   if (force_swap)
     CHECK_NE(new_instance, current_instance);
   return new_instance;
 }
 
@@ skipping 79 matching lines @@
     // TODO(nasko): This is the same condition as later in the function. This
     // should be taken into account when refactoring this method as part of
     // http://crbug.com/123007.
     if (dest_is_view_source_mode)
       return SiteInstance::CreateForURL(browser_context, dest_url);
 
     // If we are navigating from a blank SiteInstance to a WebUI, make sure we
     // create a new SiteInstance.
     if (WebUIControllerFactoryRegistry::GetInstance()->UseWebUIForURL(
             browser_context, dest_url)) {
-        return SiteInstance::CreateForURL(browser_context, dest_url);
+      return SiteInstance::CreateForURL(browser_context, dest_url);
     }
 
     // Normally the "site" on the SiteInstance is set lazily when the load
     // actually commits. This is to support better process sharing in case
     // the site redirects to some other site: we want to use the destination
     // site in the site instance.
     //
     // In the case of session restore, as it loads all the pages immediately
     // we need to set the site first, otherwise after a restore none of the
     // pages would share renderers in process-per-site.
@@ skipping 840 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