Reland "Don't run NavigationThrottles on page activations"

This is a reland of 7d90f2699d which was
speculatively reverted but was not the root cause of the issue.

Original change's description:
> Don't run NavigationThrottles on page activations
>
> Background reading:
> https://groups.google.com/u/1/a/chromium.org/g/navigation-dev/c/RJVDOjG8sQ4
>
> https://docs.google.com/document/d/1aNCGTjfRg9KwNYIz6oA8VKZ8EjD2nLA9OfwcJ9dTUxc/edit#heading=h.w2ddswjnb45
>
> For an activating navigation like activating a prerendered page, or
> restoring a page from the BackForward cache, NavigationThrottles have
> already run when the page was navigated and loaded the first time. We
> don't want to run throttles a second time when activating the page to
> become primary.
>
> This CL avoids registering throttles when the NavigationRequest is a
> page activating request. We still proceed through the navigation using
> calls on NavigationThrottleRunner since that's pretty tightly coupled to
> the phases of a NavigationRequest.
>
> Summary:
>
>  * Remove two BFCache tests (added in https://crrev.com/c/1760220) that
>    were checking behavior of throttles on restoration, these are no
>    longer relevant.
>  * TestNavigationManager is used to pause at certain points in a
>    navigation. It uses a throttle to do this today. This CL adds a
>    CommitDeferringCondition instead where the request is PageActivating
>    to allow callers to pause at the equivalent stage. (this means such
>    callers cannot use WaitForRequestStart)
>  * Fixes NavigationSimulatorImpl to work for activating navigations
>    without throttles.
>
> Bug: 1199724
> Change-Id: I61e58912acf865cb4b82fc97ccf26d8808b61a9e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2911320
> Commit-Queue: David Bokan <bokan@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#889886}

Bug: 1199724
Change-Id: Ie9fd0e3ab6f90296fe14521d6ab8f4d232f86336
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2945384
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Owners-Override: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#890798}

chrome/browser/plugins/pdf_iframe_navigation_throttle_browsertest.cc

@@ -112,7 +112,7 @@ IN_PROC_BROWSER_TEST_F(PDFIFrameNavigationThrottleBrowserTest,
   // frame, we expect the navigation to be deferred during WillStartRequest
   // until the prerender is activated.
   {
-    pdf_navigation.WaitForDidStartNavigation();
+    pdf_navigation.WaitForFirstYieldAfterDidStartNavigation();
     EXPECT_FALSE(pdf_navigation.GetNavigationHandle()->HasCommitted());
     EXPECT_TRUE(pdf_navigation.GetNavigationHandle()->IsDeferredForTesting());
   }

content/browser/back_forward_cache_browsertest.cc

@@ -71,6 +71,7 @@
 #include "content/public/test/text_input_test_utils.h"
 #include "content/public/test/url_loader_interceptor.h"
 #include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_content_browser_client.h"
 #include "content/shell/browser/shell_javascript_dialog_manager.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "content/test/echo.test-mojom.h"
@@ -3844,80 +3845,6 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, EvictPageWithInfiniteLoop) {
       {}, {}, FROM_HERE);
 }
 
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
-                       NavigationCancelledAtWillStartRequest) {
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
-
-  // 1) Navigate to A.
-  EXPECT_TRUE(NavigateToURL(shell(), url_a));
-  RenderFrameHostImpl* rfh_a = current_frame_host();
-  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
-
-  // 2) Navigate to B.
-  EXPECT_TRUE(NavigateToURL(shell(), url_b));
-  RenderFrameHostImpl* rfh_b = current_frame_host();
-  EXPECT_FALSE(delete_observer_rfh_a.deleted());
-  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
-
-  // Cancel all navigation attempts.
-  content::TestNavigationThrottleInserter throttle_inserter(
-      shell()->web_contents(),
-      base::BindLambdaForTesting(
-          [&](NavigationHandle* handle) -> std::unique_ptr<NavigationThrottle> {
-            auto throttle = std::make_unique<TestNavigationThrottle>(handle);
-            throttle->SetResponse(TestNavigationThrottle::WILL_START_REQUEST,
-                                  TestNavigationThrottle::SYNCHRONOUS,
-                                  NavigationThrottle::CANCEL_AND_IGNORE);
-            return throttle;
-          }));
-
-  // 3) Go back to A, which will be cancelled by the NavigationThrottle.
-  web_contents()->GetController().GoBack();
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-
-  // We should still be showing page B.
-  EXPECT_EQ(rfh_b, current_frame_host());
-}
-
-IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
-                       NavigationCancelledAtWillProcessResponse) {
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
-  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
-
-  // 1) Navigate to A.
-  EXPECT_TRUE(NavigateToURL(shell(), url_a));
-  RenderFrameHostImpl* rfh_a = current_frame_host();
-  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
-
-  // 2) Navigate to B.
-  EXPECT_TRUE(NavigateToURL(shell(), url_b));
-  RenderFrameHostImpl* rfh_b = current_frame_host();
-  EXPECT_FALSE(delete_observer_rfh_a.deleted());
-  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
-
-  // Cancel all navigation attempts.
-  content::TestNavigationThrottleInserter throttle_inserter(
-      shell()->web_contents(),
-      base::BindLambdaForTesting(
-          [&](NavigationHandle* handle) -> std::unique_ptr<NavigationThrottle> {
-            auto throttle = std::make_unique<TestNavigationThrottle>(handle);
-            throttle->SetResponse(TestNavigationThrottle::WILL_PROCESS_RESPONSE,
-                                  TestNavigationThrottle::SYNCHRONOUS,
-                                  NavigationThrottle::CANCEL_AND_IGNORE);
-            return throttle;
-          }));
-
-  // 3) Go back to A, which will be cancelled by the NavigationThrottle.
-  web_contents()->GetController().GoBack();
-  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-
-  // We should still be showing page B.
-  EXPECT_EQ(rfh_b, current_frame_host());
-}
-
 // Test the race condition where a document is evicted from the BackForwardCache
 // while it is in the middle of being restored and before URL loader starts a
 // response.
