Allow asynchronous deferral in NavigationSimulator

content/public/test/navigation_simulator.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/public/test/navigation_simulator.h"
 
-#include "content/browser/frame_host/navigation_handle_impl.h"
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/run_loop.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/common/frame_messages.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/test/test_navigation_url_loader.h"
 #include "content/test/test_render_frame_host.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/redirect_info.h"
 
@@ skipping 36 matching lines @@
 
 }  // namespace
 
 // static
 void NavigationSimulator::NavigateAndCommitFromDocument(
     const GURL& original_url,
     RenderFrameHost* render_frame_host) {
   NavigationSimulator simulator(
       original_url, static_cast<TestRenderFrameHost*>(render_frame_host));
   simulator.Commit();
+  simulator.WaitForThrottleChecksComplete();
 }
 
 // static
 void NavigationSimulator::NavigateAndFailFromDocument(
     const GURL& original_url,
     int net_error_code,
     RenderFrameHost* render_frame_host) {
   NavigationSimulator simulator(
       original_url, static_cast<TestRenderFrameHost*>(render_frame_host));
   simulator.Fail(net_error_code);
@@ skipping 20 matching lines @@
   if (render_frame_host->GetParent()) {
     if (!render_frame_host->frame_tree_node()->has_committed_real_load())
       transition_ = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
     else
       transition_ = ui::PAGE_TRANSITION_MANUAL_SUBFRAME;
   }
 }
 
 NavigationSimulator::~NavigationSimulator() {}
 
-void NavigationSimulator::Start() {
+NavigationThrottle::ThrottleCheckResult NavigationSimulator::Start() {
   CHECK(state_ == INITIALIZATION)
       << "NavigationSimulator::Start should only be called once.";
   state_ = STARTED;
 
   if (IsBrowserSideNavigationEnabled()) {
     BeginNavigationParams begin_params(
         std::string(), net::LOAD_NORMAL, true /* has_user_gesture */,
         false /* skip_service_worker */, REQUEST_CONTEXT_TYPE_HYPERLINK,
         blink::WebMixedContentContextType::Blockable, url::Origin());
     CommonNavigationParams common_params;
     common_params.url = navigation_url_;
     common_params.referrer = referrer_;
     common_params.transition = transition_;
     common_params.navigation_type =
         PageTransitionCoreTypeIs(transition_, ui::PAGE_TRANSITION_RELOAD)
             ? FrameMsg_Navigate_Type::RELOAD
             : FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT;
     render_frame_host_->OnMessageReceived(FrameHostMsg_BeginNavigation(
         render_frame_host_->GetRoutingID(), common_params, begin_params));
     NavigationRequest* request =
         render_frame_host_->frame_tree_node()->navigation_request();
-    DCHECK(request);
+    // The request failed synchronously.
+    if (!request)
+      return last_throttle_check_result_.value();
     DCHECK_EQ(handle_, request->navigation_handle());
   } else {
     render_frame_host_->OnMessageReceived(
         FrameHostMsg_DidStartLoading(render_frame_host_->GetRoutingID(), true));
     render_frame_host_->OnMessageReceived(FrameHostMsg_DidStartProvisionalLoad(
         render_frame_host_->GetRoutingID(), navigation_url_,
         std::vector<GURL>(), base::TimeTicks::Now()));
+    DCHECK(handle_);
     DCHECK_EQ(handle_, render_frame_host_->navigation_handle());
     handle_->WillStartRequest(
         "GET", scoped_refptr<content::ResourceRequestBodyImpl>(), referrer_,
         true /* user_gesture */, transition_, false /* is_external_protocol */,
         REQUEST_CONTEXT_TYPE_LOCATION,
         blink::WebMixedContentContextType::NotMixedContent,
-        base::Callback<void(NavigationThrottle::ThrottleCheckResult)>());
+        GenerateCompleteCallback());
   }
-
   CHECK(handle_);
-
-  // Make sure all NavigationThrottles have run.
-  // TODO(clamy): provide a non auto-advance mode if needed.
-  while (handle_->state_for_testing() == NavigationHandleImpl::DEFERRING_START)
-    handle_->Resume();
-
-  CHECK_EQ(1, num_did_start_navigation_called_);
-  CHECK_EQ(1, num_will_start_request_called_);
+  return RunOrStoreThrottleCompleteCallback(base::Bind(
+      &NavigationSimulator::FinishStart, weak_factory_.GetWeakPtr()));
 }
 
