Force a new BrowsingInstance in OAC WPT tests upon reset.

This CL detects the reset used between WPT tests and forces a new
BrowsingInstance when the flag

--force-browsing-instance-reset-between-tests

is included with a virtual test suite. It also adds this flag to
the virtual test suite used to run the OriginAgentCluster (OAC)
web tests. This prevents OAC state from a previous test from affecting
subsequent tests, since OAC state is per-BrowsingInstance.

Bug: 1227911
Change-Id: Ia330d3cb781e57e8709f80e51b8feef208e6d058
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212936
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Erik Staab <estaab@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: James Maclean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/main@{#974736}

content/browser/renderer_host/navigation_controller_impl.cc

@@ -3766,6 +3766,8 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
       params.impression, params.is_pdf);
   navigation_request->set_from_download_cross_origin_redirect(
       params.from_download_cross_origin_redirect);
+  navigation_request->set_force_new_browsing_instance(
+      params.force_new_browsing_instance);
   return navigation_request;
 }
 

content/browser/renderer_host/navigation_controller_impl_browsertest.cc

@@ -21052,6 +21052,32 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerInitialNavigationEntryBrowserTest,
   EXPECT_EQ(url, new_controller.GetLastCommittedEntry()->GetURL());
 }
 
+// A test to verify that a navigation with |force_new_browsing_instance| set to
+// true results in a new BrowsingInstance, even if it's to a URL compatible with
+// the original SiteInstance.
+IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
+                       ResetForTestForcesNewBrowsingInstance) {
+  GURL test_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), test_url));
+
+  SiteInstance* initial_site_instance = contents()->GetSiteInstance();
+
+  FrameTreeNode* root = contents()->GetPrimaryFrameTree().root();
+  DisableProactiveBrowsingInstanceSwapFor(root->current_frame_host());
+
+  TestNavigationObserver navigation_observer(contents());
+  NavigationController::LoadURLParams params(test_url);
+  params.force_new_browsing_instance = true;
+  contents()->GetController().LoadURLWithParams(params);
+  navigation_observer.Wait();
+  EXPECT_TRUE(navigation_observer.last_navigation_succeeded());
+
+  SiteInstance* post_reset_site_instance = contents()->GetSiteInstance();
+  EXPECT_NE(initial_site_instance, post_reset_site_instance);
+  EXPECT_FALSE(
+      post_reset_site_instance->IsRelatedSiteInstance(initial_site_instance));
+}
+
 INSTANTIATE_TEST_SUITE_P(
     All,
     NavigationControllerAlertDialogBrowserTest,

content/browser/renderer_host/navigation_request.h

@@ -920,6 +920,16 @@ class CONTENT_EXPORT NavigationRequest
     prerender_embedder_histogram_suffix_ = suffix;
   }
 
+  // Used in tests to indicate this navigation should force a BrowsingInstance
+  // swap.
+  void set_force_new_browsing_instance(bool force_new_browsing_instance) {
+    force_new_browsing_instance_ = force_new_browsing_instance;
+  }
+
+  // When this returns true, it indicates this navigation should force a
+  // BrowsingInstance swap. Used only in tests.
+  bool force_new_browsing_instance() { return force_new_browsing_instance_; }
+
  private:
   friend class NavigationRequestTest;
 
@@ -1978,6 +1988,10 @@ class CONTENT_EXPORT NavigationRequest
   // navigation.
   std::unique_ptr<ui::CompositorLock> compositor_lock_;
 
+  // This navigation request should swap browsing instances as part of a test
+  // reset.
+  bool force_new_browsing_instance_ = false;
+
   base::WeakPtrFactory<NavigationRequest> weak_factory_{this};
 };
 

content/browser/renderer_host/render_frame_host_manager.cc

