Move existing kSitePerProcess checks to a policy-oracle object

content/browser/loader/cross_site_resource_handler.cc

Index: content/browser/loader/cross_site_resource_handler.cc
diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc
index ebf40eb023a6abfd0047b64e11de9ef04dd57eb7..a070f16b6d940c25c28b6ab9a6e4d077591f4522 100644
--- a/content/browser/loader/cross_site_resource_handler.cc
+++ b/content/browser/loader/cross_site_resource_handler.cc
@@ -23,6 +23,7 @@
 #include "content/public/browser/site_instance.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/resource_response.h"
+#include "content/public/common/site_isolation_policy.h"
 #include "content/public/common/url_constants.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
@@ -71,8 +72,7 @@ void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) {
       // We should only swap processes for subframes in --site-per-process mode.
       // CrossSiteResourceHandler is not installed on subframe requests in
       // default Chrome.
-      CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSitePerProcess));
+      CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
     }
     rfh->OnCrossSiteResponse(
         params.global_request_id, cross_site_transferring_request.Pass(),
@@ -87,8 +87,7 @@ void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) {
 
 // Returns whether a transfer is needed by doing a check on the UI thread.
 bool CheckNavigationPolicyOnUI(GURL url, int process_id, int render_frame_id) {
-  CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
-      switches::kSitePerProcess));
+  CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
   RenderFrameHostImpl* rfh =
       RenderFrameHostImpl::FromID(process_id, render_frame_id);
   if (!rfh)
@@ -99,12 +98,18 @@ bool CheckNavigationPolicyOnUI(GURL url, int process_id, int render_frame_id) {
   if (!rfh->GetSiteInstance()->HasSite())
     return false;
 
-  // TODO(nasko): This check is very simplistic and is used temporarily only
-  // for --site-per-process. It should be updated to match the check performed
-  // by RenderFrameHostManager::UpdateStateForNavigate.
-  return !SiteInstance::IsSameWebSite(
-      rfh->GetSiteInstance()->GetBrowserContext(),
-      rfh->GetSiteInstance()->GetSiteURL(), url);
+  // TODO(nasko, nick): These following --site-per-process checks are
+  // overly simplistic. Update them to match all the cases
+  // considered by RenderFrameHostManager::UpdateStateForNavigate.

[nasko, 2015/07/08 12:52:23]

We should update this comment to point to
RenderFrameHostManager::DetermineSiteInstanceForURL instead of
UpdateStateForNavigate.

[ncarter (slow), 2015/07/10 23:29:18]

Done.

+  if (SiteInstance::IsSameWebSite(rfh->GetSiteInstance()->GetBrowserContext(),
+                                  rfh->GetSiteInstance()->GetSiteURL(), url)) {
+    return false;  // The same site, no transition needed.
+  }
+
+  // The sites differ. If either one requires a dedicated process,
+  // then a transfer is needed.
+  return rfh->GetSiteInstance()->RequiresDedicatedProcess() ||
+         SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(url);
 }
 
 }  // namespace
@@ -162,9 +167,11 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
   // We only need to pause the response if a transfer to a different process is
   // required.  Other cross-process navigations can proceed immediately, since
   // we run the unload handler at commit time.
+  // TODO(nick): What does "other cross-process navigations" mean in the

[nasko, 2015/07/08 12:52:23]

There are two types of cross-process navigations:
* We determine we need to do a cross-process navigation, allocate the new
renderer, then make the request. This is usually how browser initiated
navigations happen.
* A resource request for the navigation is made and at the network stack/IO
thread we check whether a cross-process navigation is needed, then we pause the
response, hop to the UI thread, allocate the new renderer, update the response
metadata with the new renderer id, then resume the response, so it goes to the
new renderer.

I think the above comment means the first type when it says "other", since it
didn't go through this transfer logic.

[ncarter (slow), 2015/07/10 23:29:18]

Done.

+  // above language. Is that a typo?
   // Note that a process swap may no longer be necessary if we transferred back
   // into the original process due to a redirect.
-  bool should_transfer =
+  bool embedder_requests_transfer =

[nasko, 2015/07/08 12:52:23]

What is "embedder" in this context?

[ncarter (slow), 2015/07/10 23:29:18]

I meant the content embedder (GetContentClient()). I changed the variable name.

       GetContentClient()->browser()->ShouldSwapProcessesForRedirect(
           info->GetContext(), request()->original_url(), request()->url());
 
@@ -189,30 +196,30 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted(
     return next_handler_->OnResponseStarted(response, defer);
   }
 
-  // When the --site-per-process flag is passed, we transfer processes for
-  // cross-site navigations. This is skipped if a transfer is already required
-  // or for WebUI processes for now, since pages like the NTP host multiple
-  // cross-site WebUI iframes.
-  if (!should_transfer &&
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSitePerProcess) &&
+  if (embedder_requests_transfer) {
+    // Now that we know a transfer is needed and we have something to commit, we
+    // pause to let the UI thread set up the transfer.
+    StartCrossSiteTransition(response);
+
+    // Defer loading until after the new renderer process has issued a
+    // corresponding request.
+    *defer = true;
+    OnDidDefer();
+    return true;
+  }
+
+  // Under the --site-per-process flag, we'll need to consult the policy to
+  // determine whether to transfer processes. Process transfers are skipped for
+  // WebUI processes for now, since pages like the NTP host multiple cross-site
+  // WebUI iframes.
+  if (SiteIsolationPolicy::AreCrossProcessFramesPossible() &&
       !ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
           info->GetChildID())) {
     return DeferForNavigationPolicyCheck(info, response, defer);
   }
 
-  if (!should_transfer)
-    return next_handler_->OnResponseStarted(response, defer);
-
-  // Now that we know a transfer is needed and we have something to commit, we
-  // pause to let the UI thread set up the transfer.
-  StartCrossSiteTransition(response);
-
-  // Defer loading until after the new renderer process has issued a
-  // corresponding request.
-  *defer = true;
-  OnDidDefer();
-  return true;
+  // No process transfer needed. Pass the response through.
+  return next_handler_->OnResponseStarted(response, defer);
 }
 
 void CrossSiteResourceHandler::ResumeOrTransfer(bool is_transfer) {