@@ -4006,7 +3933,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   TestNavigationManager navigation_manager(shell()->web_contents(), url_a);
   web_contents()->GetController().GoBack();
 
-  EXPECT_TRUE(navigation_manager.WaitForRequestStart());
+  EXPECT_TRUE(navigation_manager.WaitForResponse());
 
   // Flush the cache, which contains the document being navigated to.
   web_contents()->GetController().GetBackForwardCache().Flush();
@@ -4578,7 +4505,6 @@ IN_PROC_BROWSER_TEST_F(
 
   // 3) Go back to the first page using TestNavigationManager so that we split
   // the navigation into stages.
-  // web_contents()->GetController().GoBack();
   TestNavigationManager navigation_manager_back(shell()->web_contents(), url);
   web_contents()->GetController().GoBack();
   EXPECT_TRUE(navigation_manager_back.WaitForResponse());
@@ -4587,6 +4513,9 @@ IN_PROC_BROWSER_TEST_F(
   // asynchronously for renderers to reply that they've unfrozen. Finish the
   // image response in that time.
   navigation_manager_back.ResumeNavigation();
+  ASSERT_TRUE(
+      NavigationRequest::From(navigation_manager_back.GetNavigationHandle())
+          ->IsCommitDeferringConditionDeferredForTesting());
   ASSERT_FALSE(navigation_manager_back.GetNavigationHandle()->HasCommitted());
 
   image_response.Send(net::HTTP_OK, "image/png");
@@ -11479,7 +11408,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   // 3) Start navigating back.
   TestNavigationManager nav_manager(shell->web_contents(), url_a);
   shell->web_contents()->GetController().GoBack();
-  EXPECT_TRUE(nav_manager.WaitForRequestStart());
+  nav_manager.WaitForFirstYieldAfterDidStartNavigation();
 
   testing::NiceMock<MockWebContentsObserver> observer(shell->web_contents());
   EXPECT_CALL(observer, DidFinishNavigation(_))
@@ -11834,6 +11763,48 @@ IN_PROC_BROWSER_TEST_P(BackForwardCacheUnloadStrategyBrowserTest,
   ExpectRestored(FROM_HERE);
 }
 
+IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, NoThrottlesOnCacheRestore) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title1.html"));
+
+  // 1) Navigate to A.
+  ASSERT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  bool did_register_throttles = false;
+
+  // This will track for each navigation whether we attempted to register
+  // NavigationThrottles.
+  content::ShellContentBrowserClient::Get()
+      ->set_create_throttles_for_navigation_callback(base::BindLambdaForTesting(
+          [&did_register_throttles](content::NavigationHandle* handle)
+              -> std::vector<std::unique_ptr<content::NavigationThrottle>> {
+            did_register_throttles = true;
+            return std::vector<std::unique_ptr<content::NavigationThrottle>>();
+          }));
+
+  // 2) Navigate to B.
+  ASSERT_TRUE(NavigateToURL(shell(), url_b));
+  RenderFrameHostImpl* rfh_b = current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);
+  ASSERT_FALSE(delete_observer_rfh_a.deleted());
+  ASSERT_TRUE(rfh_a->IsInBackForwardCache());
+  EXPECT_TRUE(did_register_throttles);
+  did_register_throttles = false;
+
+  // 3) Go back to A which is in the BackForward cache and will be restored via
+  // an IsPageActivation navigation. Ensure that we did not register
+  // NavigationThrottles for this navigation since we already ran their checks
+  // when we navigated to A in step 1.
+  web_contents()->GetController().GoBack();
+  ASSERT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_FALSE(did_register_throttles);
+
+  ExpectRestored(FROM_HERE);
+}
+
 namespace {
 enum class SubframeType { SameSite, CrossSite };
 }

