Refactor ResourceHandler API.

content/browser/loader/redirect_to_file_resource_handler.cc

Index: content/browser/loader/redirect_to_file_resource_handler.cc
diff --git a/content/browser/loader/redirect_to_file_resource_handler.cc b/content/browser/loader/redirect_to_file_resource_handler.cc
index a30d9dc68e2a3e4363d634b9cd6eff428d7061dd..b80288ff7aac0cf2e8657f4dd13631512ea2092e 100644
--- a/content/browser/loader/redirect_to_file_resource_handler.cc
+++ b/content/browser/loader/redirect_to_file_resource_handler.cc
@@ -138,7 +138,6 @@ RedirectToFileResourceHandler::RedirectToFileResourceHandler(
       write_cursor_(0),
       writer_(NULL),
       next_buffer_size_(kInitialReadBufSize),
-      did_defer_(false),
       completed_during_write_(false),
       weak_factory_(this) {}
 
@@ -156,22 +155,24 @@ void RedirectToFileResourceHandler::
   create_temporary_file_stream_ = create_temporary_file_stream;
 }
 
-bool RedirectToFileResourceHandler::OnResponseStarted(
+void RedirectToFileResourceHandler::OnResponseStarted(
     ResourceResponse* response,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   DCHECK(writer_);
   response->head.download_file_path = writer_->path();
-  return next_handler_->OnResponseStarted(response, defer);
+  next_handler_->OnResponseStarted(response, std::move(controller));
 }
 
-bool RedirectToFileResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+void RedirectToFileResourceHandler::OnWillStart(
+    const GURL& url,
+    std::unique_ptr<ResourceController> controller) {
   DCHECK(!writer_);
 
   // Defer starting the request until we have created the temporary file.
   // TODO(darin): This is sub-optimal.  We should not delay starting the
   // network request like this.
   will_start_url_ = url;
-  did_defer_ = *defer = true;
+  HoldController(std::move(controller));
   request()->LogBlockedBy("RedirectToFileResourceHandler");
   if (create_temporary_file_stream_.is_null()) {
     CreateTemporaryFileStream(
@@ -182,7 +183,6 @@ bool RedirectToFileResourceHandler::OnWillStart(const GURL& url, bool* defer) {
         base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile,
                    weak_factory_.GetWeakPtr()));
   }
-  return true;
 }
 
 bool RedirectToFileResourceHandler::OnWillRead(
@@ -204,8 +204,9 @@ bool RedirectToFileResourceHandler::OnWillRead(
   return true;
 }
 
-bool RedirectToFileResourceHandler::OnReadCompleted(int bytes_read,
-                                                    bool* defer) {
+void RedirectToFileResourceHandler::OnReadCompleted(
+    int bytes_read,
+    std::unique_ptr<ResourceController> controller) {
   DCHECK(buf_write_pending_);
   buf_write_pending_ = false;
 
@@ -214,32 +215,32 @@ bool RedirectToFileResourceHandler::OnReadCompleted(int bytes_read,
   DCHECK(new_offset <= buf_->capacity());
   buf_->set_offset(new_offset);
 
-  if (BufIsFull()) {
-    did_defer_ = *defer = true;
-    request()->LogBlockedBy("RedirectToFileResourceHandler");
+  HoldController(std::move(controller));
+  request()->LogBlockedBy("RedirectToFileResourceHandler");
 
-    if (buf_->capacity() == bytes_read) {
-      // The network layer has saturated our buffer in one read. Next time, we
-      // should give it a bigger buffer for it to fill.
-      next_buffer_size_ = std::min(next_buffer_size_ * 2, kMaxReadBufSize);
-    }
+  if (BufIsFull() && buf_->capacity() == bytes_read) {
+    // The network layer has saturated our buffer in one read. Next time, we
+    // should give it a bigger buffer for it to fill.
+    next_buffer_size_ = std::min(next_buffer_size_ * 2, kMaxReadBufSize);
   }
 
-  return WriteMore();
+  if (!WriteMore()) {

[Charlie Harrison, 2017/01/25 20:23:00]

Can you comment that WriteMore will resume the request if there is more data
/buffer space?

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

Done.

+    Cancel();
+    return;
+  }
 }
 
 void RedirectToFileResourceHandler::OnResponseCompleted(
     const net::URLRequestStatus& status,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   if (writer_ && writer_->is_writing()) {
     completed_during_write_ = true;
     completed_status_ = status;
-    did_defer_ = true;
-    *defer = true;
+    HoldController(std::move(controller));
     request()->LogBlockedBy("RedirectToFileResourceHandler");
     return;
   }
-  next_handler_->OnResponseCompleted(status, defer);
+  next_handler_->OnResponseCompleted(status, std::move(controller));
 }
 
 void RedirectToFileResourceHandler::DidCreateTemporaryFile(
@@ -247,25 +248,17 @@ void RedirectToFileResourceHandler::DidCreateTemporaryFile(
     std::unique_ptr<net::FileStream> file_stream,
     ShareableFileReference* deletable_file) {
   DCHECK(!writer_);
+  DCHECK(has_controller());
   if (error_code != base::File::FILE_OK) {
-    controller()->CancelWithError(net::FileErrorToNetError(error_code));
+    CancelWithError(net::FileErrorToNetError(error_code));
     return;
   }
 
   writer_ = new Writer(this, std::move(file_stream), deletable_file);
 
   // Resume the request.
-  DCHECK(did_defer_);
-  bool defer = false;
   request()->LogUnblocked();
-  if (!next_handler_->OnWillStart(will_start_url_, &defer)) {
-    controller()->Cancel();
-  } else if (!defer) {
-    Resume();
-  } else {
-    did_defer_ = false;
-  }
-  will_start_url_ = GURL();
+  next_handler_->OnWillStart(std::move(will_start_url_), ReleaseController());
 }
 
 void RedirectToFileResourceHandler::DidWriteToFile(int result) {
@@ -288,22 +281,25 @@ void RedirectToFileResourceHandler::DidWriteToFile(int result) {
       completed_status_ = net::URLRequestStatus(net::URLRequestStatus::CANCELED,
                                                 net::ERR_FAILED);
     }
-    if (!completed_during_write_)
-      controller()->CancelWithError(net::ERR_FAILED);
+    if (!completed_during_write_) {
+      if (has_controller()) {
+        // If the write buffer is full, |this| has deferred the request, and
+        // can do an in-band cancel.
+        CancelWithError(net::ERR_FAILED);
+      } else {
+        OutOfBandCancel(net::ERR_FAILED, true);
+      }
+      return;
+    }
   }
 
   if (completed_during_write_ && !writer_->is_writing()) {
     // Resume shutdown now that all data has been written to disk. Note that
     // this should run even in the |failed| case above, otherwise a failed write
     // leaves the handler stuck.
-    bool defer = false;
+    DCHECK(has_controller());
     request()->LogUnblocked();
-    next_handler_->OnResponseCompleted(completed_status_, &defer);
-    if (!defer) {
-      Resume();
-    } else {
-      did_defer_ = false;
-    }
+    next_handler_->OnResponseCompleted(completed_status_, ReleaseController());
   }
 }
 
@@ -314,17 +310,13 @@ bool RedirectToFileResourceHandler::WriteMore() {
       // We've caught up to the network load, but it may be in the process of
       // appending more data to the buffer.
       if (!buf_write_pending_) {
-        if (BufIsFull()) {
-          request()->LogUnblocked();
-          Resume();
-        }
         buf_->set_offset(0);
         write_cursor_ = 0;
       }
-      return true;
+      break;
     }
     if (writer_->is_writing())
-      return true;
+      break;
     DCHECK(write_cursor_ < buf_->offset());
 
     // Create a temporary buffer pointing to a subsection of the data buffer so
@@ -347,12 +339,18 @@ bool RedirectToFileResourceHandler::WriteMore() {
 
     int rv = writer_->Write(wrapped.get(), write_len);
     if (rv == net::ERR_IO_PENDING)
-      return true;
+      break;
     if (rv <= 0)
       return false;
     next_handler_->OnDataDownloaded(rv);
     write_cursor_ += rv;
   }
+
+  if (has_controller() && !BufIsFull() && !completed_during_write_) {
+    request()->LogUnblocked();
+    Resume();
+  }
+  return true;
 }
 
 bool RedirectToFileResourceHandler::BufIsFull() const {
@@ -363,10 +361,4 @@ bool RedirectToFileResourceHandler::BufIsFull() const {
   return buf_->RemainingCapacity() <= (2 * net::kMaxBytesToSniff);
 }
 
-void RedirectToFileResourceHandler::Resume() {
-  DCHECK(did_defer_);
-  did_defer_ = false;
-  controller()->Resume();
-}
-
 }  // namespace content