Move existing kSitePerProcess checks to a policy-oracle object

content/browser/loader/cross_site_resource_handler.cc

 // Copyright (c) 2012 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/loader/cross_site_resource_handler.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "content/browser/appcache/appcache_interceptor.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/resource_controller.h"
 #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"
 
 namespace content {
 
 namespace {
 
 bool leak_requests_for_testing_ = false;
 
@@ skipping 28 matching lines @@
       new CrossSiteTransferringRequest(params.global_request_id));
 
   RenderFrameHostImpl* rfh =
       RenderFrameHostImpl::FromID(params.global_request_id.child_id,
                                   params.render_frame_id);
   if (rfh) {
     if (rfh->GetParent()) {
       // 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(),
         params.transfer_url_chain, params.referrer,
         params.page_transition, params.should_replace_current_entry);
   } else if (leak_requests_for_testing_ && cross_site_transferring_request) {
     // Some unit tests expect requests to be leaked in this case, so they can
     // pass them along manually.
     cross_site_transferring_request->ReleaseRequest();
   }
 }
 
 // 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)
     return false;
 
   // A transfer is not needed if the current SiteInstance doesn't yet have a
   // site.  This is the case for tests that use NavigateToURL.
   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
 
 CrossSiteResourceHandler::CrossSiteResourceHandler(
     scoped_ptr<ResourceHandler> next_handler,
     net::URLRequest* request)
     : LayeredResourceHandler(request, next_handler.Pass()),
       has_started_response_(false),
       in_cross_site_transition_(false),
@@ skipping 37 matching lines @@
   // renderer: it is not a download, and it has passed the SSL and safe
   // browsing checks.
   // We should not have already started the transition before now.
   DCHECK(!in_cross_site_transition_);
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
 
   // 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());
 
   // If this is a download, just pass the response through without doing a
   // cross-site check.  The renderer will see it is a download and abort the
   // request.
   //
   // Similarly, HTTP 204 (No Content) responses leave us showing the previous
   // page.  We should allow the navigation to finish without running the unload
   // handler or swapping in the pending RenderFrameHost.
   //
   // In both cases, any pending RenderFrameHost (if one was created for this
   // navigation) will stick around until the next cross-site navigation, since
   // we are unable to tell when to destroy it.
   // See RenderFrameHostManager::RendererAbortedProvisionalLoad.
   //
   // TODO(davidben): Unify IsDownload() and is_stream(). Several places need to
   // check for both and remembering about streams is error-prone.
   if (info->IsDownload() || info->is_stream() ||
       (response->head.headers.get() &&
        response->head.headers->response_code() == 204)) {
     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) {
   if (is_transfer) {
     StartCrossSiteTransition(response_.get());
   } else {
     ResumeResponse();
   }
 }
 
@@ skipping 146 matching lines @@
     controller()->Resume();
   }
 }
 
 void CrossSiteResourceHandler::OnDidDefer() {
   did_defer_ = true;
   request()->LogBlockedBy("CrossSiteResourceHandler");
 }
 
 }  // namespace content