-void NavigationSimulator::Redirect(const GURL& new_url) {
+void NavigationSimulator::FinishStart(
+    NavigationThrottle::ThrottleCheckResult result) {
+  CHECK_EQ(1, num_did_start_navigation_called_);
+  if (result == NavigationThrottle::PROCEED) {
+    CHECK_EQ(1, num_will_start_request_called_);
+  } else {
+    state_ = FAILED;
+  }
+}
+
+NavigationThrottle::ThrottleCheckResult NavigationSimulator::Redirect(
+    const GURL& new_url) {
   CHECK(state_ <= STARTED) << "NavigationSimulator::Redirect should be "
                               "called before Fail or Commit";
   CHECK_EQ(0, num_did_finish_navigation_called_)
       << "NavigationSimulator::Redirect cannot be called after the "
          "navigation has finished";
 
   if (state_ == INITIALIZATION)
-    Start();
+    StartAndExpectToProceed();
 
   navigation_url_ = new_url;
 
   int previous_num_will_redirect_request_called =
       num_will_redirect_request_called_;
   int previous_did_redirect_navigation_called =
       num_did_redirect_navigation_called_;
 
+  last_throttle_check_result_.reset();
   if (IsBrowserSideNavigationEnabled()) {
     NavigationRequest* request =
         render_frame_host_->frame_tree_node()->navigation_request();
+    request->set_on_checks_complete_callback_for_testing(
+        GenerateCompleteCallback());
     TestNavigationURLLoader* url_loader =
         static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
     CHECK(url_loader);
 
     net::RedirectInfo redirect_info;
     redirect_info.status_code = 302;
     redirect_info.new_method = "GET";
     redirect_info.new_url = new_url;
     redirect_info.new_first_party_for_cookies = new_url;
     redirect_info.new_referrer = referrer_.url.spec();
     redirect_info.new_referrer_policy =
         Referrer::ReferrerPolicyForUrlRequest(referrer_);
 
     url_loader->CallOnRequestRedirected(
         redirect_info, scoped_refptr<ResourceResponse>(new ResourceResponse));
   } else {
     handle_->WillRedirectRequest(
         new_url, "GET", referrer_.url, false /* is_external_protocol */,
         scoped_refptr<net::HttpResponseHeaders>(),
-        net::HttpResponseInfo::ConnectionInfo(),
-        base::Callback<void(NavigationThrottle::ThrottleCheckResult)>());
+        net::HttpResponseInfo::ConnectionInfo(), GenerateCompleteCallback());
   }
-
-  // Make sure all NavigationThrottles have run.
-  // TODO(clamy): provide a non auto-advance mode if needed.
-  while (handle_->state_for_testing() ==
-         NavigationHandleImpl::DEFERRING_REDIRECT) {
-    handle_->Resume();
-  }
-
-  CHECK_EQ(previous_num_will_redirect_request_called + 1,
-           num_will_redirect_request_called_);
-  CHECK_EQ(previous_did_redirect_navigation_called + 1,
-           num_did_redirect_navigation_called_);
+  return RunOrStoreThrottleCompleteCallback(base::Bind(
+      &NavigationSimulator::FinishRedirect, weak_factory_.GetWeakPtr(),
+      previous_num_will_redirect_request_called,
+      previous_did_redirect_navigation_called));
 }
 