content/browser/prerender/prerender_browsertest.cc

@@ -50,8 +50,10 @@
 #include "content/public/test/navigation_handle_observer.h"
 #include "content/public/test/prerender_test_util.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_navigation_throttle.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_content_browser_client.h"
 #include "content/test/content_browser_test_utils_internal.h"
 #include "content/test/mock_commit_deferring_condition.h"
 #include "content/test/test_content_browser_client.h"
@@ -868,6 +870,75 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
   EXPECT_TRUE(is_activation_observer.was_activation());
 }
 
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, ActivationDoesntRunThrottles) {
+  const GURL kInitialUrl = GetUrl("/empty.html");
+  const GURL kPrerenderingUrl = GetUrl("/empty.html?prerender");
+
+  test::PrerenderHostObserver prerender_observer(*shell()->web_contents(),
+                                                 kPrerenderingUrl);
+
+  // Navigate to the initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+  ASSERT_EQ(web_contents()->GetURL(), kInitialUrl);
+  ASSERT_TRUE(WaitForLoadStop(shell()->web_contents()));
+
+  NavigationThrottle* throttle = nullptr;
+  // This will attempt to insert a throttle that DEFERs the navigation at
+  // WillStartRequest into all new navigations.
+  content::ShellContentBrowserClient::Get()
+      ->set_create_throttles_for_navigation_callback(base::BindLambdaForTesting(
+          [&throttle](content::NavigationHandle* handle)
+              -> std::vector<std::unique_ptr<content::NavigationThrottle>> {
+            std::vector<std::unique_ptr<content::NavigationThrottle>> throttles;
+
+            auto throttle_ptr =
+                std::make_unique<TestNavigationThrottle>(handle);
+            DCHECK(!throttle);
+            throttle = throttle_ptr.get();
+            throttle_ptr->SetResponse(
+                TestNavigationThrottle::WILL_START_REQUEST,
+                TestNavigationThrottle::SYNCHRONOUS, NavigationThrottle::DEFER);
+
+            throttles.push_back(std::move(throttle_ptr));
+            return throttles;
+          }));
+
+  // Start a prerender and ensure that a NavigationThrottle can defer the
+  // prerendering navigation. Then resume the navigation so the prerender
+  // navigation and load completes.
+  {
+    TestNavigationManager prerender_manager(shell()->web_contents(),
+                                            kPrerenderingUrl);
+    AddPrerenderAsync(kPrerenderingUrl);
+    prerender_manager.WaitForFirstYieldAfterDidStartNavigation();
+    ASSERT_NE(throttle, nullptr);
+
+    auto* request =
+        NavigationRequest::From(prerender_manager.GetNavigationHandle());
+    ASSERT_TRUE(request->IsDeferredForTesting());
+    EXPECT_EQ(request->GetDeferringThrottleForTesting(), throttle);
+    throttle = nullptr;
+
+    request->GetNavigationThrottleRunnerForTesting()->CallResumeForTesting();
+    prerender_manager.WaitForNavigationFinished();
+
+    auto host_id = GetHostForUrl(kPrerenderingUrl);
+    ASSERT_NE(host_id, RenderFrameHost::kNoFrameTreeNodeId);
+    EXPECT_EQ(GetPrerenderedMainFrameHost(host_id)->GetLastCommittedURL(),
+              kPrerenderingUrl);
+  }
+
+  // Now navigate the primary page to the prerendered URL so that we activate
+  // the prerender. The throttle should not have been registered for the
+  // activating navigation.
+  {
+    NavigatePrimaryPage(kPrerenderingUrl);
+    prerender_observer.WaitForActivation();
+    EXPECT_EQ(web_contents()->GetURL(), kPrerenderingUrl);
+    EXPECT_EQ(throttle, nullptr);
+  }
+}
+
 // Ensures that if we attempt to open a URL while prerendering with a window
 // disposition other than CURRENT_TAB, we fail.
 IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, SuppressOpenURL) {
@@ -2027,7 +2098,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
         kNewIframeUrl, Referrer(), child_frame->GetFrameTreeNodeId(),
         WindowOpenDisposition::CURRENT_TAB, ui::PAGE_TRANSITION_AUTO_SUBFRAME,
         /*is_renderer_initiated=*/false));
