Centralize SiteInstanceImpl::set_process_reuse_policy() usage.

- Move the majority of set_process_reuse_policy() usage into creation
functions since most calls are made almost immediately after a Create
call.

- Created SiteInstanceImpl::CreateReusableInstanceForTesting() to
  explicitly mark instances where tests are creating SiteInstances
  that they expect to be able to reuse a process AND are expecting to
  not ever receive a default SiteInstance. This method will make it easier
  to avoid breaking tests when SiteInstanceImpll::CreateForURL() is
  updated to allow the default SiteInstance to be returned.

- Minor changes to make sure set_process_reuse_policy() is never called
  on the default SiteInstance.

Bug: 958060
Change-Id: I3e6a2dc9eb1f3f2abaccc690b2d8b1edd7e2b182
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1603413Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659307}

content/browser/frame_host/render_frame_host_manager.cc

@@ -1241,7 +1241,9 @@ RenderFrameHostManager::GetSiteInstanceForNavigation(
   // consolidated into existing processes for that site.
   SiteInstanceImpl* new_instance_impl =
       static_cast<SiteInstanceImpl*>(new_instance.get());
-  if (!frame_tree_node_->IsMainFrame() && !new_instance_impl->HasProcess() &&
+  if (!frame_tree_node_->IsMainFrame() &&
+      !new_instance_impl->IsDefaultSiteInstance() &&
+      !new_instance_impl->HasProcess() &&
       new_instance_impl->RequiresDedicatedProcess()) {
     // Also give the embedder a chance to override this decision.  Certain
     // frames have different enough workloads so that it's better to avoid

content/browser/isolated_origin_browsertest.cc

@@ -1100,9 +1100,8 @@ IN_PROC_BROWSER_TEST_F(
   // not reuse the unsuitable first process.
   scoped_refptr<SiteInstanceImpl> sw_site_instance =
       SiteInstanceImpl::CreateForServiceWorker(
-          web_contents()->GetBrowserContext(), hung_isolated_url);
-  sw_site_instance->set_process_reuse_policy(
-      SiteInstanceImpl::ProcessReusePolicy::REUSE_PENDING_OR_COMMITTED_SITE);
+          web_contents()->GetBrowserContext(), hung_isolated_url,
+          /* can_reuse_process */ true);
   RenderProcessHost* sw_host = sw_site_instance->GetProcess();
   EXPECT_NE(new_shell->web_contents()->GetMainFrame()->GetProcess(), sw_host);
 

content/browser/service_worker/service_worker_process_manager.cc

@@ -115,21 +115,11 @@ ServiceWorkerProcessManager::AllocateWorkerProcess(
       !storage_partition_->site_for_service_worker().is_empty();
   scoped_refptr<SiteInstanceImpl> site_instance =
       SiteInstanceImpl::CreateForServiceWorker(
-          browser_context_, use_url_from_storage_partition
-                                ? storage_partition_->site_for_service_worker()
-                                : script_url);
-
-  // Attempt to reuse a renderer process if possible. Note that in the
-  // <webview> case, process reuse isn't currently supported and a new
-  // process will always be created (https://crbug.com/752667).
-  DCHECK(site_instance->process_reuse_policy() ==
-             SiteInstanceImpl::ProcessReusePolicy::DEFAULT ||
-         site_instance->process_reuse_policy() ==
-             SiteInstanceImpl::ProcessReusePolicy::PROCESS_PER_SITE);
-  if (can_use_existing_process) {
-    site_instance->set_process_reuse_policy(
-        SiteInstanceImpl::ProcessReusePolicy::REUSE_PENDING_OR_COMMITTED_SITE);
-  }
+          browser_context_,
+          use_url_from_storage_partition
+              ? storage_partition_->site_for_service_worker()
+              : script_url,
+          can_use_existing_process);
 
   // Get the process from the SiteInstance.
   RenderProcessHost* rph = site_instance->GetProcess();

content/browser/site_instance_impl.cc

@@ -97,7 +97,8 @@ scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::CreateForURL(
 // static
 scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::CreateForServiceWorker(
     BrowserContext* browser_context,
-    const GURL& url) {
+    const GURL& url,
+    bool can_reuse_process) {
   // This will create a new SiteInstance and BrowsingInstance.
   scoped_refptr<BrowsingInstance> instance(
       new BrowsingInstance(browser_context));
@@ -107,6 +108,35 @@ scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::CreateForServiceWorker(
   scoped_refptr<SiteInstanceImpl> site_instance =
       instance->GetSiteInstanceForURL(url, /* allow_default_instance */ false);
   site_instance->is_for_service_worker_ = true;
+
+  // Attempt to reuse a renderer process if possible. Note that in the
+  // <webview> case, process reuse isn't currently supported and a new
+  // process will always be created (https://crbug.com/752667).
+  DCHECK(site_instance->process_reuse_policy() ==
+             SiteInstanceImpl::ProcessReusePolicy::DEFAULT ||
+         site_instance->process_reuse_policy() ==
+             SiteInstanceImpl::ProcessReusePolicy::PROCESS_PER_SITE);
+  if (can_reuse_process) {
+    site_instance->set_process_reuse_policy(
+        SiteInstanceImpl::ProcessReusePolicy::REUSE_PENDING_OR_COMMITTED_SITE);
+  }
+  return site_instance;
+}
+
+// static
+scoped_refptr<SiteInstanceImpl>
+SiteInstanceImpl::CreateReusableInstanceForTesting(
+    BrowserContext* browser_context,
+    const GURL& url) {
+  DCHECK(browser_context);
+  // This will create a new SiteInstance and BrowsingInstance.
+  scoped_refptr<BrowsingInstance> instance(
+      new BrowsingInstance(browser_context));
+  auto site_instance =
+      instance->GetSiteInstanceForURL(url,
+                                      /* allow_default_instance */ false);
+  site_instance->set_process_reuse_policy(
+      SiteInstanceImpl::ProcessReusePolicy::REUSE_PENDING_OR_COMMITTED_SITE);
   return site_instance;
 }
 

content/browser/site_instance_impl.h

@@ -41,8 +41,18 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
       BrowserContext* browser_context,
       const GURL& url);
   static scoped_refptr<SiteInstanceImpl> CreateForServiceWorker(
+      BrowserContext* browser_context,
+      const GURL& url,
+      bool can_reuse_process = false);
+
+  // Creates a SiteInstance for |url| like CreateForURL() would except the
+  // instance that is returned has its process_reuse_policy set to
+  // REUSE_PENDING_OR_COMMITTED_SITE and the default SiteInstance will never
+  // be returned.
+  static scoped_refptr<SiteInstanceImpl> CreateReusableInstanceForTesting(
       BrowserContext* browser_context,
       const GURL& url);
+
   static bool ShouldAssignSiteForURL(const GURL& url);
 
   // Returns whether |lock_url| is at least at the granularity of a site (i.e.,
@@ -106,6 +116,7 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
   };
 
   void set_process_reuse_policy(ProcessReusePolicy policy) {
+    DCHECK(!IsDefaultSiteInstance());
     process_reuse_policy_ = policy;
   }
   ProcessReusePolicy process_reuse_policy() const {