Refactor ResourceHandler API.

content/browser/loader/async_resource_handler.cc

Index: content/browser/loader/async_resource_handler.cc
diff --git a/content/browser/loader/async_resource_handler.cc b/content/browser/loader/async_resource_handler.cc
index 69df993843453e1105111ba1193f1c8dc8fe3edd..162345836480fbde18d24a78c0967b598b8a2334 100644
--- a/content/browser/loader/async_resource_handler.cc
+++ b/content/browser/loader/async_resource_handler.cc
@@ -296,16 +296,17 @@ void AsyncResourceHandler::ReportUploadProgress() {
   }
 }
 
-bool AsyncResourceHandler::OnRequestRedirected(
+void AsyncResourceHandler::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
     ResourceResponse* response,
-    bool* defer) {
-  const ResourceRequestInfoImpl* info = GetRequestInfo();
-  if (!info->filter())
-    return false;
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(!has_controller());
 
-  *defer = did_defer_ = true;
-  OnDefer();
+  const ResourceRequestInfoImpl* info = GetRequestInfo();
+  if (!info->filter()) {
+    controller->Cancel();
+    return;
+  }
 
   NetLogObserver::PopulateResponseInfo(request(), response);
   response->head.encoded_data_length = request()->GetTotalReceivedBytes();
@@ -316,24 +317,34 @@ bool AsyncResourceHandler::OnRequestRedirected(
   // cookies? The only case where it can change is top-level navigation requests
   // and hopefully those will eventually all be owned by the browser. It's
   // possible this is still needed while renderer-owned ones exist.
-  return info->filter()->Send(new ResourceMsg_ReceivedRedirect(
-      GetRequestID(), redirect_info, response->head));
+  if (!info->filter()->Send(new ResourceMsg_ReceivedRedirect(
+          GetRequestID(), redirect_info, response->head))) {
+    controller->Cancel();
+  } else {
+    did_defer_ = true;
+    OnDefer();
+    set_controller(std::move(controller));
+  }
 }
 
-bool AsyncResourceHandler::OnResponseStarted(ResourceResponse* response,
-                                             bool* defer) {
+void AsyncResourceHandler::OnResponseStarted(
+    ResourceResponse* response,
+    std::unique_ptr<ResourceController> controller) {
   // For changes to the main frame, inform the renderer of the new URL's
   // per-host settings before the request actually commits.  This way the
   // renderer will be able to set these precisely at the time the
   // request commits, avoiding the possibility of e.g. zooming the old content
   // or of having to layout the new content twice.
+  DCHECK(!has_controller());
 
   response_started_ticks_ = base::TimeTicks::Now();
 
   progress_timer_.Stop();
   const ResourceRequestInfoImpl* info = GetRequestInfo();
-  if (!info->filter())
-    return false;
+  if (!info->filter()) {
+    Cancel();
+    return;
+  }
 
   // We want to send a final upload progress message prior to sending the
   // response complete message even if we're waiting for an ack to to a
@@ -374,10 +385,12 @@ bool AsyncResourceHandler::OnResponseStarted(ResourceResponse* response,
   }
 
   inlining_helper_->OnResponseReceived(*response);
-  return true;
+  controller->Resume();
 }
 
-bool AsyncResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+void AsyncResourceHandler::OnWillStart(
+    const GURL& url,
+    std::unique_ptr<ResourceController> controller) {
   if (GetRequestInfo()->is_upload_progress_enabled() &&
       request()->has_upload()) {
     ReportUploadProgress();
@@ -387,14 +400,16 @@ bool AsyncResourceHandler::OnWillStart(const GURL& url, bool* defer) {
         this,
         &AsyncResourceHandler::ReportUploadProgress);
   }
-  return true;
+  controller->Resume();
 }
 
 bool AsyncResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
                                       int* buf_size,
                                       int min_size) {
+  DCHECK(!has_controller());
   DCHECK_EQ(-1, min_size);
 
+  // TODO(mmenke):  Should fail with ERR_INSUFFICIENT_RESOURCES here.

[Randy Smith (Not in Mondays), 2016/12/16 21:37:26]

Is there a reason why CancelWithError() can't be called from here?  (This may be
"haven't gotten to it but plan to in this CL" issue, in which case feel free to
ignore this comment.)

[mmenke, 2016/12/22 16:29:35]

We don't have a ResourceController to call it on.  OnWillRead always completes
synchronously and returns a bool.  I want to make it take a controller like
everything else, but that's well beyond the scope of this CL.  The fact that
this code called cancel and returned false is a bug.

   if (!CheckForSufficientResource())
     return false;
 
@@ -416,15 +431,22 @@ bool AsyncResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
   return true;
 }
 