-void NavigationSimulator::Commit() {
+void NavigationSimulator::FinishRedirect(
+    int previous_num_will_redirect_request_called,
+    int previous_did_redirect_navigation_called,
+    NavigationThrottle::ThrottleCheckResult result) {
+  if (result == NavigationThrottle::PROCEED) {
+    CHECK_EQ(previous_num_will_redirect_request_called + 1,
+             num_will_redirect_request_called_);
+    CHECK_EQ(previous_did_redirect_navigation_called + 1,
+             num_did_redirect_navigation_called_);
+  } else {
+    state_ = FAILED;
+  }
+}
+
+NavigationThrottle::ThrottleCheckResult NavigationSimulator::Commit() {
   CHECK_LE(state_, STARTED) << "NavigationSimulator::Commit can only be "
                                "called once, and cannot be called after "
                                "NavigationSimulator::Fail";
   CHECK_EQ(0, num_did_finish_navigation_called_)
       << "NavigationSimulator::Commit cannot be called after the "
          "navigation has finished";
 
   if (state_ == INITIALIZATION)
-    Start();
+    StartAndExpectToProceed();
 
-  if (IsBrowserSideNavigationEnabled() &&
-      render_frame_host_->frame_tree_node()->navigation_request()) {
-    render_frame_host_->PrepareForCommit();
+  last_throttle_check_result_.reset();
+  if (IsBrowserSideNavigationEnabled()) {
+    NavigationRequest* request =
+        render_frame_host_->frame_tree_node()->navigation_request();
+    if (request) {
+      request->set_on_checks_complete_callback_for_testing(
+          GenerateCompleteCallback());
+      render_frame_host_->PrepareForCommit();
+    }
   }
 
   // Call NavigationHandle::WillProcessResponse if needed.
-  if (handle_->state_for_testing() <
-      NavigationHandleImpl::WILL_PROCESS_RESPONSE) {
+  // Note that the handle's state can be CANCELING if a throttle cancelled it
+  // synchronously in PrepareForCommit.
+  if (handle_->state_for_testing() < NavigationHandleImpl::CANCELING) {
     handle_->WillProcessResponse(
         render_frame_host_, scoped_refptr<net::HttpResponseHeaders>(),
         net::HttpResponseInfo::ConnectionInfo(), SSLStatus(), GlobalRequestID(),
         false /* should_replace_current_entry */, false /* is_download */,
-        false /* is_stream */, base::Closure(),
-        base::Callback<void(NavigationThrottle::ThrottleCheckResult)>());
+        false /* is_stream */, base::Closure(), GenerateCompleteCallback());
   }
+  return RunOrStoreThrottleCompleteCallback(base::Bind(
+      &NavigationSimulator::FinishCommit, weak_factory_.GetWeakPtr()));
+}
 
-  // Make sure all NavigationThrottles have run.
-  // TODO(clamy): provide a non auto-advance mode if needed.
-  while (handle_->state_for_testing() ==
-         NavigationHandleImpl::DEFERRING_RESPONSE) {
-    handle_->Resume();
+void NavigationSimulator::FinishCommit(
+    NavigationThrottle::ThrottleCheckResult result) {
+  if (result != NavigationThrottle::PROCEED) {
+    state_ = FAILED;
+    return;
   }
-
   CHECK_EQ(1, num_will_process_response_called_);
   CHECK_EQ(1, num_ready_to_commit_called_);
 
   // Update the RenderFrameHost now that we know which RenderFrameHost will
   // commit the navigation.
   TestRenderFrameHost* new_render_frame_host =
       static_cast<TestRenderFrameHost*>(handle_->GetRenderFrameHost());
   if (!IsBrowserSideNavigationEnabled() &&
       new_render_frame_host != render_frame_host_) {
     CHECK(handle_->is_transferring());
@@ skipping 49 matching lines @@
 }
 
 void NavigationSimulator::Fail(int error_code) {
   CHECK_LE(state_, STARTED) << "NavigationSimulator::Fail can only be "
                                "called.";
   CHECK_EQ(0, num_did_finish_navigation_called_)
       << "NavigationSimulator::Fail cannot be called after the "
          "navigation has finished";
 
   if (state_ == INITIALIZATION)
-    Start();
+    StartAndExpectToProceed();
 
   state_ = FAILED;
 
   bool should_result_in_error_page = error_code != net::ERR_ABORTED;
   if (IsBrowserSideNavigationEnabled()) {
     NavigationRequest* request =
         render_frame_host_->frame_tree_node()->navigation_request();
     CHECK(request);
     TestNavigationURLLoader* url_loader =
         static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
@@ skipping 115 matching lines @@
       << "The transition cannot be set after the navigation has started";
   transition_ = transition;
 }
 
 void NavigationSimulator::SetReferrer(const Referrer& referrer) {
   CHECK_LE(state_, STARTED) << "The referrer cannot be set after the "
                                "navigation has committed or has failed";
   referrer_ = referrer;
 }
 
+NavigationThrottle::ThrottleCheckResult
+NavigationSimulator::WaitForThrottleChecksComplete() {
+  if (!last_throttle_check_result_) {
+    base::RunLoop run_loop;
+    throttle_checks_wait_closure_ = run_loop.QuitClosure();
+    run_loop.Run();
+    throttle_checks_wait_closure_.Reset();
+  }
+  return last_throttle_check_result_.value();
+}
+
 void NavigationSimulator::DidStartNavigation(
     NavigationHandle* navigation_handle) {
   // Check if this navigation is the one we're simulating.
   if (handle_)
     return;
 
   if (navigation_handle->GetURL() != navigation_url_)
     return;
 
   NavigationHandleImpl* handle =
       static_cast<NavigationHandleImpl*>(navigation_handle);
 
   if (handle->frame_tree_node() != render_frame_host_->frame_tree_node())
     return;
 
   handle_ = handle;
 
   num_did_start_navigation_called_++;
 
   // Add a throttle to count NavigationThrottle calls count.
   handle->RegisterThrottleForTesting(
       base::MakeUnique<NavigationThrottleCallbackRunner>(
           handle,
           base::Bind(&NavigationSimulator::OnWillStartRequest,
                      weak_factory_.GetWeakPtr()),
           base::Bind(&NavigationSimulator::OnWillRedirectRequest,
                      weak_factory_.GetWeakPtr()),
           base::Bind(&NavigationSimulator::OnWillProcessResponse,
                      weak_factory_.GetWeakPtr())));
+
+  if (IsBrowserSideNavigationEnabled()) {

[clamy, 2017/03/06 15:22:15]

What do you think of the previous suggestion to store a special test callback in
NavigationHandle. We can add it here, and we won't have to special case
PlzNavigate or the current architecture when generating the callbacks.

[Charlie Harrison, 2017/03/06 20:18:43]

Done.

+    NavigationRequest* request =
+        render_frame_host_->frame_tree_node()->navigation_request();
+    if (request) {
+      request->set_on_checks_complete_callback_for_testing(
+          GenerateCompleteCallback());
+    }
+  }
 }
 
 void NavigationSimulator::DidRedirectNavigation(
     NavigationHandle* navigation_handle) {
   if (navigation_handle == handle_)
     num_did_redirect_navigation_called_++;
 }
 
 void NavigationSimulator::ReadyToCommitNavigation(
     NavigationHandle* navigation_handle) {
@@ skipping 12 matching lines @@
 }
 
 void NavigationSimulator::OnWillRedirectRequest() {
   num_will_redirect_request_called_++;
 }
 
 void NavigationSimulator::OnWillProcessResponse() {
   num_will_process_response_called_++;
 }
 
+// These methods are a little subtle. Overall, the design constraints are:
+// 1. Synchronous throttle checks should not cause subsequent
+// WaitForThrottleChecksComplete to block.
+//
+// 2. |complete_callback_| is called after all synchronous code is executed in
+// response to the throttle checks. This point is important because in some
+// cases (e.g. cancellation), the OnThrottleChecksComplete callback  that the
+// NavigationRequest/NavigationHandle holds on to will be called before things
+// like navigation tear down and DidFinishNavigation. This code ensures that
+// |complete_callback_| is called after that occurs, either synchronously in
+// RunOrStoreThrottleCompleteCallback, or asynchronously in
+// OnThrottleChecksComplete.
+NavigationThrottle::ThrottleCheckResult
+NavigationSimulator::RunOrStoreThrottleCompleteCallback(
+    ThrottleChecksFinishedCallback finish_callback) {
+  // If completed the checks synchronously, call Finish* here. Else, save the
+  // callback to call asynchronously when the checks finish.
+  if (last_throttle_check_result_) {
+    finish_callback.Run(last_throttle_check_result_.value());
+    return last_throttle_check_result_.value();
+  }
+  complete_callback_ = std::move(finish_callback);
+  return NavigationThrottle::DEFER;
+}
+
+void NavigationSimulator::OnThrottleChecksComplete(
+    NavigationThrottle::ThrottleCheckResult result) {
+  // For synchronous throttle checks, set this here, and read from it downstream
+  // in RunOrStoreThrottleCompleteCallback.
+  DCHECK(!last_throttle_check_result_);
+  last_throttle_check_result_ = result;
+  if (!complete_callback_)
+    return;
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(std::move(complete_callback_), result),
+      throttle_checks_wait_closure_);
+}
+
+ThrottleChecksFinishedCallback NavigationSimulator::GenerateCompleteCallback() {
+  return base::Bind(&NavigationSimulator::OnThrottleChecksComplete,
+                    weak_factory_.GetWeakPtr());
+}
+
+void NavigationSimulator::StartAndExpectToProceed() {
+  NavigationThrottle::ThrottleCheckResult result = Start();
+  if (result == NavigationThrottle::DEFER)
+    result = WaitForThrottleChecksComplete();
+  CHECK_EQ(NavigationThrottle::PROCEED, result);
+}
+
 }  // namespace content