Prevent RFH from attempting to transfer after it's been swapped out

content/public/test/browser_test_utils.cc

Index: content/public/test/browser_test_utils.cc
diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc
index 5211cafb5b23381c965be8cde80b84cff3550b8c..5e8bcb3c67ecb1eadb5ce3596b117feb66e47238 100644
--- a/content/public/test/browser_test_utils.cc
+++ b/content/public/test/browser_test_utils.cc
@@ -361,9 +361,11 @@ CrossSiteRedirectResponseHandler(const GURL& server_base_url,
 class TestNavigationManagerThrottle : public NavigationThrottle {
  public:
   TestNavigationManagerThrottle(NavigationHandle* handle,
-                                base::Closure on_will_start_request_closure)
+                                base::Closure on_will_start_request_closure,
+                                base::Closure on_will_process_response_closure)
       : NavigationThrottle(handle),
-        on_will_start_request_closure_(on_will_start_request_closure) {}
+        on_will_start_request_closure_(on_will_start_request_closure),
+        on_will_process_response_closure_(on_will_process_response_closure) {}
   ~TestNavigationManagerThrottle() override {}
 
  private:
@@ -374,7 +376,14 @@ class TestNavigationManagerThrottle : public NavigationThrottle {
     return NavigationThrottle::DEFER;
   }
 
+  NavigationThrottle::ThrottleCheckResult WillProcessResponse() override {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            on_will_process_response_closure_);
+    return NavigationThrottle::DEFER;
+  }
+
   base::Closure on_will_start_request_closure_;
+  base::Closure on_will_process_response_closure_;
 };
 
 bool HasGzipHeader(const base::RefCountedMemory& maybe_gzipped) {
@@ -1636,7 +1645,8 @@ TestNavigationManager::TestNavigationManager(WebContents* web_contents,
                                              const GURL& url)
     : WebContentsObserver(web_contents),
       url_(url),
-      navigation_paused_(false),
+      navigation_paused_in_will_start_(false),
+      navigation_paused_in_will_process_response_(false),
       handle_(nullptr),
       handled_navigation_(false),
       weak_factory_(this) {}
@@ -1649,8 +1659,9 @@ bool TestNavigationManager::WaitForWillStartRequest() {
   DCHECK(!did_finish_loop_runner_);
   if (!handle_ && handled_navigation_)
     return true;
-  if (navigation_paused_)
+  if (navigation_paused_in_will_start_)
     return true;
+  DCHECK(!navigation_paused_in_will_process_response_);
   will_start_loop_runner_ = new MessageLoopRunner();
   will_start_loop_runner_->Run();
   will_start_loop_runner_ = nullptr;
@@ -1661,13 +1672,33 @@ bool TestNavigationManager::WaitForWillStartRequest() {
   return !handled_navigation_;
 }
 
+bool TestNavigationManager::WaitForWillProcessResponse() {
+  DCHECK(!did_finish_loop_runner_);
+  if (!handle_ && handled_navigation_)
+    return true;
+  if (navigation_paused_in_will_process_response_)
+    return true;
+  // Ensure the navigation is resumed if the manager paused it previously.
+  if (navigation_paused_in_will_start_)

[nasko, 2016/09/30 23:21:33]

How would that happen?

[clamy, 2016/10/04 11:54:41]

We always pause by default in WillStart since on PlzNavigate it may happen
synchronously when we start the navigation. Not pausing there all the time would
make us miss the event when the user really wants to pause the navigation before
it starts.

+    ResumeNavigation();
+  will_process_response_loop_runner_ = new MessageLoopRunner();
+  will_process_response_loop_runner_->Run();
+  will_process_response_loop_runner_ = nullptr;
+
+  // This will only be false if DidFinishNavigation is called before
+  // OnWillProcessResponse.
+  return !handled_navigation_;
+}
+
 void TestNavigationManager::WaitForNavigationFinished() {
   DCHECK(!will_start_loop_runner_);
   if (!handle_ && handled_navigation_)
     return;
   // Ensure the navigation is resumed if the manager paused it previously.
-  if (navigation_paused_)
+  if (navigation_paused_in_will_start_ ||
+      navigation_paused_in_will_process_response_) {
     ResumeNavigation();
+  }
   did_finish_loop_runner_ = new MessageLoopRunner();
   did_finish_loop_runner_->Run();
   did_finish_loop_runner_ = nullptr;
@@ -1681,7 +1712,9 @@ void TestNavigationManager::DidStartNavigation(NavigationHandle* handle) {
   std::unique_ptr<NavigationThrottle> throttle(
       new TestNavigationManagerThrottle(
           handle_, base::Bind(&TestNavigationManager::OnWillStartRequest,
-                              weak_factory_.GetWeakPtr())));
+                              weak_factory_.GetWeakPtr()),
+          base::Bind(&TestNavigationManager::OnWillProcessResponse,
+                     weak_factory_.GetWeakPtr())));
   handle_->RegisterThrottleForTesting(std::move(throttle));
 }
 
@@ -1690,7 +1723,8 @@ void TestNavigationManager::DidFinishNavigation(NavigationHandle* handle) {
     return;
   handle_ = nullptr;
   handled_navigation_ = true;
-  navigation_paused_ = false;
+  navigation_paused_in_will_start_ = false;
+  navigation_paused_in_will_process_response_ = false;
 
   // Resume any clients that are waiting for the end of the navigation. Note

[nasko, 2016/09/30 23:21:33]

I'm a bit confused on how we can get multiple message loops to be spinning
simultaneously. Maybe land this CL as is to ensure we make branch point, but
overall I'd like to understand this a bit better and see if it can be
simplified.

[clamy, 2016/10/04 11:54:41]

Well we shouldn't have them spinning, and this would probably be better
expressed as else if statements. I agree that it should be improved, but would
like to land this before branch point :).

   // that |will_start_loop_runner_| can be running if the navigation was
@@ -1699,22 +1733,39 @@ void TestNavigationManager::DidFinishNavigation(NavigationHandle* handle) {
     did_finish_loop_runner_->Quit();
   if (will_start_loop_runner_)
     will_start_loop_runner_->Quit();
+  if (will_process_response_loop_runner_)
+    will_process_response_loop_runner_->Quit();
 }
 
 void TestNavigationManager::OnWillStartRequest() {
-  navigation_paused_ = true;
+  navigation_paused_in_will_start_ = true;
   if (will_start_loop_runner_)
     will_start_loop_runner_->Quit();
 
-  // If waiting for the navigation to finish, resume the navigation.
+  // If waiting for further events in the navigation, resume the navigation.
+  if (did_finish_loop_runner_ || will_process_response_loop_runner_)
+    ResumeNavigation();
+}
+
+void TestNavigationManager::OnWillProcessResponse() {
+  navigation_paused_in_will_process_response_ = true;
+  DCHECK(!will_start_loop_runner_);
+  if (will_process_response_loop_runner_)
+    will_process_response_loop_runner_->Quit();
+
+  // If waiting for further events in the navigation, resume the navigation.
   if (did_finish_loop_runner_)
     ResumeNavigation();
 }
 
 void TestNavigationManager::ResumeNavigation() {
-  if (!navigation_paused_ || !handle_)
+  if (!(navigation_paused_in_will_start_ ||
+        navigation_paused_in_will_process_response_) ||
+      !handle_) {
     return;
-  navigation_paused_ = false;
+  }
+  navigation_paused_in_will_start_ = false;
+  navigation_paused_in_will_process_response_ = false;
   handle_->Resume();
 }