Remove IsDefaultSiteInstance() call from RenderFrameHostManager.

Removing an unnecessary call to IsDefaultSiteInstance() in
RenderFrameHostManager. The condition the call was in expects
RequiresDedicatedProcess() to return true on the SiteInstance which
won't happen on a default SiteInstance.

- Removed IsDefaultSiteInstance() call from RFHM.
- Added a test that explicitly verifies that a default SiteInstance
  returns fals for RequiresDedicatedProcess() to help prove the
  call was not needed and ensure that we don't accidentally change
  this behavior.
- Converted DCHECK in set_process_reuse_policy() to a check to make
  sure we don't accidentally allow reuse to be set on production builds.

Bug: 1085275
Change-Id: I628df05fe1b2eb27031f8bec087913867717e9a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538251
Commit-Queue: Aaron Colwell <acolwell@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Auto-Submit: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827481}

content/browser/renderer_host/render_frame_host_manager.cc

@@ -1631,7 +1631,6 @@ RenderFrameHostManager::GetSiteInstanceForNavigation(
   // dedicated process, set its process reuse policy so that such subframes are
   // consolidated into existing processes for that site.
   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

content/browser/site_instance_impl.h

@@ -363,7 +363,7 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
   };
 
   void set_process_reuse_policy(ProcessReusePolicy policy) {
-    DCHECK(!IsDefaultSiteInstance());
+    CHECK(!IsDefaultSiteInstance());
     process_reuse_policy_ = policy;
   }
   ProcessReusePolicy process_reuse_policy() const {

content/browser/site_instance_impl_unittest.cc

@@ -424,6 +424,40 @@ TEST_F(SiteInstanceTest, SiteInstanceDestructor) {
   // contents is now deleted, along with instance and browsing_instance
 }
 
+// Verifies some basic properties of default SiteInstances.
+TEST_F(SiteInstanceTest, DefaultSiteInstanceProperties) {
+  TestBrowserContext browser_context;
+
+  // Make sure feature list command-line options are set in a way that forces
+  // default SiteInstance creation on all platforms.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(
+      features::kProcessSharingWithDefaultSiteInstances);
+  EXPECT_TRUE(base::FeatureList::IsEnabled(
+      features::kProcessSharingWithDefaultSiteInstances));
+  EXPECT_FALSE(base::FeatureList::IsEnabled(
+      features::kProcessSharingWithStrictSiteInstances));
+
+  base::test::ScopedCommandLine scoped_command_line;
+  // Disable site isolation so we can get default SiteInstances on all
+  // platforms.
+  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
+      switches::kDisableSiteIsolation);
+
+  const auto cross_origin_isolation_info =
+      CoopCoepCrossOriginIsolatedInfo::CreateNonIsolated();
+  auto site_instance = SiteInstanceImpl::CreateForUrlInfo(
+      &browser_context, UrlInfo::CreateForTesting(GURL("http://foo.com")),
+      cross_origin_isolation_info);
+
+  EXPECT_TRUE(site_instance->IsDefaultSiteInstance());
+  EXPECT_TRUE(site_instance->HasSite());
+  EXPECT_EQ(
+      site_instance->GetSiteInfo(),
+      SiteInfo::CreateForDefaultSiteInstance(cross_origin_isolation_info));
+  EXPECT_FALSE(site_instance->RequiresDedicatedProcess());
+}
+
 // Ensure that default SiteInstances are deleted when all references to them
 // are gone.
 TEST_F(SiteInstanceTest, DefaultSiteInstanceDestruction) {