Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(302)

Unified Diff: content/browser/frame_host/render_frame_host_manager.cc

Issue 471603002: Remove dependency on NavigationEntry to get a SiteInstance (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressed Charlie's comments Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index b40cc2b39218a103adb484a3a35fc33e66e78b57..ca2a3ff815b7af4e85ef6d034bd6642ad8966d36 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -645,53 +645,43 @@ bool RenderFrameHostManager::ShouldTransitionCrossSite() {
}
bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(
- const NavigationEntry* current_entry,
- const NavigationEntryImpl* new_entry) const {
- DCHECK(new_entry);
-
+ const GURL& current_effective_url,
+ bool current_is_view_source_mode,
+ SiteInstance* new_site_instance,
+ const GURL& new_effective_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))
+ if (IsRendererDebugURL(new_effective_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)) {
+ browser_context, current_effective_url)) {
// If so, force a swap if destination is not an acceptable URL for Web UI.
// Here, data URLs are never allowed.
if (!WebUIControllerFactoryRegistry::GetInstance()->IsURLAcceptableForWebUI(
- browser_context, new_url)) {
+ browser_context, new_effective_url)) {
return true;
}
} else {
// Force a swap if it's a Web UI URL.
if (WebUIControllerFactoryRegistry::GetInstance()->UseWebUIForURL(
- browser_context, new_url)) {
+ browser_context, new_effective_url)) {
return true;
}
}
@@ -700,7 +690,7 @@ bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(
// which uses the SiteInstance's site if there is no current_entry.
if (GetContentClient()->browser()->ShouldSwapBrowsingInstancesForNavigation(
render_frame_host_->GetSiteInstance(),
- current_url, new_url)) {
+ current_effective_url, new_effective_url)) {
return true;
}
@@ -708,8 +698,7 @@ bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(
// 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())
+ if (current_is_view_source_mode != new_is_view_source_mode)
return true;
return false;
@@ -727,24 +716,26 @@ bool RenderFrameHostManager::ShouldReuseWebUI(
controller.GetBrowserContext(), new_entry->GetURL()));
}
-SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
- const NavigationEntryImpl& entry,
+SiteInstance* RenderFrameHostManager::GetSiteInstanceForURL(
+ const GURL& dest_url,
+ SiteInstance* dest_instance,
+ PageTransition dest_transition,
+ bool dest_is_restore,
+ bool dest_is_view_source_mode,
SiteInstance* current_instance,
bool force_browsing_instance_swap) {
- // Determine which SiteInstance to use for navigating to |entry|.
- const GURL& dest_url = entry.GetURL();
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 (dest_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(!dest_instance->IsRelatedSiteInstance(
render_frame_host_->GetSiteInstance()));
}
- return entry.site_instance();
+ return dest_instance;
}
// If a swap is required, we need to force the SiteInstance AND
@@ -763,8 +754,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
// RenderViews in response to a link click.
//
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kProcessPerSite) &&
- PageTransitionCoreTypeIs(entry.GetTransitionType(),
- PAGE_TRANSITION_GENERATED)) {
+ PageTransitionCoreTypeIs(dest_transition, PAGE_TRANSITION_GENERATED)) {
return current_instance;
}
@@ -806,7 +796,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
// 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 (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
@@ -830,7 +820,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
// 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 (dest_is_restore &&
GetContentClient()->browser()->ShouldAssignSiteForURL(dest_url)) {
current_site_instance->SetSite(dest_url);
}
@@ -873,7 +863,7 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForEntry(
// 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() != dest_is_view_source_mode &&
!IsRendererDebugURL(dest_url)) {
return SiteInstance::CreateForURL(browser_context, dest_url);
}
@@ -1332,10 +1322,31 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
// process-per-tab model, such as WebUI pages.
const NavigationEntry* current_entry =
delegate_->GetLastCommittedNavigationEntryForRenderManager();
+ BrowserContext* browser_context =
+ delegate_->GetControllerForRenderManager().GetBrowserContext();
+ const GURL& current_effective_url = current_entry ?
+ SiteInstanceImpl::GetEffectiveURL(browser_context,
+ current_entry->GetURL()) :
+ render_frame_host_->GetSiteInstance()->GetSiteURL();
+ bool current_is_view_source_mode = current_entry ?
+ current_entry->IsViewSourceMode() : entry.IsViewSourceMode();
bool force_swap = !is_guest_scheme &&
- ShouldSwapBrowsingInstancesForNavigation(current_entry, &entry);
- if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap))
- new_instance = GetSiteInstanceForEntry(entry, current_instance, force_swap);
+ ShouldSwapBrowsingInstancesForNavigation(
+ current_effective_url,
+ current_is_view_source_mode,
+ entry.site_instance(),
+ SiteInstanceImpl::GetEffectiveURL(browser_context, entry.GetURL()),
+ entry.IsViewSourceMode());
+ if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap)) {
+ new_instance = GetSiteInstanceForURL(
+ entry.GetURL(),
+ entry.site_instance(),
+ entry.GetTransitionType(),
+ entry.restore_type() != NavigationEntryImpl::RESTORE_NONE,
+ entry.IsViewSourceMode(),
+ current_instance,
+ 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

Powered by Google App Engine
This is Rietveld 408576698