Remove dependency on NavigationEntry to get a SiteInstance

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 628 matching lines @@
   // in swapped_out_hosts_.
   // True if we are using process-per-site-instance (default) or
   // process-per-site (kProcessPerSite).
   return
       !CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) &&
       !CommandLine::ForCurrentProcess()->HasSwitch(switches::kProcessPerTab);
 }
 
 bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(
     const NavigationEntry* current_entry,
-    const NavigationEntryImpl* new_entry) const {
-  DCHECK(new_entry);
-
+    SiteInstance* new_site_instance,
+    const GURL& new_url,
+    bool new_is_view_source_mode) const {
   // If new_entry already has a SiteInstance, assume it is correct.  We only
   // need to force a swap if it is in a different BrowsingInstance.
-  if (new_entry->site_instance()) {
-    return !new_entry->site_instance()->IsRelatedSiteInstance(
+  if (new_site_instance) {
+    return !new_site_instance->IsRelatedSiteInstance(
         render_frame_host_->GetSiteInstance());
   }
 
   // Check for reasons to swap processes even if we are in a process model that
   // doesn't usually swap (e.g., process-per-tab).  Any time we return true,
   // the new_entry will be rendered in a new SiteInstance AND BrowsingInstance.
 
   // We use the effective URL here, since that's what is used in the
   // SiteInstance's site and when we later call IsSameWebSite.  If there is no
   // current_entry, check the current SiteInstance's site, which might already
   // be committed to a Web UI URL (such as the NTP).
   BrowserContext* browser_context =
       delegate_->GetControllerForRenderManager().GetBrowserContext();
   const GURL& current_url = (current_entry) ?
       SiteInstanceImpl::GetEffectiveURL(browser_context,
                                         current_entry->GetURL()) :
       render_frame_host_->GetSiteInstance()->GetSiteURL();
-  const GURL& new_url = SiteInstanceImpl::GetEffectiveURL(browser_context,
-                                                          new_entry->GetURL());
 
   // Don't force a new BrowsingInstance for debug URLs that are handled in the
   // renderer process, like javascript: or chrome://crash.
   if (IsRendererDebugURL(new_url))
     return false;
 
   // For security, we should transition between processes when one is a Web UI
   // page and one isn't.
   if (WebUIControllerFactoryRegistry::GetInstance()->UseWebUIForURL(
           browser_context, current_url)) {
@@ skipping 17 matching lines @@
           render_frame_host_->GetSiteInstance(),
           current_url, new_url)) {
     return true;
   }
 
   // We can't switch a RenderView between view source and non-view source mode
   // without screwing up the session history sometimes (when navigating between
   // "view-source:http://foo.com/" and "http://foo.com/", Blink doesn't treat
   // it as a new navigation). So require a BrowsingInstance switch.
   if (current_entry &&
-      current_entry->IsViewSourceMode() != new_entry->IsViewSourceMode())
+      current_entry->IsViewSourceMode() != new_is_view_source_mode)
     return true;
 
   return false;
 }
 
 bool RenderFrameHostManager::ShouldReuseWebUI(
     const NavigationEntry* current_entry,
     const NavigationEntryImpl* new_entry) const {
   NavigationControllerImpl& controller =
       delegate_->GetControllerForRenderManager();
   return current_entry && web_ui_.get() &&
       (WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
           controller.GetBrowserContext(), current_entry->GetURL()) ==
        WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
           controller.GetBrowserContext(), new_entry->GetURL()));
 }
 
-SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
-    const NavigationEntryImpl& entry,
+SiteInstance* RenderFrameHostManager::GetSiteInstanceForURL(
+    const GURL& dest_url,
+    SiteInstance* navigation_instance,
     SiteInstance* current_instance,
-    bool force_browsing_instance_swap) {
-  // Determine which SiteInstance to use for navigating to |entry|.
-  const GURL& dest_url = entry.GetURL();
+    PageTransition page_transition,
+    NavigationEntryImpl::RestoreType restore_type,
+    bool force_browsing_instance_swap,
+    bool is_view_source_mode) {
   NavigationControllerImpl& controller =
       delegate_->GetControllerForRenderManager();
   BrowserContext* browser_context = controller.GetBrowserContext();
 
   // If the entry has an instance already we should use it.
-  if (entry.site_instance()) {
+  if (navigation_instance) {
     // If we are forcing a swap, this should be in a different BrowsingInstance.
     if (force_browsing_instance_swap) {
-      CHECK(!entry.site_instance()->IsRelatedSiteInstance(
+      CHECK(!navigation_instance->IsRelatedSiteInstance(
                 render_frame_host_->GetSiteInstance()));
     }
-    return entry.site_instance();
+    return navigation_instance;
   }
 
   // If a swap is required, we need to force the SiteInstance AND
   // BrowsingInstance to be different ones, using CreateForURL.
   if (force_browsing_instance_swap)
     return SiteInstance::CreateForURL(browser_context, dest_url);
 
   // (UGLY) HEURISTIC, process-per-site only:
   //
   // If this navigation is generated, then it probably corresponds to a search
   // query.  Given that search results typically lead to users navigating to
   // other sites, we don't really want to use the search engine hostname to
   // determine the site instance for this navigation.
   //
   // NOTE: This can be removed once we have a way to transition between
   //       RenderViews in response to a link click.
   //
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kProcessPerSite) &&
-      PageTransitionCoreTypeIs(entry.GetTransitionType(),
-                               PAGE_TRANSITION_GENERATED)) {
+      PageTransitionCoreTypeIs(page_transition, PAGE_TRANSITION_GENERATED)) {
     return current_instance;
   }
 
   SiteInstanceImpl* current_site_instance =
       static_cast<SiteInstanceImpl*>(current_instance);
 
   // If we haven't used our SiteInstance (and thus RVH) yet, then we can use it
   // for this entry.  We won't commit the SiteInstance to this site until the
   // navigation commits (in DidNavigate), unless the navigation entry was
   // restored or it's a Web UI as described below.
@@ skipping 21 matching lines @@
     // not want to use the current_instance if it has no site, since it will
     // have a RenderProcessHost of PRIV_NORMAL.  Create a new SiteInstance for
     // this URL instead (with the correct process type).
     if (current_site_instance->HasWrongProcessForURL(dest_url))
       return current_site_instance->GetRelatedSiteInstance(dest_url);
 
     // View-source URLs must use a new SiteInstance and BrowsingInstance.
     // 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 (entry.IsViewSourceMode())
+    if (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);
     }
 
     // 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.
     //
     // The embedder can request some urls never to be assigned to SiteInstance
     // through the ShouldAssignSiteForURL() content client method, so that
     // renderers created for particular chrome urls (e.g. the chrome-native://
     // scheme) can be reused for subsequent navigations in the same WebContents.
     // See http://crbug.com/386542.
-    if (entry.restore_type() != NavigationEntryImpl::RESTORE_NONE &&
+    if (restore_type != NavigationEntryImpl::RESTORE_NONE &&
         GetContentClient()->browser()->ShouldAssignSiteForURL(dest_url)) {
       current_site_instance->SetSite(dest_url);
     }
 
     return current_site_instance;
   }
 
   // Otherwise, only create a new SiteInstance for a cross-site navigation.
 
   // TODO(creis): Once we intercept links and script-based navigations, we
@@ skipping 22 matching lines @@
   // 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() != entry.IsViewSourceMode() &&
+      current_entry->IsViewSourceMode() != 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.)
   if (SiteInstance::IsSameWebSite(browser_context, current_url, dest_url) &&
       !current_site_instance->HasWrongProcessForURL(dest_url)) {
     return current_instance;
@@ skipping 438 matching lines @@
   // We do not currently swap processes for navigations in webview tag guests.
   bool is_guest_scheme = current_instance->GetSiteURL().SchemeIs(kGuestScheme);
 
   // Determine if we need a new BrowsingInstance for this entry.  If true, this
   // implies that it will get a new SiteInstance (and likely process), and that
   // other tabs in the current BrowsingInstance will be unable to script it.
   // This is used for cases that require a process swap even in the
   // process-per-tab model, such as WebUI pages.
   const NavigationEntry* current_entry =
       delegate_->GetLastCommittedNavigationEntryForRenderManager();
+  BrowserContext* browser_context =
+      delegate_->GetControllerForRenderManager().GetBrowserContext();
   bool force_swap = !is_guest_scheme &&
-      ShouldSwapBrowsingInstancesForNavigation(current_entry, &entry);
+      ShouldSwapBrowsingInstancesForNavigation(
+          current_entry,
+          entry.site_instance(),
+          SiteInstanceImpl::GetEffectiveURL(browser_context, entry.GetURL()),
+          entry.IsViewSourceMode());
   if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap))

[Charlie Reis, 2014/08/14 06:28:08]

This needs braces if the body is more than one line.

[clamy, 2014/08/14 10:02:59]

Done.

-    new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap);
+    new_instance = GetSiteInstanceForURL(
+        entry.GetURL(),
+        entry.site_instance(),
+        current_instance,
+        entry.GetTransitionType(),
+        entry.restore_type(),
+        force_swap,
+        entry.IsViewSourceMode());
 
   // 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);
 
   if (new_instance != current_instance) {
     // New SiteInstance: create a pending RFH to navigate.
     DCHECK(!cross_navigation_pending_);
@@ skipping 241 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