-    iframe_observer.WaitForDidStartNavigation();
+    iframe_observer.WaitForFirstYieldAfterDidStartNavigation();
     NavigationRequest* request =
         static_cast<NavigationRequest*>(iframe_observer.GetNavigationHandle());
     EXPECT_EQ(request->state(), NavigationRequest::WILL_START_REQUEST);

content/browser/renderer_host/navigation_request.cc

@@ -4791,6 +4791,11 @@ void NavigationRequest::CancelDeferredNavigation(
 
 void NavigationRequest::RegisterThrottleForTesting(
     std::unique_ptr<NavigationThrottle> navigation_throttle) {
+  // Throttles will already have run the first time the page was navigated, we
+  // won't run them again on activation. See instead CommitDeferringCondition.
+  DCHECK(!IsPageActivation())
+      << "Attempted to register a NavigationThrottle for an activating "
+         "navigation which will not work.";
   throttle_runner_->AddThrottle(std::move(navigation_throttle));
 }
 bool NavigationRequest::IsDeferredForTesting() {
@@ -4868,7 +4873,11 @@ void NavigationRequest::WillStartRequest() {
     return;
   }
 
-  throttle_runner_->RegisterNavigationThrottles();
+  // Throttles will already have run the first time the page was navigated, we
+  // won't run them again on activation.
+  if (!IsPageActivation())
+    throttle_runner_->RegisterNavigationThrottles();
+
   commit_deferrer_->RegisterDeferringConditions(*this);
 
   // If the content/ embedder did not pass the NavigationUIData at the beginning

content/public/test/browser_test_utils.cc

@@ -84,6 +84,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/test/did_commit_navigation_interceptor.h"
+#include "content/test/mock_commit_deferring_condition.h"
 #include "ipc/ipc_security_test_util.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -2911,11 +2912,10 @@ TestNavigationManager::TestNavigationManager(WebContents* web_contents,
     : WebContentsObserver(web_contents), url_(url) {}
 
 TestNavigationManager::~TestNavigationManager() {
-  if (navigation_paused_)
-    request_->GetNavigationThrottleRunnerForTesting()->CallResumeForTesting();
+  ResumeIfPaused();
 }
 
-void TestNavigationManager::WaitForDidStartNavigation() {
+void TestNavigationManager::WaitForFirstYieldAfterDidStartNavigation() {
   if (current_state_ >= NavigationState::WILL_START)
     return;
 
@@ -2933,8 +2933,7 @@ void TestNavigationManager::ResumeNavigation() {
          current_state_ == NavigationState::RESPONSE);
   DCHECK_EQ(current_state_, desired_state_);
   DCHECK(navigation_paused_);
-  navigation_paused_ = false;
-  request_->GetNavigationThrottleRunnerForTesting()->CallResumeForTesting();
+  ResumeIfPaused();
 }
 
 NavigationHandle* TestNavigationManager::GetNavigationHandle() {
@@ -2956,13 +2955,27 @@ void TestNavigationManager::DidStartNavigation(NavigationHandle* handle) {
     return;
 
   request_ = NavigationRequest::From(handle);
-  auto throttle = std::make_unique<TestNavigationManagerThrottle>(
-      request_,
-      base::BindOnce(&TestNavigationManager::OnWillStartRequest,
-                     weak_factory_.GetWeakPtr()),
-      base::BindOnce(&TestNavigationManager::OnWillProcessResponse,
-                     weak_factory_.GetWeakPtr()));
-  request_->RegisterThrottleForTesting(std::move(throttle));
+  if (request_->IsPageActivation()) {
+    // For activating navigations, we have no way of stopping at
+    // WillStartRequest since we don't run throttles. Callers should use
+    // WaitForResponse() or WaitForFirstYieldAfterDidStartNavigation().
+    DCHECK_NE(desired_state_, NavigationState::STARTED);
+
+    auto condition = std::make_unique<MockCommitDeferringCondition>(
+        /*is_ready_to_commit=*/false,
+        base::BindOnce(
+            &TestNavigationManager::OnRunningCommitDeferringConditions,
+            weak_factory_.GetWeakPtr()));
+    request_->RegisterCommitDeferringConditionForTesting(std::move(condition));
+  } else {
+    auto throttle = std::make_unique<TestNavigationManagerThrottle>(
+        request_,
+        base::BindOnce(&TestNavigationManager::OnWillStartRequest,
+                       weak_factory_.GetWeakPtr()),
+        base::BindOnce(&TestNavigationManager::OnWillProcessResponse,
+                       weak_factory_.GetWeakPtr()));
+    request_->RegisterThrottleForTesting(std::move(throttle));
+  }
 
   current_state_ = NavigationState::WILL_START;
 
@@ -2973,8 +2986,10 @@ void TestNavigationManager::DidStartNavigation(NavigationHandle* handle) {
   // is set to always pause navigations at WillStartRequest. This ensures the
   // navigation will defer and the user can always call
   // WaitForRequestStart.
-  if (desired_state_ == NavigationState::WILL_START)
+  if (!request_->IsPageActivation() &&
+      desired_state_ == NavigationState::WILL_START) {
     desired_state_ = NavigationState::STARTED;
+  }
 }
 
 void TestNavigationManager::DidFinishNavigation(NavigationHandle* handle) {
@@ -3006,6 +3021,14 @@ void TestNavigationManager::OnWillProcessResponse() {
   OnNavigationStateChanged();
 }
 
+void TestNavigationManager::OnRunningCommitDeferringConditions(
+    base::OnceClosure resume_closure) {
+  current_state_ = NavigationState::RESPONSE;
+  commit_deferring_condition_resume_closure_ = std::move(resume_closure);
+  navigation_paused_ = true;
+  OnNavigationStateChanged();
+}
+
 // TODO(csharrison): Remove CallResumeForTesting method calls in favor of doing
 // it through the throttle.
 bool TestNavigationManager::WaitForDesiredState() {
@@ -3014,8 +3037,7 @@ bool TestNavigationManager::WaitForDesiredState() {
     return true;
 
   // Resume the navigation if it was paused.
-  if (navigation_paused_)
-    request_->GetNavigationThrottleRunnerForTesting()->CallResumeForTesting();
+  ResumeIfPaused();
 
   // Wait for the desired state if needed.
   if (current_state_ < desired_state_) {
@@ -3031,6 +3053,12 @@ bool TestNavigationManager::WaitForDesiredState() {
 }
 
 void TestNavigationManager::OnNavigationStateChanged() {
+  if (request_ && request_->IsPageActivation()) {
+    DCHECK_NE(desired_state_, NavigationState::STARTED)
+        << "Cannot use WaitForRequestStart() when managing an activating "
+           "navigation. Use either WaitForFirstYieldAfterDidStartNavigation() "
+           "or WaitForResponse()";
+  }
   // If the state the user was waiting for has been reached, exit the message
   // loop.
   if (current_state_ >= desired_state_) {
@@ -3040,8 +3068,19 @@ void TestNavigationManager::OnNavigationStateChanged() {
   }
 
   // Otherwise, the navigation should be resumed if it was previously paused.
-  if (navigation_paused_)
+  ResumeIfPaused();
+}
+
+void TestNavigationManager::ResumeIfPaused() {
+  if (!navigation_paused_)
+    return;
+
+  navigation_paused_ = false;
+
+  if (!request_->IsPageActivation())
     request_->GetNavigationThrottleRunnerForTesting()->CallResumeForTesting();
+  else if (commit_deferring_condition_resume_closure_)
+    std::move(commit_deferring_condition_resume_closure_).Run();
 }
 
 bool TestNavigationManager::ShouldMonitorNavigation(NavigationHandle* handle) {

content/public/test/browser_test_utils.h

@@ -1507,6 +1507,12 @@ class FrameDeletedObserver {
 // Note: This class is one time use only! After it successfully tracks a
 // navigation it will ignore all subsequent navigations. Explicitly create
 // multiple instances of this class if you want to pause multiple navigations.
+// Note2: For page activating navigations (like a prerender activation, or a
+// BFCache restore navigation), the navigation will not run throttles. The
+// manager in this case uses a CommitDeferringCondition for pausing the
+// navigation at the equivalent of WillProcessResponse. However, in these kinds
+// of navigations you cannot use WaitForRequestStart; if you want to yield
+// before WillProcessResponse, use WaitForFirstYieldAfterDidStartNavigation.
 class TestNavigationManager : public WebContentsObserver {
  public:
   // Monitors any frame in WebContents.
@@ -1514,15 +1520,23 @@ class TestNavigationManager : public WebContentsObserver {
 
   ~TestNavigationManager() override;
 
-  // Waits until the navigation starts. This is called from
-  // WebContentsObserver::DidStartNavigation and is run before any navigation
-  // throttles are registered.
-  void WaitForDidStartNavigation();
+  // Waits until the first yield point after DidStartNavigation. Unlike
+  // WaitForRequestStart, this can be used to wait for a pause in cases where a
+  // test expects a NavigationThrottle to defer in WillStartRequest. In cases
+  // where throttles run and none defer, this will break at the same time as
+  // WaitForRequestStart. Also unlike WaitForRequestStart, this can be used to
+  // wait on a page activating navigation to start. Note: since we won't know
+  // which throttle deferred, don't use ResumeNavigation() after this call since
+  // it assumes we paused from the TestNavigationManagerThrottle.
+  void WaitForFirstYieldAfterDidStartNavigation();
 
   // Waits until the navigation request is ready to be sent to the network
   // stack. This will wait until all NavigationThrottles have proceeded through
-  // WillStartRequest. Returns false if the request was aborted before
-  // starting.
+  // WillStartRequest. Returns false if the request was aborted before starting.
+  // Note: RequestStart is never reached for page activating navigations (e.g.
+  // prerender activation, BFCache restore). In those cases you should either
+  // use WaitForFirstYieldAfterDidStartNavigation or WaitForResponse. See
+  // TestNavigationManager class comment for more detail.
   WARN_UNUSED_RESULT bool WaitForRequestStart();
 
   // Waits until the navigation response's headers have been received. This
@@ -1580,6 +1594,10 @@ class TestNavigationManager : public WebContentsObserver {
   // WillProcessResponse.
   void OnWillProcessResponse();
 
+  // Called when the navigation pauses in the MockCommitDeferringCondition. This
+  // happens only for page activating navigations like a prerender activation.
+  void OnRunningCommitDeferringConditions(base::OnceClosure resume_closure);
+
   // Waits for the desired state. Returns false if the desired state cannot be
   // reached (eg the navigation finishes before reaching this state).
   bool WaitForDesiredState();
@@ -1589,6 +1607,8 @@ class TestNavigationManager : public WebContentsObserver {
   // resume the navigation if it hasn't been reached yet.
   void OnNavigationStateChanged();
 
+  void ResumeIfPaused();
+
   const GURL url_;
   NavigationRequest* request_ = nullptr;
   bool navigation_paused_ = false;
@@ -1599,6 +1619,12 @@ class TestNavigationManager : public WebContentsObserver {
   base::OnceClosure quit_closure_;
   base::RunLoop::Type message_loop_type_ = base::RunLoop::Type::kDefault;
 
+  // In a page activating navigation (prerender activation, back-forward cache
+  // activation), the navigation will be stopped in a commit deferring condition
+  // (since NavigationThrottles aren't run in a page activation). When that
+  // happens, the navigation can be resumed using this closure.
+  base::OnceClosure commit_deferring_condition_resume_closure_;
+
   base::WeakPtrFactory<TestNavigationManager> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TestNavigationManager);

content/test/navigation_simulator_impl.cc

@@ -385,6 +385,11 @@ void NavigationSimulatorImpl::InitializeFromStartedRequest(
 }
 
 void NavigationSimulatorImpl::RegisterTestThrottle(NavigationRequest* request) {
+  // Page activating navigations don't run throttles so we don't need to
+  // register it in that case.
+  if (request_->IsPageActivation())
+    return;
+
   request->RegisterThrottleForTesting(
       std::make_unique<NavigationThrottleCallbackRunner>(
           request,
@@ -552,7 +557,7 @@ void NavigationSimulatorImpl::ReadyToCommit() {
   auto complete_closure =
       base::BindOnce(&NavigationSimulatorImpl::WillProcessResponseComplete,
                      weak_factory_.GetWeakPtr());
-  if (NeedsThrottleChecks()) {
+  if (NeedsPreCommitChecks()) {
     MaybeWaitForThrottleChecksComplete(std::move(complete_closure));
     MaybeWaitForReadyToCommitCheckComplete();
     if (state_ == READY_TO_COMMIT) {
@@ -565,6 +570,7 @@ void NavigationSimulatorImpl::ReadyToCommit() {
     }
     return;
   }
+
   std::move(complete_closure).Run();
   ReadyToCommitComplete();
 }
@@ -1236,7 +1242,7 @@ bool NavigationSimulatorImpl::SimulateRendererInitiatedStart() {
 
 void NavigationSimulatorImpl::MaybeWaitForThrottleChecksComplete(
     base::OnceClosure complete_closure) {
-  // If last_throttle_check_result_ is set, then throttle checks completed
+  // If last_throttle_check_result_ is set, then the navigation phase completed
   // synchronously.
   if (last_throttle_check_result_) {
     std::move(complete_closure).Run();
@@ -1269,6 +1275,11 @@ void NavigationSimulatorImpl::Wait() {
 
 bool NavigationSimulatorImpl::OnThrottleChecksComplete(
     NavigationThrottle::ThrottleCheckResult result) {
+  if (request_->IsPageActivation()) {
+    // Throttles don't run in page activations so we shouldn't ever get back
+    // anything other than PROCEED.
+    CHECK_EQ(result.action(), NavigationThrottle::PROCEED);
+  }
   CHECK(!last_throttle_check_result_);
   last_throttle_check_result_ = result;
   if (wait_closure_)
@@ -1482,7 +1493,19 @@ bool NavigationSimulatorImpl::NeedsThrottleChecks() const {
     return false;
   }
 
+  // Back/forward cache restores and prerendering page activations do not run
+  // NavigationThrottles since they were already run when the page was first
+  // loaded.
+  DCHECK(request_);
+  if (request_->IsPageActivation())
+    return false;
+
   return IsURLHandledByNetworkStack(navigation_url_);
 }
 
+bool NavigationSimulatorImpl::NeedsPreCommitChecks() const {
+  DCHECK(request_);
+  return NeedsThrottleChecks() || request_->IsPageActivation();
+}
+
 }  // namespace content

content/test/navigation_simulator_impl.h

@@ -210,18 +210,20 @@ class NavigationSimulatorImpl : public NavigationSimulator,
   // navigation failed synchronously.
   bool SimulateRendererInitiatedStart();
 
-  // This method will block waiting for throttle checks to complete if
-  // |auto_advance_|. Otherwise will just set up state for checking the result
-  // when the throttles end up finishing.
+  // This method will block waiting for the navigation to reach the next
+  // NavigationThrottle phase of the navigation to complete
+  // (StartRequest|Redirect|Failed|ProcessResponse) if |auto_advance_|. This
+  // waits until *after* throttle checks are run (if the navigation requires
+  // throttle checks).  If |!auto_advance_| this will just set up state for
+  // checking the result when the throttles end up finishing.
   void MaybeWaitForThrottleChecksComplete(base::OnceClosure complete_closure);
 
   // Like above but blocks waiting for the ReadyToCommit checks to complete.
   // This check calls ReadyToCommitComplete() when finished.
   void MaybeWaitForReadyToCommitCheckComplete();
 
-  // Sets |last_throttle_check_result_| and calls both the
-  // |wait_closure_| and the |throttle_checks_complete_closure_|, if they are
-  // set.
+  // Sets |last_throttle_check_result_| and calls both the |wait_closure_| and
+  // the |throttle_checks_complete_closure_|, if they are set.
   bool OnThrottleChecksComplete(NavigationThrottle::ThrottleCheckResult result);
 
   // Helper method to set the OnThrottleChecksComplete callback on the
@@ -252,8 +254,16 @@ class NavigationSimulatorImpl : public NavigationSimulator,
   // - same-document navigations
   // - about:blank navigations
   // - navigations not handled by the network stack
+  // - page activations like prerendering and back-forward cache.
   bool NeedsThrottleChecks() const;
 
+  // Whether the navigation performs CommitDeferringCondition checks before
+  // committing. i.e. if it goes through the full
+  // WillStartRequest->WillProcessResponse->etc.->Commit phases. This includes
+  // all navigations that require throttle checks plus page activations like
+  // prerendering/BFCache.
+  bool NeedsPreCommitChecks() const;
+
   enum State {
     INITIALIZATION,
     WAITING_BEFORE_UNLOAD,
@@ -366,10 +376,9 @@ class NavigationSimulatorImpl : public NavigationSimulator,
   // result. Calling this will quit the nested run loop.
   base::OnceClosure wait_closure_;
 
-  // This member simply ensures that we do not disconnect
-  // the NavigationClient interface, as it would be interpreted as a
-  // cancellation coming from the renderer process side. This member interface
-  // will never be bound.
+  // This member simply ensures that we do not disconnect the NavigationClient
+  // interface, as it would be interpreted as a cancellation coming from the
+  // renderer process side. This member interface will never be bound.
   mojo::PendingAssociatedReceiver<mojom::NavigationClient>
       navigation_client_receiver_;
 

docs/navigation.md

@@ -160,5 +160,10 @@ They are typically registered in
 `NavigationThrottleRunner::RegisterNavigationThrottles` or
 `ContentBrowserClient::CreateThrottlesForNavigation`.
 
+NavigationThrottles are only invoked on navigations that require a URLLoader
+(see NavigationRequest::NeedsUrlLoader).  This means they don't typically run in
+cases like same-document navigations, about:blank, etc. They are also not run in
+page-activation navigations, such as activating a prerendered page or restoring
+a page from the back-forward cache.
 
 [WebContentsObserver]: https://source.chromium.org/chromium/chromium/src/+/main:content/public/browser/web_contents_observer.h