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

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

Issue 1208143002: Move existing kSitePerProcess checks to a policy-oracle object (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@swapped_out_cmdline_checks
Patch Set: Partial fixes to Nasko's comments. Created 5 years, 5 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 cd85433714fe8f693e255ef4be2f90db85e957f3..164a7626633130211f7bb7d4c63b903712de77f0 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -40,6 +40,7 @@
#include "content/public/browser/web_ui_controller.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/referrer.h"
+#include "content/public/common/site_isolation_policy.h"
#include "content/public/common/url_constants.h"
namespace content {
@@ -52,8 +53,7 @@ bool RenderFrameHostManager::ClearRFHsPendingShutdown(FrameTreeNode* node) {
// static
bool RenderFrameHostManager::IsSwappedOutStateForbidden() {
- return base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
+ return SiteIsolationPolicy::AreCrossProcessFramesPossible();
}
RenderFrameHostManager::RenderFrameHostManager(
@@ -802,12 +802,11 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
// TODO(carlosk): Have renderer-initated main frame navigations swap processes
// if needed when it no longer breaks OAuth popups (see
// https://crbug.com/440266).
- bool site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
bool is_main_frame = frame_tree_node_->IsMainFrame();
if (current_site_instance == dest_site_instance.get() ||
(!request.browser_initiated() && is_main_frame) ||
- (!is_main_frame && !site_per_process)) {
+ (!is_main_frame && !dest_site_instance->RequiresDedicatedProcess() &&
+ !current_site_instance->RequiresDedicatedProcess())) {
// Reuse the current RFH if its SiteInstance matches the the navigation's
// or if this is a subframe navigation. We only swap RFHs for subframes when
// --site-per-process is enabled.
@@ -921,8 +920,9 @@ void RenderFrameHostManager::OnDidUpdateName(const std::string& name) {
// The window.name message may be sent outside of --site-per-process when
// report_frame_name_changes renderer preference is set (used by
// WebView). Don't send the update to proxies in those cases.
- if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess))
+ // TODO(nick,nasko): Should this be IsSwappedOutStateForbidden, to match
Charlie Reis 2015/07/13 22:13:14 That sounds reasonable to me.
ncarter (slow) 2015/07/20 17:45:46 Acknowledged.
+ // OnDidUpdateOrigin?
+ if (!SiteIsolationPolicy::AreCrossProcessFramesPossible())
return;
for (const auto& pair : proxy_hosts_) {
@@ -1005,16 +1005,23 @@ bool RenderFrameHostManager::ResetProxiesInSiteInstance(int32 site_instance_id,
}
bool RenderFrameHostManager::ShouldTransitionCrossSite() {
- // True for --site-per-process, which overrides both kSingleProcess and
- // kProcessPerTab.
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess))
+ // The logic below is weaker than "are all sites isolated" -- it asks instead,
+ // "is any site isolated". That's appropriate here since we're just trying to
+ // figure out if we're in any kind of site isolated mode, and in which case,
+ // we ignore the kSingleProcess and kProcessPerTab settings.
+ //
+ // TODO(nick): Move all handling of kSingleProcess/kProcessPerTab into
+ // SiteIsolationPolicy so we have a consistent behavior around the interaction
+ // of the process model flags.
+ if (SiteIsolationPolicy::AreCrossProcessFramesPossible())
return true;
// False in the single-process mode, as it makes RVHs to accumulate
// in swapped_out_hosts_.
// True if we are using process-per-site-instance (default) or
// process-per-site (kProcessPerSite).
+ // TODO(nick): Move handling of kSingleProcess and kProcessPerTab into
+ // SiteIsolationPolicy.
return !base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kSingleProcess) &&
!base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -1359,8 +1366,7 @@ const GURL& RenderFrameHostManager::GetCurrentURLForSiteInstance(
// TODO(creis): Remove this when we can check the FrameNavigationEntry's url.
// See http://crbug.com/369654
if (!frame_tree_node_->IsMainFrame() &&
- base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess))
+ SiteIsolationPolicy::AreCrossProcessFramesPossible())
return frame_tree_node_->current_url();
// If there is no last non-interstitial entry (and current_instance already
@@ -1416,8 +1422,7 @@ int RenderFrameHostManager::CreateProxiesForNewRenderFrameHost(
// Only create opener proxies if they are in the same BrowsingInstance.
if (new_instance->IsRelatedSiteInstance(old_instance))
opener_route_id = CreateOpenerProxiesIfNeeded(new_instance);
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess)) {
+ if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
// Ensure that the frame tree has RenderFrameProxyHosts for the new
// SiteInstance in all nodes except the current one. We do this for all
// frames in the tree, whether they are in the same BrowsingInstance or not.
@@ -1517,12 +1522,11 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
int* view_routing_id_ptr) {
bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
bool swapped_out_forbidden = IsSwappedOutStateForbidden();
- bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
CHECK(instance);
CHECK_IMPLIES(swapped_out_forbidden, !swapped_out);
- CHECK_IMPLIES(!is_site_per_process, frame_tree_node_->IsMainFrame());
+ CHECK_IMPLIES(!SiteIsolationPolicy::AreCrossProcessFramesPossible(),
+ frame_tree_node_->IsMainFrame());
// Swapped out views should always be hidden.
DCHECK_IMPLIES(swapped_out, (flags & CREATE_RF_HIDDEN));
@@ -1703,8 +1707,7 @@ void RenderFrameHostManager::EnsureRenderViewInitialized(
void RenderFrameHostManager::CreateOuterDelegateProxy(
SiteInstance* outer_contents_site_instance,
RenderFrameHostImpl* render_frame_host) {
- CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess));
+ CHECK(SiteIsolationPolicy::GuestsShouldUseCrossProcessFrames());
RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
outer_contents_site_instance, nullptr, frame_tree_node_);
proxy_hosts_[outer_contents_site_instance->GetId()] = proxy;
@@ -1972,18 +1975,17 @@ void RenderFrameHostManager::CommitPending() {
}
}
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess)) {
- // If this is a subframe, it should have a CrossProcessFrameConnector
- // created already. Use it to link the new RFH's view to the proxy that
- // belongs to the parent frame's SiteInstance. If this navigation causes
- // an out-of-process frame to return to the same process as its parent, the
- // proxy would have been removed from proxy_hosts_ above.
- // Note: We do this after swapping out the old RFH because that may create
- // the proxy we're looking for.
- RenderFrameProxyHost* proxy_to_parent = GetProxyToParent();
- if (proxy_to_parent)
- proxy_to_parent->SetChildRWHView(render_frame_host_->GetView());
+ // If this is a subframe, it should have a CrossProcessFrameConnector
+ // created already. Use it to link the new RFH's view to the proxy that
+ // belongs to the parent frame's SiteInstance. If this navigation causes
+ // an out-of-process frame to return to the same process as its parent, the
+ // proxy would have been removed from proxy_hosts_ above.
+ // Note: We do this after swapping out the old RFH because that may create
+ // the proxy we're looking for.
+ RenderFrameProxyHost* proxy_to_parent = GetProxyToParent();
+ if (proxy_to_parent) {
+ CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
+ proxy_to_parent->SetChildRWHView(render_frame_host_->GetView());
}
// After all is done, there must never be a proxy in the list which has the
@@ -2044,9 +2046,9 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
// Don't swap for subframes unless we are in --site-per-process. We can get
// here in tests for subframes (e.g., NavigateFrameToURL).
if (!frame_tree_node_->IsMainFrame() &&
- !base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess))
+ !SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
return render_frame_host_.get();
+ }
// If we are currently navigating cross-process, we want to get back to normal
// and then navigate as usual.
@@ -2306,7 +2308,7 @@ int RenderFrameHostManager::CreateOpenerProxies(SiteInstance* instance) {
return rvh->GetRoutingID();
int render_view_routing_id = MSG_ROUTING_NONE;
- if (RenderFrameHostManager::IsSwappedOutStateForbidden()) {
+ if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
// Ensure that all the nodes in the opener's frame tree have
// RenderFrameProxyHosts for the new SiteInstance.
frame_tree->CreateProxiesForSiteInstance(nullptr, instance);

Powered by Google App Engine
This is Rietveld 408576698