Update content/ tests to pass with ProcessSharingWithStrictSiteInstances

This patch updates test expectations so that they pass when the
ProcessSharingWithStrictSiteInstances process model is enabled instead
of ProcessSharingWithDefaultSiteInstances. Most of the changes just
rearrange the existing expectations to make it clearer which ones
apply to the default SiteInstance mode and which ones apply to
"strict SiteInstance" modes where we don't allow multiple sites to be
assigned to a single SiteInstance. Several helper functions were added
to make it easier to update expectations as the process model changes
in future CLs.

- Added AreStrictSiteInstancesEnabled(),and
  IsIsolatedOriginRequiredToGuaranteeDedicatedProcess() helper functions
  to make it easier to express differences in test expectations.
- Updated TestWebContents::CrossProcessNavigationPending() to actually
  verify that the navigation is cross process instead of just verifying
  whether a speculative RenderFrameHost was created. This is needed for
  ProcessSharingWithDefaultSiteInstances because this mode can trigger
  speculative RenderFrameHosts creation for a navigation that will
  stay in the same process.
- Updated many test expectations that compare SiteInstance pointers.
  Many cases only should expect pointers to match when the default
  SiteInstance mode is enabled. Logic related to these comparisons were
  reorganized to make this more clear.
- Updated several tests to explicitly isolate origins when
  ProcessSharingWithStrictSiteInstances is enabled, just like they are
  for ProcessSharingWithDefaultSiteInstances. This is needed because
  these two modes might allow sites to share a process when the tests
  explicitly want them to have dedicated processes.

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

content/browser/back_forward_cache_browsertest.cc

