Teach navigation throttles how to cancel requests in WillProcessResponse.

content/browser/loader/navigation_resource_throttle.cc

 // Copyright 2015 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/browser/loader/navigation_resource_throttle.h"
 
 #include "base/callback.h"
+#include "base/logging.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/resource_controller.h"
 #include "content/public/browser/resource_request_info.h"
 #include "content/public/common/referrer.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_job_factory.h"
 #include "ui/base/page_transition_types.h"
 
 namespace content {
 
 namespace {
 typedef base::Callback<void(NavigationThrottle::ThrottleCheckResult)>
     UIChecksPerformedCallback;
 
 void SendCheckResultToIOThread(UIChecksPerformedCallback callback,
                                NavigationThrottle::ThrottleCheckResult result) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  CHECK(result != NavigationThrottle::DEFER);
+  CHECK_NE(result, NavigationThrottle::DEFER);

[clamy, 2016/01/26 14:08:55]

DCHECK? I think we're trying to avoid CHECKs in the code.

[Mike West, 2016/01/27 07:44:33]

Done.

   BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                           base::Bind(callback, result));
 }
+
+void MaybeCommitNavigationAndSendCheckResultToIOThread(
+    UIChecksPerformedCallback callback,
+    int render_process_id,
+    int render_frame_host_id,
+    scoped_refptr<net::HttpResponseHeaders> headers,
+    NavigationThrottle::ThrottleCheckResult result) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  CHECK_NE(result, NavigationThrottle::DEFER);

[mmenke, 2016/01/26 16:40:08]

DCHECK here, as well, I assume.

[Mike West, 2016/01/27 07:44:33]

Done.

+  if (result == NavigationThrottle::PROCEED) {
+    RenderFrameHostImpl* render_frame_host =
+        RenderFrameHostImpl::FromID(render_process_id, render_frame_host_id);
+    if (render_frame_host && render_frame_host->navigation_handle()) {
+      render_frame_host->navigation_handle()->ReadyToCommitNavigation(
+          render_frame_host, headers);

[mmenke, 2016/01/26 16:40:08]

This seems weird.  We call into the NavigationHandle, and tell it to call us
back.  If it calls us back, and it told us to proceed, we then call *back* into
the NavigationHandle, telling it that it told us to proceed?

[Mike West, 2016/01/27 07:44:33]

As I noted in my response to clamy@, it made sense to me to think of the
NavigationResourceThrottle as being in control of the navigation, and the
NavigationHandle as something of a state machine. But both of you have flagged
this as being weird, so I'm happy to agree. :)

+    }
+  }
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(callback, result));
+}
 
 void CheckWillStartRequestOnUIThread(UIChecksPerformedCallback callback,
                                      int render_process_id,
                                      int render_frame_host_id,
                                      bool is_post,
                                      const Referrer& sanitized_referrer,
                                      bool has_user_gesture,
                                      ui::PageTransition transition,
                                      bool is_external_protocol) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderFrameHostImpl* render_frame_host =
       RenderFrameHostImpl::FromID(render_process_id, render_frame_host_id);
   if (!render_frame_host) {
     SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED);
     return;
   }
 
   NavigationHandleImpl* navigation_handle =
       render_frame_host->navigation_handle();
   if (!navigation_handle) {

[mmenke, 2016/01/26 16:40:08]

How certain are we that this is "our" navigation handle, and not the handle for
a new navigation?

[clamy, 2016/01/26 16:48:05]

Because the ResourceRequest is started after the browser creates the
NavigationHandle and stopped before it destroys it.

     SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED);
     return;
   }
 
   navigation_handle->WillStartRequest(
       is_post, sanitized_referrer, has_user_gesture, transition,
       is_external_protocol, base::Bind(&SendCheckResultToIOThread, callback));
 }
 
 void CheckWillRedirectRequestOnUIThread(
@@ skipping 23 matching lines @@
   GURL new_validated_url(new_url);
   RenderProcessHost::FromID(render_process_id)
       ->FilterURL(false, &new_validated_url);
   navigation_handle->WillRedirectRequest(
       new_validated_url, new_method_is_post, new_referrer_url,
       new_is_external_protocol, headers,
       base::Bind(&SendCheckResultToIOThread, callback));
 }
 
 void WillProcessResponseOnUIThread(
+    UIChecksPerformedCallback callback,
     int render_process_id,
     int render_frame_host_id,
     scoped_refptr<net::HttpResponseHeaders> headers) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderFrameHostImpl* render_frame_host =
       RenderFrameHostImpl::FromID(render_process_id, render_frame_host_id);
-  if (!render_frame_host)
+  if (!render_frame_host) {
+    SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED);
     return;
+  }
 
   NavigationHandleImpl* navigation_handle =
       render_frame_host->navigation_handle();

[mmenke, 2016/01/26 16:40:08]

How sure are we that this is our navigation handle, and not that for a new
navigation?  I don't see a cancellation path.

-  if (!navigation_handle)
+  if (!navigation_handle) {
+    SendCheckResultToIOThread(callback, NavigationThrottle::PROCEED);
     return;
+  }
 
-  navigation_handle->ReadyToCommitNavigation(render_frame_host, headers);
+  navigation_handle->WillProcessResponse(
+      headers,
+      base::Bind(&MaybeCommitNavigationAndSendCheckResultToIOThread, callback,
+                 render_process_id, render_frame_host_id, headers));

[mmenke, 2016/01/26 16:40:08]

Hrm...Does seem kinda weird to me that the NavigationHandle doesn't know its own
render frame IDs

[clamy, 2016/01/26 16:48:05]

This is because the NavigationHandle was built as an interface that works both
with the current navigation architecture and with PlzNavigate. In PlzNavigate
you only get a RFH after processing the response, so the NavigationHandle does
not know it before that point.

 }
 
 }  // namespace
 
 NavigationResourceThrottle::NavigationResourceThrottle(net::URLRequest* request)
     : request_(request), weak_ptr_factory_(this) {}
 
 NavigationResourceThrottle::~NavigationResourceThrottle() {}
 
 void NavigationResourceThrottle::WillStartRequest(bool* defer) {
@@ skipping 76 matching lines @@
     return;
 
   // Send a copy of the response headers to the NavigationHandle on the UI
   // thread.
   scoped_refptr<net::HttpResponseHeaders> response_headers;
   if (request_->response_headers()) {
     response_headers = new net::HttpResponseHeaders(
         request_->response_headers()->raw_headers());
   }
 
+  UIChecksPerformedCallback callback =
+      base::Bind(&NavigationResourceThrottle::OnUIChecksPerformed,
+                 weak_ptr_factory_.GetWeakPtr());
+
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&WillProcessResponseOnUIThread, render_process_id,
+      base::Bind(&WillProcessResponseOnUIThread, callback, render_process_id,
                  render_frame_id, response_headers));
+  *defer = true;
 }
 
 const char* NavigationResourceThrottle::GetNameForLogging() const {
   return "NavigationResourceThrottle";
 }
 
 void NavigationResourceThrottle::OnUIChecksPerformed(
     NavigationThrottle::ThrottleCheckResult result) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (result == NavigationThrottle::CANCEL_AND_IGNORE) {
     controller()->CancelAndIgnore();
   } else if (result == NavigationThrottle::CANCEL) {
     controller()->Cancel();
   } else {
     controller()->Resume();
   }
 }
 
 }  // namespace content