-bool AsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+void AsyncResourceHandler::OnReadCompleted(
+    int bytes_read,
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(!has_controller());
   DCHECK_GE(bytes_read, 0);
 
-  if (!bytes_read)
-    return true;
+  if (!bytes_read) {
+    controller->Resume();
+    return;
+  }
 
   ResourceMessageFilter* filter = GetFilter();
-  if (!filter)
-    return false;
+  if (!filter) {
+    controller->Cancel();
+    return;
+  }
 
   int encoded_data_length = CalculateEncodedDataLengthToReport();
   if (!first_chunk_read_)
@@ -436,16 +458,20 @@ bool AsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
   // Return early if InliningHelper handled the received data.
   if (inlining_helper_->SendInlinedDataIfApplicable(
           bytes_read, encoded_data_length, encoded_body_length, filter,
-          GetRequestID()))
-    return true;
+          GetRequestID())) {
+    controller->Resume();
+    return;
+  }
 
   buffer_->ShrinkLastAllocation(bytes_read);
 
   if (!sent_data_buffer_msg_) {
     base::SharedMemoryHandle handle = base::SharedMemory::DuplicateHandle(
         buffer_->GetSharedMemory().handle());
-    if (!base::SharedMemory::IsHandleValid(handle))
-      return false;
+    if (!base::SharedMemory::IsHandleValid(handle)) {
+      controller->Cancel();
+      return;
+    }
     filter->Send(new ResourceMsg_SetDataBuffer(
         GetRequestID(), handle, buffer_->GetSharedMemory().mapped_size(),
         filter->peer_pid()));
@@ -460,11 +486,12 @@ bool AsyncResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
   ++pending_data_count_;
 
   if (!buffer_->CanAllocate()) {
-    *defer = did_defer_ = true;
+    did_defer_ = true;
     OnDefer();
+    set_controller(std::move(controller));
+  } else {
+    controller->Resume();
   }
-
-  return true;
 }
 
 void AsyncResourceHandler::OnDataDownloaded(int bytes_downloaded) {
@@ -479,10 +506,12 @@ void AsyncResourceHandler::OnDataDownloaded(int bytes_downloaded) {
 
 void AsyncResourceHandler::OnResponseCompleted(
     const net::URLRequestStatus& status,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   const ResourceRequestInfoImpl* info = GetRequestInfo();
-  if (!info->filter())
+  if (!info->filter()) {
+    controller->Resume();
     return;
+  }
 
   // If we crash here, figure out what URL the renderer was requesting.
   // http://crbug.com/107692
@@ -519,6 +548,7 @@ void AsyncResourceHandler::OnResponseCompleted(
 
   if (status.is_success())
     RecordHistogram();
+  controller->Resume();
 }
 
 bool AsyncResourceHandler::EnsureResourceBufferIsInitialized() {
@@ -537,7 +567,7 @@ void AsyncResourceHandler::ResumeIfDeferred() {
   if (did_defer_) {
     did_defer_ = false;
     request()->LogUnblocked();
-    controller()->Resume();
+    Resume();
   }
 }
 
@@ -553,7 +583,6 @@ bool AsyncResourceHandler::CheckForSufficientResource() {
   if (rdh_->HasSufficientResourcesForRequest(request()))
     return true;
 
-  controller()->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
   return false;
 }