@@ -2528,7 +2528,7 @@ IN_PROC_BROWSER_TEST_F(
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 
-  if (AreAllSitesIsolatedForTesting()) {
+  if (AreStrictSiteInstancesEnabled()) {
     ExpectNotRestored(
         {BackForwardCacheMetrics::NotRestoredReason::
              kRelatedActiveContentsExist,
@@ -4240,8 +4240,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, CacheHTTPDocumentOnly) {
     // On Android, navigations to about:blank keeps the same RenderFrameHost.
     // Obviously, it can't enter the BackForwardCache, because it is still used
     // to display the current document.
-    if (test_case.url == blank_url &&
-        !SiteIsolationPolicy::UseDedicatedProcessesForAllSites()) {
+    if (test_case.url == blank_url && !AreStrictSiteInstancesEnabled()) {
       EXPECT_FALSE(delete_observer.deleted());
       EXPECT_FALSE(rfh->IsInBackForwardCache());
       EXPECT_EQ(rfh, current_frame_host());
@@ -5060,13 +5059,13 @@ IN_PROC_BROWSER_TEST_F(
   RenderFrameHostImpl* rfh_a1 = current_frame_host();
   RenderFrameHostImpl* rfh_b = rfh_a1->child_at(0)->current_frame_host();
   EXPECT_TRUE(ExecJs(rfh_b, "window.onunload = () => {} "));
-  EXPECT_EQ(AreAllSitesIsolatedForTesting(),
+  EXPECT_EQ(AreStrictSiteInstancesEnabled(),
             rfh_a1->GetSiteInstance() != rfh_b->GetSiteInstance());
 
   // 2) Navigate to A2.
   EXPECT_TRUE(NavigateToURL(shell(), url_a2));
   RenderFrameHostImpl* rfh_a2 = current_frame_host();
-  if (AreAllSitesIsolatedForTesting()) {
+  if (AreStrictSiteInstancesEnabled()) {
     // We should swap RFH & BIs and A1 should be in the back-forward cache.
     EXPECT_NE(rfh_a1, rfh_a2);
     EXPECT_FALSE(rfh_a1->GetSiteInstance()->IsRelatedSiteInstance(

content/browser/isolated_origin_browsertest.cc

@@ -2019,14 +2019,24 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest,
             child->current_frame_host()->GetSiteInstance()->GetSiteURL());
 
   // Navigate the child frame cross-site, but to a non-isolated origin. When
-  // not in --site-per-process, this should bring the subframe back into the
-  // main frame's SiteInstance.
+  // strict SiteInstaces are not enabled, this should bring the subframe back
+  // into the main frame's SiteInstance. If strict SiteInstances are enabled,
+  // we expect the SiteInstances to be different because a SiteInstance is not
+  // allowed to contain multiple sites in that mode. In all cases though we
+  // expect the navigation to end up in the same process.
   GURL bar_url(embedded_test_server()->GetURL("bar.com", "/title1.html"));
   EXPECT_FALSE(IsIsolatedOrigin(bar_url));
   NavigateIframeToURL(web_contents(), "test_iframe", bar_url);
-  EXPECT_EQ(web_contents()->GetSiteInstance(),
-            child->current_frame_host()->GetSiteInstance());
-  EXPECT_FALSE(child->current_frame_host()->IsCrossProcessSubframe());
+
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(web_contents()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  } else {
+    EXPECT_EQ(web_contents()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  }
+  EXPECT_EQ(web_contents()->GetSiteInstance()->GetProcess(),
+            child->current_frame_host()->GetSiteInstance()->GetProcess());
 }
 
 // Check that a new isolated origin subframe will attempt to reuse an existing
@@ -2125,12 +2135,13 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest,
   FrameTreeNode* child = root->child_at(0);
 
   // Navigate iframe cross-site, but not to an isolated origin.  This should
-  // stay in the main frame's SiteInstance, unless we're in --site-per-process
-  // mode.  (Note that the bug for which this test is written is exclusive to
-  // --isolate-origins and does not happen with --site-per-process.)
+  // stay in the main frame's SiteInstance, unless we're in a strict
+  // SiteInstance mode (including --site-per-process). (Note that the bug for
+  // which this test is written is exclusive to --isolate-origins and does not
+  // happen with --site-per-process.)
   GURL bar_url(embedded_test_server()->GetURL("bar.com", "/title1.html"));
   NavigateIframeToURL(web_contents(), "test_iframe", bar_url);
-  if (AreAllSitesIsolatedForTesting()) {
+  if (AreStrictSiteInstancesEnabled()) {
     EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
               child->current_frame_host()->GetSiteInstance());
   } else {
@@ -2236,8 +2247,7 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest,
     // At this point, the popup and the opener should still be in separate
     // SiteInstances.
     EXPECT_NE(newshell_site_instance_impl, root_site_instance_impl);
-    EXPECT_NE(AreAllSitesIsolatedForTesting(),
-              newshell_site_instance_impl->IsDefaultSiteInstance());
+    EXPECT_FALSE(newshell_site_instance_impl->IsDefaultSiteInstance());
     EXPECT_FALSE(root_site_instance_impl->IsDefaultSiteInstance());
   }
 
@@ -3060,7 +3070,11 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginLongListTest, Test) {
   if (!AreAllSitesIsolatedForTesting()) {
     EXPECT_EQ(main_frame->GetProcess()->GetID(),
               subframe3->GetProcess()->GetID());
-    EXPECT_EQ(main_frame->GetSiteInstance(), subframe3->GetSiteInstance());
+    if (AreStrictSiteInstancesEnabled()) {
+      EXPECT_NE(main_frame->GetSiteInstance(), subframe3->GetSiteInstance());
+    } else {
+      EXPECT_EQ(main_frame->GetSiteInstance(), subframe3->GetSiteInstance());
+    }
   }
 
   // isolated.foo.com and foo999.com are on the list of origins to isolate -
@@ -3115,7 +3129,7 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, SubframeErrorPages) {
     observer.Wait();
     EXPECT_EQ(child2->current_url(), regular_url);
     EXPECT_TRUE(handle_observer.is_error());
-    if (AreAllSitesIsolatedForTesting()) {
+    if (AreStrictSiteInstancesEnabled()) {
       EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
                 child2->current_frame_host()->GetSiteInstance());
       EXPECT_EQ(SiteInstance::GetSiteForURL(web_contents()->GetBrowserContext(),
@@ -3215,15 +3229,11 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, AIsolatedCA) {
 
     EXPECT_TRUE(HasDefaultSiteInstance(a));
     EXPECT_FALSE(HasDefaultSiteInstance(b));
-  } else {
-    // Documenting current behavior where the top level document doesn't end
-    // up in a default SiteInstance even though it is not isolated and does not
-    // require a dedicated process. c.com does get placed in a default
-    // SiteInstance because we currently allow subframes that don't require
-    // isolation to share a process. This behavior should go away once we
-    // turn on default SiteInstances by default.
+  } else if (AreStrictSiteInstancesEnabled()) {
+    // All sites have their own SiteInstance and sites that are not isolated
+    // are all placed in the same process.
     EXPECT_NE(a->GetProcess()->GetID(), b->GetProcess()->GetID());
-    EXPECT_NE(a->GetProcess()->GetID(), c->GetProcess()->GetID());
+    EXPECT_EQ(a->GetProcess()->GetID(), c->GetProcess()->GetID());
 
     EXPECT_NE(a->GetSiteInstance(), b->GetSiteInstance());
     EXPECT_NE(a->GetSiteInstance(), c->GetSiteInstance());
@@ -3232,7 +3242,9 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, AIsolatedCA) {
 
     EXPECT_FALSE(HasDefaultSiteInstance(a));
     EXPECT_FALSE(HasDefaultSiteInstance(b));
-    EXPECT_TRUE(HasDefaultSiteInstance(c));
+    EXPECT_FALSE(HasDefaultSiteInstance(c));
+  } else {
+    FAIL() << "Unexpected process model configuration.";
   }
 }
 
@@ -3466,12 +3478,15 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest,
 
   // The two frames should be in the same process, since neither site is
   // isolated so far.
-  if (!AreAllSitesIsolatedForTesting()) {
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  } else {
     EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
               child->current_frame_host()->GetSiteInstance());
-    EXPECT_EQ(root->current_frame_host()->GetProcess(),
-              child->current_frame_host()->GetProcess());
   }
+  EXPECT_EQ(root->current_frame_host()->GetProcess(),
+            child->current_frame_host()->GetProcess());
 
   // Start isolating foo.com.
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
@@ -3493,12 +3508,23 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest,
       "bar.com", "/cross_site_iframe_factory.html?bar.com(foo.com)"));
   NavigateIframeToURL(web_contents(), "test_iframe", bar_with_foo_url);
   FrameTreeNode* grandchild = child->child_at(0);
-  EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
-            child->current_frame_host()->GetSiteInstance());
-  EXPECT_EQ(child->current_frame_host()->GetSiteInstance(),
-            grandchild->current_frame_host()->GetSiteInstance());
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+    EXPECT_NE(child->current_frame_host()->GetSiteInstance(),
+              grandchild->current_frame_host()->GetSiteInstance());
+  } else {
+    EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+    EXPECT_EQ(child->current_frame_host()->GetSiteInstance(),
+              grandchild->current_frame_host()->GetSiteInstance());
+  }
   EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
             grandchild->current_frame_host()->GetSiteInstance());
+  EXPECT_EQ(root->current_frame_host()->GetProcess(),
+            child->current_frame_host()->GetProcess());
+  EXPECT_EQ(child->current_frame_host()->GetProcess(),
+            grandchild->current_frame_host()->GetProcess());
 
   // Create an unrelated window, which will be in a new BrowsingInstance.
   // Ensure that foo.com becomes an isolated origin in that window.  A
@@ -3773,8 +3799,14 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest,
   EXPECT_EQ(child->current_url(), bar_url);
 
   // The iframe should not be in an OOPIF yet.
-  EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
-            child->current_frame_host()->GetSiteInstance());
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+
+  } else {
+    EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  }
   EXPECT_EQ(root->current_frame_host()->GetProcess(),
             child->current_frame_host()->GetProcess());
 
@@ -3959,8 +3991,13 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest, PerProfileIsolation) {
   root = web_contents()->GetFrameTree()->root();
   child = root->child_at(0);
   EXPECT_EQ(child->current_url(), bar_url);
-  EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
-            child->current_frame_host()->GetSiteInstance());
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  } else {
+    EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),
+              child->current_frame_host()->GetSiteInstance());
+  }
   EXPECT_EQ(root->current_frame_host()->GetProcess(),
             child->current_frame_host()->GetProcess());
 }
