diff --git a/content/browser/back_forward_cache_browsertest.cc b/content/browser/back_forward_cache_browsertest.cc index 5f5a017a5908f4..8f74ac741a33e6 100644 --- a/content/browser/back_forward_cache_browsertest.cc +++ b/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( diff --git a/content/browser/isolated_origin_browsertest.cc b/content/browser/isolated_origin_browsertest.cc index da27a0335a2c88..88731fd4db8d14 100644 --- a/content/browser/isolated_origin_browsertest.cc +++ b/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 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(); diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc index 5c4ff95a345ef3..f6add29f1614a0 100644 --- a/content/browser/navigation_browsertest.cc +++ b/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()); } } diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc index e168413aeb2c10..fd5f07f6ee9b1e 100644 --- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc +++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc @@ -4593,7 +4593,7 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, FrameTreeNode* root = static_cast(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; } diff --git a/content/browser/renderer_host/navigator_unittest.cc b/content/browser/renderer_host/navigator_unittest.cc index a1432ab3bd0ed1..0ba339241fec82 100644 --- a/content/browser/renderer_host/navigator_unittest.cc +++ b/content/browser/renderer_host/navigator_unittest.cc @@ -40,6 +40,19 @@ namespace content { +namespace { + +// Helper function that determines if a test should expect a cross-site +// navigation to trigger a SiteInstance change based on the current process +// model. +bool ExpectSiteInstanceChange(SiteInstanceImpl* site_instance) { + return AreAllSitesIsolatedForTesting() || + CanCrossSiteNavigationsProactivelySwapBrowsingInstances() || + !site_instance->IsDefaultSiteInstance(); +} + +} // namespace + class NavigatorTest : public RenderViewHostImplTestHarness { public: using SiteInstanceDescriptor = RenderFrameHostManager::SiteInstanceDescriptor; @@ -190,7 +203,10 @@ TEST_F(NavigatorTest, SimpleRendererInitiatedCrossSiteNavigation) { contents()->NavigateAndCommit(kUrl1); EXPECT_TRUE(main_test_rfh()->IsRenderFrameLive()); - int32_t site_instance_id_1 = main_test_rfh()->GetSiteInstance()->GetId(); + scoped_refptr site_instance_1 = + main_test_rfh()->GetSiteInstance(); + bool expect_site_instance_change = + ExpectSiteInstanceChange(site_instance_1.get()); // Start a renderer-initiated navigation. EXPECT_FALSE(main_test_rfh()->navigation_request()); @@ -206,8 +222,7 @@ TEST_F(NavigatorTest, SimpleRendererInitiatedCrossSiteNavigation) { EXPECT_EQ(NavigationRequest::WILL_START_REQUEST, request->state()); EXPECT_EQ(kUrl2, request->common_params().url); EXPECT_FALSE(request->browser_initiated()); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -215,8 +230,7 @@ TEST_F(NavigatorTest, SimpleRendererInitiatedCrossSiteNavigation) { // Have the current RenderFrameHost commit the navigation. navigation->ReadyToCommit(); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_EQ(navigation->GetFinalRenderFrameHost(), GetSpeculativeRenderFrameHost(node)); } @@ -228,12 +242,14 @@ TEST_F(NavigatorTest, SimpleRendererInitiatedCrossSiteNavigation) { EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL()); EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); - // The SiteInstance did not change unless site-per-process is enabled. - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { - EXPECT_NE(site_instance_id_1, main_test_rfh()->GetSiteInstance()->GetId()); + if (expect_site_instance_change) { + EXPECT_NE(site_instance_1->GetId(), + main_test_rfh()->GetSiteInstance()->GetId()); + EXPECT_EQ(site_instance_1->IsDefaultSiteInstance(), + main_test_rfh()->GetSiteInstance()->IsDefaultSiteInstance()); } else { - EXPECT_EQ(site_instance_id_1, main_test_rfh()->GetSiteInstance()->GetId()); + EXPECT_EQ(site_instance_1->GetId(), + main_test_rfh()->GetSiteInstance()->GetId()); } } @@ -315,9 +331,11 @@ TEST_F(NavigatorTest, BeginNavigation) { EXPECT_FALSE(GetSpeculativeRenderFrameHost(root_node)); // Subframe navigations should never create a speculative RenderFrameHost, - // unless site-per-process is enabled. In that case, as the subframe - // navigation is to a different site and is still ongoing, it should have one. - if (AreAllSitesIsolatedForTesting()) { + // unless site-per-process or ProcessSharingWithStrictSiteInstances is + // enabled. In that case, as the subframe navigation is to a different site + // and is still ongoing, it should have one. + bool expect_site_instance_change = AreStrictSiteInstancesEnabled(); + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node)); @@ -359,7 +377,7 @@ TEST_F(NavigatorTest, BeginNavigation) { // As the main frame hasn't yet committed the subframe still exists. Thus, the // above situation regarding subframe navigations is valid here. - if (AreAllSitesIsolatedForTesting()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node)); @@ -598,6 +616,8 @@ TEST_F(NavigatorTest, RendererUserInitiatedNavigationCancel) { // Initialization. contents()->NavigateAndCommit(kUrl0); FrameTreeNode* node = main_test_rfh()->frame_tree_node(); + bool expect_site_instance_change = + ExpectSiteInstanceChange(main_test_rfh()->GetSiteInstance()); // Start a browser-initiated navigation to the 1st URL and invoke its // beforeUnload completion callback. @@ -634,8 +654,7 @@ TEST_F(NavigatorTest, RendererUserInitiatedNavigationCancel) { // Confirm that the speculative RenderFrameHost was destroyed in the non // SitePerProcess case. - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -661,6 +680,8 @@ TEST_F(NavigatorTest, // Initialization. contents()->NavigateAndCommit(kUrl0); FrameTreeNode* node = main_test_rfh()->frame_tree_node(); + bool expect_site_instance_change = + ExpectSiteInstanceChange(main_test_rfh()->GetSiteInstance()); // Start a renderer-initiated user-initiated navigation to the 1st URL. EXPECT_FALSE(main_test_rfh()->navigation_request()); @@ -674,8 +695,7 @@ TEST_F(NavigatorTest, EXPECT_EQ(kUrl1, request1->common_params().url); EXPECT_FALSE(request1->browser_initiated()); EXPECT_TRUE(request1->common_params().has_user_gesture); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -695,8 +715,7 @@ TEST_F(NavigatorTest, EXPECT_EQ(kUrl2, request2->common_params().url); EXPECT_FALSE(request2->browser_initiated()); EXPECT_FALSE(request2->common_params().has_user_gesture); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -762,6 +781,8 @@ TEST_F(NavigatorTest, contents()->NavigateAndCommit(kUrl0); FrameTreeNode* node = main_test_rfh()->frame_tree_node(); int32_t site_instance_id_0 = main_test_rfh()->GetSiteInstance()->GetId(); + bool expect_site_instance_change = + ExpectSiteInstanceChange(main_test_rfh()->GetSiteInstance()); // Start a renderer-initiated non-user-initiated navigation to the 1st URL. EXPECT_FALSE(main_test_rfh()->navigation_request()); @@ -776,8 +797,7 @@ TEST_F(NavigatorTest, EXPECT_EQ(kUrl1, request1->common_params().url); EXPECT_FALSE(request1->browser_initiated()); EXPECT_FALSE(request1->common_params().has_user_gesture); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -797,8 +817,7 @@ TEST_F(NavigatorTest, EXPECT_EQ(kUrl2, request2->common_params().url); EXPECT_FALSE(request2->browser_initiated()); EXPECT_FALSE(request2->common_params().has_user_gesture); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(node)); @@ -811,9 +830,7 @@ TEST_F(NavigatorTest, navigation2->Commit(); EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL()); - // The SiteInstance did not change unless site-per-process is enabled. - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (expect_site_instance_change) { EXPECT_NE(site_instance_id_0, main_test_rfh()->GetSiteInstance()->GetId()); } else { EXPECT_EQ(site_instance_id_0, main_test_rfh()->GetSiteInstance()->GetId()); @@ -1042,7 +1059,7 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) { ChildProcessSecurityPolicy::IsolatedOriginSource::TEST, browser_context()); contents()->NavigateAndCommit(kUrl1); - SiteInstance* current_instance = main_test_rfh()->GetSiteInstance(); + SiteInstanceImpl* current_instance = main_test_rfh()->GetSiteInstance(); ASSERT_TRUE(current_instance); // 1) Convert a descriptor pointing to the current instance. @@ -1101,12 +1118,15 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) { EXPECT_NE(current_instance, related_instance.get()); EXPECT_NE(unrelated_instance.get(), related_instance.get()); - if (AreAllSitesIsolatedForTesting()) { - EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrlSameSiteAs2), - related_instance->GetSiteURL()); + auto* related_instance_impl = + static_cast(related_instance.get()); + + if (AreDefaultSiteInstancesEnabled()) { + ASSERT_TRUE(related_instance_impl->IsDefaultSiteInstance()); } else { - EXPECT_TRUE(static_cast(related_instance.get()) - ->IsDefaultSiteInstance()); + EXPECT_EQ(SiteInstanceImpl::ComputeSiteInfoForTesting( + current_instance->GetIsolationContext(), kUrlSameSiteAs2), + related_instance_impl->GetSiteInfo()); } } diff --git a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc index e1673753110983..13bc06dd46d18f 100644 --- a/content/browser/renderer_host/render_frame_host_impl_browsertest.cc +++ b/content/browser/renderer_host/render_frame_host_impl_browsertest.cc @@ -791,14 +791,14 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBeforeUnloadBrowserTest, EXPECT_EQ(main_frame, child->GetBeforeUnloadInitiator()); EXPECT_EQ(main_frame, main_frame->GetBeforeUnloadInitiator()); - // When in --site-per-process mode, LoadURL() should trigger two beforeunload - // IPCs for subframe and the main frame: the subframe has a beforeunload - // handler, and while the main frame does not, we always send the IPC to - // navigating frames, regardless of whether or not they have a handler. + // When in a strict SiteInstances mode, LoadURL() should trigger two + // beforeunload IPCs for subframe and the main frame: the subframe has a + // beforeunload handler, and while the main frame does not, we always send the + // IPC to navigating frames, regardless of whether or not they have a handler. // - // Without --site-per-process, only one beforeunload IPC should be sent to + // Without strict SiteInstances, only one beforeunload IPC should be sent to // the main frame, which will handle both (same-process) frames. - EXPECT_EQ(AreAllSitesIsolatedForTesting() ? 2u : 1u, + EXPECT_EQ(AreStrictSiteInstancesEnabled() ? 2u : 1u, main_frame->beforeunload_pending_replies_.size()); // Wait for the beforeunload dialog to be shown from the subframe. @@ -811,12 +811,12 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBeforeUnloadBrowserTest, EXPECT_TRUE(main_frame->is_waiting_for_beforeunload_completion()); EXPECT_FALSE(child->is_waiting_for_beforeunload_completion()); - // In --site-per-process mode, the beforeunload completion callback should - // happen on the child RFH. Without --site-per-process, it will come from the - // main frame RFH, which processes beforeunload for both main frame and child - // frame, since they are in the same process. + // In a strict SiteInstances mode, the beforeunload completion callback should + // happen on the child RFH. Without strict SiteInstances, it will come from + // the main frame RFH, which processes beforeunload for both main frame and + // child frame, since they are in the same process and SiteInstance. RenderFrameHostImpl* frame_that_sent_beforeunload_ipc = - AreAllSitesIsolatedForTesting() ? child : main_frame; + AreStrictSiteInstancesEnabled() ? child : main_frame; EXPECT_TRUE(main_frame->beforeunload_pending_replies_.count( frame_that_sent_beforeunload_ipc)); diff --git a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc index da9f87698eb14b..6c151b9123522e 100644 --- a/content/browser/renderer_host/render_frame_host_manager_browsertest.cc +++ b/content/browser/renderer_host/render_frame_host_manager_browsertest.cc @@ -90,6 +90,13 @@ namespace content { namespace { +// Helper function that return true in cases where the current process model +// will return the same SiteInstance for a cross-process navigation. +bool ExpectSameSiteInstance() { + return AreDefaultSiteInstancesEnabled() && + !CanCrossSiteNavigationsProactivelySwapBrowsingInstances(); +} + const char kOpenUrlViaClickTargetFunc[] = "(function(url) {\n" " var lnk = document.createElement(\"a\");\n" @@ -680,10 +687,10 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, // Should have the same SiteInstance unless we're in site-per-process mode. scoped_refptr blank_site_instance( new_shell->web_contents()->GetSiteInstance()); - if (AreAllSitesIsolatedForTesting()) - EXPECT_NE(orig_site_instance, blank_site_instance); - else + if (AreDefaultSiteInstancesEnabled()) EXPECT_EQ(orig_site_instance, blank_site_instance); + else + EXPECT_NE(orig_site_instance, blank_site_instance); } // Test for crbug.com/24447. Following a cross-site link with rel=noreferrer @@ -715,14 +722,12 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, EXPECT_EQ("/title2.html", shell()->web_contents()->GetLastCommittedURL().path()); - // Should have the same SiteInstance unless we're in site-per-process mode. scoped_refptr noref_site_instance( shell()->web_contents()->GetSiteInstance()); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { - EXPECT_NE(orig_site_instance, noref_site_instance); - } else { + if (ExpectSameSiteInstance()) { EXPECT_EQ(orig_site_instance, noref_site_instance); + } else { + EXPECT_NE(orig_site_instance, noref_site_instance); } } @@ -754,14 +759,12 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, EXPECT_EQ("/title2.html", shell()->web_contents()->GetLastCommittedURL().path()); - // Should have the same SiteInstance unless we're in site-per-process mode. scoped_refptr noref_site_instance( shell()->web_contents()->GetSiteInstance()); - if (AreAllSitesIsolatedForTesting() || - CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { - EXPECT_NE(orig_site_instance, noref_site_instance); - } else { + if (ExpectSameSiteInstance()) { EXPECT_EQ(orig_site_instance, noref_site_instance); + } else { + EXPECT_NE(orig_site_instance, noref_site_instance); } } @@ -840,7 +843,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, MAYBE_DisownOpener) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed to get a non-default // SiteInstance for navigations to this origin. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -940,7 +943,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, DisownSubframeOpener) { IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, PreserveTopFrameWindowNameOnCrossProcessNavigations) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed it is placed in a different // process. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -1008,7 +1011,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, SupportCrossProcessPostMessage) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed it is placed in a different // process. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -1149,7 +1152,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, SupportCrossProcessPostMessageWithMessagePort) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed it is placed in a different // process. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -1239,7 +1242,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, AllowTargetedNavigationsInOpenerAfterSwap) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed to get a non-default // SiteInstance for navigations to this origin. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -1346,7 +1349,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, ProcessExitWithSwappedOutViews) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed to get a non-default // SiteInstance for navigations to this origin. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -2349,7 +2352,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, DontPreemptNavigationWithFrameTreeUpdate) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed to get a non-default // SiteInstance for navigations to this origin. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -3080,7 +3083,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, ReinitializeOpenerChainAfterCrashAndReload) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed to get a non-default // SiteInstance for navigations to this origin. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -3149,7 +3152,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, // the other popup, and ensure that the opener is updated in all processes. IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, UpdateOpener) { StartEmbeddedServer(); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "foo.com" so we are guaranteed it is placed in a different // process. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), @@ -3921,9 +3924,9 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, CoReferencingFrames) { // The FrameTree contains two successful instances of each site plus an // unsuccessfully-navigated third instance of B with a blank URL. When not in - // site-per-process mode, the FrameTreeVisualizer depicts all nodes as + // strict SiteInstance mode, the FrameTreeVisualizer depicts all nodes as // referencing Site A because iframes are identified with their root site. - if (AreAllSitesIsolatedForTesting()) { + if (AreStrictSiteInstancesEnabled()) { EXPECT_EQ( " Site A ------------ proxies for B\n" " +--Site B ------- proxies for A\n" @@ -4379,8 +4382,7 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, // Since this is a browser-initiated, cross-site navigation, it will swap // BrowsingInstances, and create a new foo.com SiteInstance, distinct from // the initial one. - if (!AreAllSitesIsolatedForTesting() && - !CanCrossSiteNavigationsProactivelySwapBrowsingInstances()) { + if (ExpectSameSiteInstance()) { EXPECT_EQ(site_instance, shell()->web_contents()->GetSiteInstance()); } else { EXPECT_NE(site_instance, shell()->web_contents()->GetSiteInstance()); @@ -4785,10 +4787,10 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, ErrorPageNavigationReload_InSubframe_NetworkError) { StartEmbeddedServer(); - // Isolating a.com helps more robustly exercise platforms without strict site - // isolation - we want to ensure that enforcing |initiator_origin| in - // BeginNavigation is compatible with process locks, even when only one of the - // frames requires isolation. + // Isolating a.com helps more robustly exercise platforms without strict + // site isolation - we want to ensure that enforcing |initiator_origin| in + // BeginNavigation is compatible with process locks, even when only one of + // the frames requires isolation. IsolateOriginsForTesting(embedded_test_server(), shell()->web_contents(), {"b.com"}); @@ -4953,11 +4955,11 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, child1->current_frame_host()->GetSiteInstance(); GURL c_site_url = child1_site_instance->GetSiteURL(); - if (AreAllSitesIsolatedForTesting()) { + if (AreDefaultSiteInstancesEnabled()) { + EXPECT_TRUE(child1_site_instance->IsDefaultSiteInstance()); + } else { EXPECT_EQ("c.com", c_site_url.host()); EXPECT_EQ(test_url.host(), c_site_url.host()); - } else { - EXPECT_TRUE(child1_site_instance->IsDefaultSiteInstance()); } EXPECT_NE(a_site_url, c_site_url); EXPECT_NE(b_site_url, c_site_url); @@ -5749,13 +5751,15 @@ IN_PROC_BROWSER_TEST_P( static_cast( web_contents->GetMainFrame()->GetSiteInstance()); - // Check that A and B are in different BrowsingInstances (both are in default - // SiteInstances of different BrowsingInstances) but have the same renderer - // process. + // Check that A and B are in different BrowsingInstances but have the same + // renderer process. When default SiteInstances are enabled, A and B are + // both default SiteInstances of different BrowsingInstances. EXPECT_FALSE(a_site_instance->IsRelatedSiteInstance(b_site_instance.get())); - EXPECT_TRUE(a_site_instance->IsDefaultSiteInstance()); - EXPECT_TRUE(b_site_instance->IsDefaultSiteInstance()); EXPECT_EQ(a_site_instance->GetProcess(), b_site_instance->GetProcess()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + a_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + b_site_instance->IsDefaultSiteInstance()); } // Different from renderer-initiated cross-site navigations, browser-initiated @@ -5784,12 +5788,15 @@ IN_PROC_BROWSER_TEST_P( static_cast( web_contents->GetMainFrame()->GetSiteInstance()); - // Check that A and B are in different BrowsingInstances (both are in default - // SiteInstances of different BrowsingInstances) and renderer processes. + // Check that A and B are in different BrowsingInstances and renderer + // processes. When default SiteInstances are enabled, A and B are + // both default SiteInstances of different BrowsingInstances. EXPECT_FALSE(a_site_instance->IsRelatedSiteInstance(b_site_instance.get())); - EXPECT_TRUE(a_site_instance->IsDefaultSiteInstance()); - EXPECT_TRUE(b_site_instance->IsDefaultSiteInstance()); EXPECT_NE(a_site_instance->GetProcess(), b_site_instance->GetProcess()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + a_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + b_site_instance->IsDefaultSiteInstance()); } // A test ContentBrowserClient implementation that enforce process-per-site mode @@ -5844,13 +5851,14 @@ IN_PROC_BROWSER_TEST_P( static_cast( web_contents->GetMainFrame()->GetSiteInstance()); - // Check that A and B are in different BrowsingInstances (both are in default - // SiteInstances of different BrowsingInstances) but have the same renderer - // process. + // Check that A and B are in different BrowsingInstances but have the same + // renderer process. EXPECT_FALSE(a_site_instance->IsRelatedSiteInstance(b_site_instance.get())); - EXPECT_TRUE(a_site_instance->IsDefaultSiteInstance()); - EXPECT_TRUE(b_site_instance->IsDefaultSiteInstance()); EXPECT_EQ(b_site_instance->GetProcess(), original_process); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + a_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + b_site_instance->IsDefaultSiteInstance()); // Make sure we will use process-per-site for C. // Note this is enforcing process-per-site for all sites, which is why we turn @@ -5864,10 +5872,11 @@ IN_PROC_BROWSER_TEST_P( static_cast( web_contents->GetMainFrame()->GetSiteInstance()); - // Check that B and C are in different BrowsingInstances (both are in default - // SiteInstances of different BrowsingInstances) and renderer processes. + // Check that B and C are in different BrowsingInstances and renderer + // processes. EXPECT_FALSE(b_site_instance->IsRelatedSiteInstance(c_site_instance.get())); - EXPECT_TRUE(c_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + c_site_instance->IsDefaultSiteInstance()); EXPECT_NE(c_site_instance->GetProcess(), original_process); // C is using the process for C's site. EXPECT_EQ(c_site_instance->GetProcess(), @@ -5885,7 +5894,8 @@ IN_PROC_BROWSER_TEST_P( web_contents->GetMainFrame()->GetSiteInstance()); EXPECT_FALSE(b2_site_instance->IsRelatedSiteInstance(c_site_instance.get())); EXPECT_FALSE(b2_site_instance->IsRelatedSiteInstance(b_site_instance.get())); - EXPECT_TRUE(b2_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + b2_site_instance->IsDefaultSiteInstance()); EXPECT_NE(b2_site_instance->GetProcess(), original_process); // B will reuse C's process here, even though C is process-per-site, because // neither of them require a dedicated process. @@ -5941,12 +5951,13 @@ IN_PROC_BROWSER_TEST_P( static_cast( web_contents->GetMainFrame()->GetSiteInstance()); - // Check that A and B are in different BrowsingInstances (both are in default - // SiteInstances of different BrowsingInstances) but B should use the sole - // process assigned to site B. + // Check that A and B are in different BrowsingInstances but B should use the + // sole process assigned to site B. EXPECT_FALSE(a_site_instance->IsRelatedSiteInstance(b_site_instance.get())); - EXPECT_TRUE(a_site_instance->IsDefaultSiteInstance()); - EXPECT_TRUE(b_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + a_site_instance->IsDefaultSiteInstance()); + EXPECT_EQ(AreDefaultSiteInstancesEnabled(), + b_site_instance->IsDefaultSiteInstance()); EXPECT_NE(b_site_instance->GetProcess(), original_process); EXPECT_EQ(b_site_instance->GetProcess(), process_for_b); EXPECT_EQ(b_site_instance->GetProcess(), @@ -7697,6 +7708,11 @@ IN_PROC_BROWSER_TEST_P(ProactivelySwapBrowsingInstancesSameSiteTest, } { + FrameTreeNode* root = web_contents->GetFrameTree()->root(); + RenderFrameHostImpl* child_rfh = root->child_at(0)->current_frame_host(); + bool subframe_was_in_same_site_instance = + root->current_frame_host()->GetSiteInstance() == + child_rfh->GetSiteInstance(); // 4) Set up a script that will call postMessage on a cross-site iframe // after we commit the next navigation. // TODO(https://crbug.com/1110497): GetAsyncScriptExecutorCallback() must be @@ -7722,7 +7738,8 @@ IN_PROC_BROWSER_TEST_P(ProactivelySwapBrowsingInstancesSameSiteTest, // have already unloaded). ExpectBucketCount(kActionAfterPagehideHistogramName, ActionAfterPagehide::kSentPostMessage, 2); - if (AreAllSitesIsolatedForTesting() && !IsBackForwardCacheEnabled()) { + + if (!subframe_was_in_same_site_instance && !IsBackForwardCacheEnabled()) { ExpectBucketCount(kActionAfterPagehideHistogramName, ActionAfterPagehide::kReceivedPostMessage, 1); } else { @@ -8543,12 +8560,8 @@ IN_PROC_BROWSER_TEST_P(RenderFrameHostManagerTest, scoped_refptr new_instance = web_contents->GetMainFrame()->GetSiteInstance(); EXPECT_EQ(url1, web_contents->GetLastCommittedURL()); - if (AreAllSitesIsolatedForTesting() || AreDefaultSiteInstancesEnabled()) { - EXPECT_NE(instance1, new_instance); - EXPECT_EQ(GURL(), new_instance->GetSiteURL()); - } else { - EXPECT_EQ(instance1, new_instance); - } + EXPECT_NE(instance1, new_instance); + EXPECT_EQ(GURL(), new_instance->GetSiteURL()); EXPECT_TRUE(new_instance->HasProcess()); // Because url1 does not set a site URL, it should not lock the new process diff --git a/content/browser/renderer_host/render_frame_host_manager_unittest.cc b/content/browser/renderer_host/render_frame_host_manager_unittest.cc index 89b09163f0c133..bbbae29007d8fa 100644 --- a/content/browser/renderer_host/render_frame_host_manager_unittest.cc +++ b/content/browser/renderer_host/render_frame_host_manager_unittest.cc @@ -314,9 +314,9 @@ class RenderFrameHostManagerTest RenderViewHostImplTestHarness::SetUp(); WebUIControllerFactory::RegisterFactory(&factory_); - if (AreDefaultSiteInstancesEnabled()) { - // Isolate |isolated_cross_site_url()| so we can't get a default - // SiteInstance for it. + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { + // Isolate |isolated_cross_site_url()|so it cannot share a process + // with another site. ChildProcessSecurityPolicyImpl::GetInstance()->AddIsolatedOrigins( {url::Origin::Create(isolated_cross_site_url())}, ChildProcessSecurityPolicy::IsolatedOriginSource::TEST, diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc index 065444d16da610..046084c5913b41 100644 --- a/content/browser/security_exploit_browsertest.cc +++ b/content/browser/security_exploit_browsertest.cc @@ -112,7 +112,7 @@ RenderFrameHostImpl* PrepareToDuplicateHosts(Shell* shell, int* target_routing_id) { GURL foo("http://foo.com/simple_page.html"); - if (AreDefaultSiteInstancesEnabled()) { + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { // Isolate "bar.com" so we are guaranteed to get a different process // for navigations to this origin. IsolateOriginsForTesting(server, shell->web_contents(), {"bar.com"}); diff --git a/content/browser/site_instance_impl_unittest.cc b/content/browser/site_instance_impl_unittest.cc index ec0849f9f054a0..c8fc1ddc3457fd 100644 --- a/content/browser/site_instance_impl_unittest.cc +++ b/content/browser/site_instance_impl_unittest.cc @@ -431,8 +431,9 @@ TEST_F(SiteInstanceTest, DefaultSiteInstanceProperties) { // 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); + feature_list.InitWithFeatures( + /* enable */ {features::kProcessSharingWithDefaultSiteInstances}, + /* disable */ {features::kProcessSharingWithStrictSiteInstances}); EXPECT_TRUE(base::FeatureList::IsEnabled( features::kProcessSharingWithDefaultSiteInstances)); EXPECT_FALSE(base::FeatureList::IsEnabled( @@ -1706,12 +1707,12 @@ TEST_F(SiteInstanceTest, CreateForGuest) { context(), UrlInfo::CreateForTesting(kGuestUrl), CoopCoepCrossOriginIsolatedInfo::CreateNonIsolated()); EXPECT_FALSE(instance1->IsGuest()); - if (AreAllSitesIsolatedForTesting()) { + if (AreDefaultSiteInstancesEnabled()) { + EXPECT_TRUE(instance1->IsDefaultSiteInstance()); + } else { EXPECT_NE(kGuestUrl, instance1->GetSiteURL()); EXPECT_EQ(GURL(std::string(kGuestScheme) + "://abc123/"), instance1->GetSiteURL()); - } else { - EXPECT_TRUE(instance1->IsDefaultSiteInstance()); } // Verify that a SiteInstance created with CreateForGuest() is considered diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index ed0e9d9b5b4019..f9b7b170f55b82 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -113,9 +113,9 @@ class WebContentsImplTest : public RenderViewHostImplTestHarness { WebUIControllerFactory::RegisterFactory( ContentWebUIControllerFactory::GetInstance()); - if (AreDefaultSiteInstancesEnabled()) { - // Isolate |isolated_cross_site_url()| so we can't get a default - // SiteInstance for it. + if (IsIsolatedOriginRequiredToGuaranteeDedicatedProcess()) { + // Isolate |isolated_cross_site_url()| so it cannot share a process + // with another site. ChildProcessSecurityPolicyImpl::GetInstance()->AddIsolatedOrigins( {url::Origin::Create(isolated_cross_site_url())}, ChildProcessSecurityPolicy::IsolatedOriginSource::TEST, diff --git a/content/browser/web_package/web_bundle_browsertest.cc b/content/browser/web_package/web_bundle_browsertest.cc index 75e0e3c32ca45f..b9282e96d06385 100644 --- a/content/browser/web_package/web_bundle_browsertest.cc +++ b/content/browser/web_package/web_bundle_browsertest.cc @@ -1370,8 +1370,8 @@ void RunIframeParentInitiatedOutOfBundleNavigationTest( GetLoadResultForNavigationTest(GetFirstChild(web_contents))); FrameTreeNode* iframe_node = GetFirstChild(web_contents); - bool is_same_process = (iframe_node->parent()->GetProcess() == - iframe_node->current_frame_host()->GetProcess()); + bool no_proxy_to_parent = + iframe_node->render_manager()->GetProxyToParent() == nullptr; RunScriptAndObserveNavigation( "Navigate the iframe to /1-page/", web_contents, @@ -1387,7 +1387,7 @@ void RunIframeParentInitiatedOutOfBundleNavigationTest( // from web bundle. To support this case we need to change // NavigationControllerImpl::NavigateFromFrameProxy() to correctly handle // the WebBundleHandleTracker. - EXPECT_EQ(is_same_process + EXPECT_EQ(no_proxy_to_parent ? "/1-page/ from wbn, /1-page/script from wbn" : "/1-page/ from server, /1-page/script from server", GetLoadResultForNavigationTest(GetFirstChild(web_contents))); diff --git a/content/public/test/test_utils.cc b/content/public/test/test_utils.cc index 2812abf41e6d17..f68f6e2afc816a 100644 --- a/content/public/test/test_utils.cc +++ b/content/public/test/test_utils.cc @@ -205,6 +205,18 @@ bool AreDefaultSiteInstancesEnabled() { features::kProcessSharingWithDefaultSiteInstances); } +bool AreStrictSiteInstancesEnabled() { + return AreAllSitesIsolatedForTesting() || + base::FeatureList::IsEnabled( + features::kProcessSharingWithStrictSiteInstances); +} + +bool IsIsolatedOriginRequiredToGuaranteeDedicatedProcess() { + return AreDefaultSiteInstancesEnabled() || + base::FeatureList::IsEnabled( + features::kProcessSharingWithStrictSiteInstances); +} + void IsolateAllSitesForTesting(base::CommandLine* command_line) { command_line->AppendSwitch(switches::kSitePerProcess); } diff --git a/content/public/test/test_utils.h b/content/public/test/test_utils.h index b55ad04cb7bb3e..3cd02f9a671d55 100644 --- a/content/public/test/test_utils.h +++ b/content/public/test/test_utils.h @@ -95,6 +95,18 @@ bool AreAllSitesIsolatedForTesting(); // to mark expectations specific to default SiteInstances. bool AreDefaultSiteInstancesEnabled(); +// Returns true if the process model only allows a SiteInstance to contain +// a single site. +bool AreStrictSiteInstancesEnabled(); + +// Returns true if a test needs to register an origin for isolation to ensure +// that navigations, for that origin, are placed in a dedicated process. Some +// process model modes allow sites to share a process if they are not isolated. +// This helper indicates when such a mode is in use and indicates the test must +// register an isolated origin to ensure the origin gets placed in its own +// process. +bool IsIsolatedOriginRequiredToGuaranteeDedicatedProcess(); + // Appends --site-per-process to the command line, enabling tests to exercise // site isolation and cross-process iframes. This must be called early in // the test; the flag will be read on the first real navigation. diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc index efb500cf04956c..3572ec583b0021 100644 --- a/content/test/test_web_contents.cc +++ b/content/test/test_web_contents.cc @@ -272,7 +272,18 @@ void TestWebContents::TestDidFailLoadWithError(const GURL& url, } bool TestWebContents::CrossProcessNavigationPending() { - return GetRenderManager()->speculative_render_frame_host_ != nullptr; + // If we don't have a speculative RenderFrameHost then it means we did not + // change SiteInstances so we must be in the same process. + if (GetRenderManager()->speculative_render_frame_host_ == nullptr) + return false; + + auto* current_instance = + GetRenderManager()->current_frame_host()->GetSiteInstance(); + auto* speculative_instance = + GetRenderManager()->speculative_frame_host()->GetSiteInstance(); + if (current_instance == speculative_instance) + return false; + return current_instance->GetProcess() != speculative_instance->GetProcess(); } bool TestWebContents::CreateRenderViewForRenderManager(