@@ -1798,6 +1798,7 @@ RenderFrameHostManager::GetSiteInstanceForNavigation(
     bool was_server_redirect,
     bool cross_origin_opener_policy_mismatch,
     bool should_replace_current_entry,
+    bool force_new_browsing_instance,
     std::string* reason) {
   // On renderer-initiated navigations, when the frame initiating the navigation
   // and the frame being navigated differ, |source_instance| is set to the
@@ -1854,12 +1855,15 @@ RenderFrameHostManager::GetSiteInstanceForNavigation(
   SiteInstanceImpl* current_instance_impl =
       static_cast<SiteInstanceImpl*>(current_instance);
   ShouldSwapBrowsingInstance should_swap_result =
-      ShouldSwapBrowsingInstancesForNavigation(
-          current_effective_url, current_is_view_source_mode, source_instance,
-          current_instance_impl, dest_instance, dest_url_info,
-          dest_is_view_source_mode, transition, is_failure, is_reload,
-          is_same_document, cross_origin_opener_policy_mismatch,
-          was_server_redirect, should_replace_current_entry);
+      force_new_browsing_instance
+          ? ShouldSwapBrowsingInstance::kYes_SameSiteProactiveSwap
+          : ShouldSwapBrowsingInstancesForNavigation(
+                current_effective_url, current_is_view_source_mode,
+                source_instance, current_instance_impl, dest_instance,
+                dest_url_info, dest_is_view_source_mode, transition, is_failure,
+                is_reload, is_same_document,
+                cross_origin_opener_policy_mismatch, was_server_redirect,
+                should_replace_current_entry);
 
   TraceShouldSwapBrowsingInstanceResult(frame_tree_node_->frame_tree_node_id(),
                                         should_swap_result);
@@ -3078,7 +3082,8 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
       request->GetRestoreType() == RestoreType::kRestored,
       request->commit_params().is_view_source, request->WasServerRedirect(),
       request->coop_status().require_browsing_instance_swap(),
-      request->common_params().should_replace_current_entry, reason);
+      request->common_params().should_replace_current_entry,
+      request->force_new_browsing_instance(), reason);
 
   TRACE_EVENT_INSTANT(
       "navigation",

content/browser/renderer_host/render_frame_host_manager.h

@@ -679,6 +679,7 @@ class CONTENT_EXPORT RenderFrameHostManager {
       bool was_server_redirect,
       bool cross_origin_opener_policy_mismatch,
       bool should_replace_current_entry,
+      bool force_new_browsing_instance,
       std::string* reason);
 
   // Returns a descriptor of the appropriate SiteInstance object for the given

content/public/browser/navigation_controller.h

@@ -294,6 +294,11 @@ class NavigationController {
 
     // Indicates that this navigation is for PDF content in a renderer.
     bool is_pdf = false;
+
+    // Indicates this navigation should use a new BrowsingInstance. For example,
+    // this is used in web platform tests to guarantee that each test starts in
+    // a fresh BrowsingInstance.
+    bool force_new_browsing_instance = false;
   };
 
   // Disables checking for a repost and prompting the user. This is used during

content/web_test/browser/web_test_control_host.cc

@@ -1818,6 +1818,12 @@ void WebTestControlHost::PrepareRendererForNextWebTest() {
   params.transition_type = ui::PageTransitionFromInt(ui::PAGE_TRANSITION_TYPED);
   params.should_clear_history_list = true;
   params.initiator_origin = url::Origin();  // Opaque initiator.
+  // We should always reset the browsing instance, but it slows down tests
+  // significantly. For efficiency, this is limited to tests known to be
+  // affected.
+  params.force_new_browsing_instance =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kResetBrowsingInstanceBetweenTests);
   web_contents->GetController().LoadURLWithParams(params);
 
   // The navigation might have to wait for before unload handler to execute. The

content/web_test/common/web_test_switches.cc

@@ -42,6 +42,12 @@ const char kEncodeBinary[] = "encode-binary";
 const char kDisableAutoWPTOriginIsolation[] =
     "disable-auto-wpt-origin-isolation";
 
+// Forces each web test to be run in a new BrowsingInstance. Required for origin
+// isolation web tests where the BrowsingInstance retains state from origin
+// isolation requests, but this flag may benefit other web tests.
+const char kResetBrowsingInstanceBetweenTests[] =
+    "reset-browsing-instance-between-tests";
+
 // This makes us disable some web-platform runtime features so that we test
 // content_shell as if it was a stable release. It is only followed when
 // kRunWebTest is set. For the features' level, see

content/web_test/common/web_test_switches.h

@@ -23,6 +23,7 @@ extern const char kEncodeBinary[];
 extern const char kStableReleaseMode[];
 extern const char kDisableHeadlessMode[];
 extern const char kDisableAutoWPTOriginIsolation[];
+extern const char kResetBrowsingInstanceBetweenTests[];
 
 #if BUILDFLAG(IS_WIN)
 extern const char kRegisterFontFiles[];

third_party/blink/web_tests/TestExpectations

@@ -3626,6 +3626,7 @@ crbug.com/626703 external/wpt/infrastructure/assumptions/non-local-ports.sub.win
 crbug.com/626703 [ Mac10.15 ] external/wpt/mediacapture-insertable-streams/MediaStreamTrackGenerator-audio.https.html [ Crash ]
 crbug.com/626703 external/wpt/webrtc-extensions/transfer-datachannel-service-worker.https.html [ Timeout ]
 crbug.com/626703 external/wpt/webrtc-extensions/transfer-datachannel.html [ Timeout ]
+crbug.com/626703 [ Mac ] virtual/no-auto-wpt-origin-isolation/external/wpt/html/browsers/origin/origin-keyed-agent-clusters/1-iframe/parent-yes-child-no-port.sub.https.html [ Failure Timeout ]
 crbug.com/626703 [ Win ] external/wpt/websockets/Create-blocked-port.any.worker.html?wpt_flags=h2 [ Failure Timeout ]
 crbug.com/626703 external/wpt/FileAPI/file/send-file-formdata-controls.any.html [ Pass Timeout ]
 crbug.com/626703 external/wpt/FileAPI/file/send-file-formdata-controls.any.worker.html [ Pass Timeout ]
@@ -7056,12 +7057,6 @@ crbug.com/1249176 [ Mac11-arm64 ] external/wpt/largest-contentful-paint/multiple
 crbug.com/1249176 [ Mac11-arm64 ] virtual/background-svg-in-lcp/external/wpt/largest-contentful-paint/multiple-redirects-TAO.html [ Failure Pass ]
 crbug.com/1249176 [ Mac11-arm64 ] virtual/webrtc-wpt-plan-b/external/wpt/webrtc/protocol/split.https.html [ Pass Timeout ]
 
-# origin-keyed-agent-clusters tests are flaky on all platforms.
-# These tests are skipped everywhere except under the not-site-per-process and
-# no-auto-wpt-origin-isolation virtual test suites (see: NeverFixTests).
-crbug.com/1227911 virtual/not-site-per-process/external/wpt/html/browsers/origin/origin-keyed-agent-clusters/* [ Failure Pass ]
-crbug.com/1227911 virtual/no-auto-wpt-origin-isolation/external/wpt/html/browsers/origin/origin-keyed-agent-clusters/* [ Failure Pass ]
-
 # mac-arm CI 10-01-2021
 crbug.com/1249176 [ Mac11-arm64 ] editing/text-iterator/beforematch-async.html [ Failure ]
 crbug.com/1249176 [ Mac11-arm64 ] external/wpt/event-timing/click-interactionid.html [ Timeout ]

third_party/blink/web_tests/VirtualTestSuites

@@ -518,7 +518,8 @@
       "external/wpt/html/browsers/origin/origin-keyed-agent-clusters",
       "external/wpt/app-history/navigate-event/cross-window"
     ],
-    "args": ["--disable-auto-wpt-origin-isolation"]
+    "args": ["--disable-auto-wpt-origin-isolation",
+             "--reset-browsing-instance-between-tests"]
   },
   {
     "prefix": "focusless-spat-nav",