@@ -3982,7 +4019,12 @@ IN_PROC_BROWSER_TEST_F(DynamicIsolatedOriginTest, ForceBrowsingInstanceSwap) {
   FrameTreeNode* child = root->child_at(0);
   scoped_refptr<SiteInstance> first_instance =
       root->current_frame_host()->GetSiteInstance();
-  EXPECT_EQ(first_instance, child->current_frame_host()->GetSiteInstance());
+
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(first_instance, child->current_frame_host()->GetSiteInstance());
+  } else {
+    EXPECT_EQ(first_instance, child->current_frame_host()->GetSiteInstance());
+  }
   EXPECT_EQ(root->current_frame_host()->GetProcess(),
             child->current_frame_host()->GetProcess());
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();

content/browser/navigation_browsertest.cc

@@ -501,13 +501,13 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest,
     EXPECT_EQ(initiator_process_id, observer.last_initiator_process_id());
   }
 
-  // The RenderFrameHost should not have changed unless site-per-process or
-  // proactive BrowsingInstance swap is enabled.
-  if (AreAllSitesIsolatedForTesting() ||
-      CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) {
-    EXPECT_NE(initial_rfh, current_frame_host());
-  } else {
+  // The RenderFrameHost should have changed unless default SiteInstances
+  // are enabled and proactive BrowsingInstance swaps are disabled.
+  if (AreDefaultSiteInstancesEnabled() &&
+      !CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) {
     EXPECT_EQ(initial_rfh, current_frame_host());
+  } else {
+    EXPECT_NE(initial_rfh, current_frame_host());
   }
 }
 

