Refactor ResourceHandler API.

content/browser/loader/navigation_resource_handler.cc

 // Copyright 2014 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_handler.h"
 
 #include <memory>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "content/browser/loader/navigation_url_loader_impl_core.h"
 #include "content/browser/loader/netlog_observer.h"
 #include "content/browser/loader/resource_controller.h"
 #include "content/browser/loader/resource_loader.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/resource_context_impl.h"
 #include "content/browser/streams/stream.h"
 #include "content/browser/streams/stream_context.h"
 #include "content/public/browser/navigation_data.h"
@@ skipping 27 matching lines @@
 }
 
 NavigationResourceHandler::~NavigationResourceHandler() {
   if (core_) {
     core_->NotifyRequestFailed(false, net::ERR_ABORTED);
     DetachFromCore();
   }
 }
 
 void NavigationResourceHandler::Cancel() {
-  controller()->Cancel();
-  core_ = nullptr;
+  if (core_) {
+    core_ = nullptr;
+    OutOfBandCancel(net::ERR_ABORTED, true);
+  }
 }
 
 void NavigationResourceHandler::FollowRedirect() {
-  controller()->Resume();
+  Resume();
 }
 
 void NavigationResourceHandler::ProceedWithResponse() {
   // Detach from the loader; at this point, the request is now owned by the
   // StreamHandle sent in OnResponseStarted.
   DetachFromCore();
-  controller()->Resume();
+  Resume();
 }
 
-void NavigationResourceHandler::SetController(ResourceController* controller) {
-  writer_.set_controller(controller);
-  ResourceHandler::SetController(controller);
-}
-
-bool NavigationResourceHandler::OnRequestRedirected(
+void NavigationResourceHandler::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
     ResourceResponse* response,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   DCHECK(core_);
+  DCHECK(!has_controller());
 
   // TODO(davidben): Perform a CSP check here, and anything else that would have
   // been done renderer-side.
   NetLogObserver::PopulateResponseInfo(request(), response);
   response->head.encoded_data_length = request()->GetTotalReceivedBytes();
   core_->NotifyRequestRedirected(redirect_info, response);
-  *defer = true;
-  return true;
+
+  HoldController(std::move(controller));
 }
 
-bool NavigationResourceHandler::OnResponseStarted(ResourceResponse* response,
-                                                  bool* defer) {
+void NavigationResourceHandler::OnResponseStarted(
+    ResourceResponse* response,
+    std::unique_ptr<ResourceController> controller) {
   DCHECK(core_);
+  DCHECK(!has_controller());
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
 
   StreamContext* stream_context =
       GetStreamContextForResourceContext(info->GetContext());
-  writer_.InitializeStream(stream_context->registry(),
-                           request()->url().GetOrigin());
+  writer_.InitializeStream(
+      stream_context->registry(), request()->url().GetOrigin(),
+      base::Bind(&NavigationResourceHandler::OutOfBandCancel,
+                 base::Unretained(this), net::ERR_ABORTED, true));
 
   NetLogObserver::PopulateResponseInfo(request(), response);
   response->head.encoded_data_length = request()->raw_header_size();
 
   std::unique_ptr<NavigationData> cloned_data;
   if (resource_dispatcher_host_delegate_) {
     // Ask the embedder for a NavigationData instance.
     NavigationData* navigation_data =
         resource_dispatcher_host_delegate_->GetNavigationData(request());
 
@@ skipping 18 matching lines @@
   // Make sure that the requests go through the throttle checks. Currently this
   // does not work as the InterceptingResourceHandler is above us and hence it
   // does not expect the old handler to defer the request.
   // TODO(clamy): We should also make the downloads wait on the
   // NavigationThrottle checks be performed. Similarly to streams, it doesn't
   // work because of the InterceptingResourceHandler.
   // TODO(clamy): This NavigationResourceHandler should be split in two, with
   // one part that wait on the NavigationThrottle to execute located between the
   // MIME sniffing and the ResourceThrotlle, and one part that write the
   // response to the stream being the leaf ResourceHandler.
-  if (!info->is_stream() && !info->IsDownload())
-    *defer = true;
-
-  return true;
+  if (!info->is_stream() && !info->IsDownload()) {

[Charlie Harrison, 2017/01/25 20:22:59]

nit: Invert the if statement:

if (info->is_stream() || info->IsDownload())
  controller->Resume()
else
  HoldController(std::move(controller))

Makes a lot more sense (resume streams / downloads, hold everything else).

[mmenke, 2017/01/25 22:07:59]

Done.  Interesting, I think of Resume() as more of the success case / continue
doing stuff case, and HoldController as more of the pseudo-error case, so think
the opposite order makes more sense (Though x || y is much clearer than !x &&
!y)

+    HoldController(std::move(controller));
+  } else {
+    controller->Resume();
+  }
 }
 
-bool NavigationResourceHandler::OnWillStart(const GURL& url, bool* defer) {
-  return true;
+void NavigationResourceHandler::OnWillStart(
+    const GURL& url,
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(!has_controller());
+  controller->Resume();
 }
 
 bool NavigationResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
                                            int* buf_size,
                                            int min_size) {
+  DCHECK(!has_controller());
   writer_.OnWillRead(buf, buf_size, min_size);
   return true;
 }
 
-bool NavigationResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
-  writer_.OnReadCompleted(bytes_read, defer);
-  return true;
+void NavigationResourceHandler::OnReadCompleted(
+    int bytes_read,
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(!has_controller());
+  writer_.OnReadCompleted(bytes_read,
+                          base::Bind(&ResourceController::Resume,
+                                     base::Passed(std::move(controller))));
 }
 
 void NavigationResourceHandler::OnResponseCompleted(
     const net::URLRequestStatus& status,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   // If the request has already committed, close the stream and leave it as-is.
   if (writer_.stream()) {
     writer_.Finalize(status.error());
+    controller->Resume();
     return;
   }
 
   if (core_) {
     DCHECK_NE(net::OK, status.error());
     core_->NotifyRequestFailed(request()->response_info().was_cached,
                                status.error());
     DetachFromCore();
   }
+  controller->Resume();
 }
 
 void NavigationResourceHandler::OnDataDownloaded(int bytes_downloaded) {
   NOTREACHED();
 }
 
 void NavigationResourceHandler::DetachFromCore() {
   DCHECK(core_);
   core_->set_resource_handler(nullptr);
   core_ = nullptr;
 }
 
 }  // namespace content