content/browser/renderer_host/navigation_controller_impl_browsertest.cc

@@ -4593,7 +4593,7 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
   FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
                             ->GetFrameTree()
                             ->root();
-  SiteInstance* main_site_instance =
+  SiteInstanceImpl* main_site_instance =
       root->current_frame_host()->GetSiteInstance();
 
   // The main frame defaults to an empty name.
@@ -4656,14 +4656,15 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
   EXPECT_TRUE(NavigateToURLFromRenderer(foo_subframe, bar_url));
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 
-  // When run just with subframe navigation entries enabled and not in
-  // site-per-process-mode the subframe should be in the same SiteInstance as
-  // its parent.
-  if (!AreAllSitesIsolatedForTesting()) {
-    EXPECT_EQ(main_site_instance,
+  if (AreStrictSiteInstancesEnabled()) {
+    EXPECT_NE(main_site_instance,
               foo_subframe->current_frame_host()->GetSiteInstance());
   } else {
-    EXPECT_NE(main_site_instance,
+    // When strict SiteInstances are not being used, the subframe should be
+    // the same as its parent because both sites get routed to the default
+    // SiteInstance.
+    EXPECT_TRUE(main_site_instance->IsDefaultSiteInstance());
+    EXPECT_EQ(main_site_instance,
               foo_subframe->current_frame_host()->GetSiteInstance());
   }
 
@@ -5518,12 +5519,16 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
   EXPECT_EQ(main_url_a, new_root->current_url());
   EXPECT_EQ(frame_url_b, new_root->child_at(0)->current_url());
 
-  // The subframe should only be in a different SiteInstance if OOPIFs are
-  // required for all sites.
-  if (AreAllSitesIsolatedForTesting()) {
+  if (AreStrictSiteInstancesEnabled()) {
     EXPECT_NE(new_root->current_frame_host()->GetSiteInstance(),
               new_root->child_at(0)->current_frame_host()->GetSiteInstance());
   } else {
+    // When strict SiteInstances are not enabled, the subframe should be in the
+    // same SiteInstance as the parent because both sites get mapped to the
+    // default SiteInstance.
+    EXPECT_TRUE(new_root->current_frame_host()
+                    ->GetSiteInstance()
+                    ->IsDefaultSiteInstance());
     EXPECT_EQ(new_root->current_frame_host()->GetSiteInstance(),
               new_root->child_at(0)->current_frame_host()->GetSiteInstance());
   }
@@ -5960,9 +5965,9 @@ class RenderProcessKilledObserver : public WebContentsObserver {
 // not modify the underlying last committed entry.)  Not crashing means that
 // the test is successful.
 IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, ReloadOriginalRequest) {
-  // TODO(lukasza): https://crbug.com/417518: Get tests working with
-  // --site-per-process.
-  if (SiteIsolationPolicy::UseDedicatedProcessesForAllSites() ||
+  // TODO(lukasza): https://crbug.com/1159466: Get tests working for all
+  // process model modes.
+  if (AreStrictSiteInstancesEnabled() ||
       CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) {
     return;
   }