Allow MojoAsyncResourceHandler::OnWillRead to complete asyncronously

content/browser/loader/mojo_async_resource_handler.cc

 // Copyright 2016 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/mojo_async_resource_handler.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
@@ skipping 11 matching lines @@
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #include "content/browser/loader/resource_request_info_impl.h"
 #include "content/browser/loader/resource_scheduler.h"
 #include "content/browser/loader/upload_progress_tracker.h"
 #include "content/common/resource_request_completion_status.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/common/resource_response.h"
 #include "mojo/public/c/system/data_pipe.h"
 #include "mojo/public/cpp/bindings/message.h"
-#include "net/base/io_buffer.h"
 #include "net/base/mime_sniffer.h"
+#include "net/base/net_errors.h"
 #include "net/url_request/redirect_info.h"
 
 namespace content {
 namespace {
 
 int g_allocation_size = MojoAsyncResourceHandler::kDefaultAllocationSize;
 
 // MimeTypeResourceHandler *implicitly* requires that the buffer size
 // returned from OnWillRead should be larger than certain size.
 // TODO(yhirano): Fix MimeTypeResourceHandler.
@@ skipping 182 matching lines @@
                             base::Unretained(this)));
   }
 
   controller->Resume();
 }
 
 void MojoAsyncResourceHandler::OnWillRead(
     scoped_refptr<net::IOBuffer>* buf,
     int* buf_size,
     std::unique_ptr<ResourceController> controller) {
+  DCHECK(!buffer_);
+  DCHECK_EQ(0u, buffer_offset_);
+
   if (!CheckForSufficientResource()) {
     controller->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
     return;
   }
 
+  bool first_call = false;
   if (!shared_writer_) {
+    first_call = true;
     MojoCreateDataPipeOptions options;
     options.struct_size = sizeof(MojoCreateDataPipeOptions);
     options.flags = MOJO_CREATE_DATA_PIPE_OPTIONS_FLAG_NONE;
     options.element_num_bytes = 1;
     options.capacity_num_bytes = g_allocation_size;
     mojo::DataPipe data_pipe(options);
 
     DCHECK(data_pipe.producer_handle.is_valid());
     DCHECK(data_pipe.consumer_handle.is_valid());
 
     response_body_consumer_handle_ = std::move(data_pipe.consumer_handle);
     shared_writer_ = new SharedWriter(std::move(data_pipe.producer_handle));
     handle_watcher_.Start(shared_writer_->writer(), MOJO_HANDLE_SIGNAL_WRITABLE,
                           base::Bind(&MojoAsyncResourceHandler::OnWritable,
                                      base::Unretained(this)));
+  }
 
-    bool defer = false;
-    scoped_refptr<net::IOBufferWithSize> buffer;
-    if (!AllocateWriterIOBuffer(&buffer, &defer)) {
+  bool defer = false;
+  if (!AllocateWriterIOBuffer(&buffer_, &defer)) {
+    controller->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
+    return;
+  }
+
+  if (defer) {
+    DCHECK(!buffer_);
+    parent_buffer_ = buf;
+    parent_buffer_size_ = buf_size;
+    HoldController(std::move(controller));
+    request()->LogBlockedBy("MojoAsyncResourceHandler");
+    did_defer_on_will_read_ = true;
+    return;
+  }
+
+  // The first call to OnWillRead must return a buffer of at least
+  // kMinAllocationSize. If the Mojo buffer is too small, need to allocate an
+  // intermediary buffer.
+  if (first_call && static_cast<size_t>(buffer_->size()) < kMinAllocationSize) {
+    // The allocated buffer is too small, so need to create an intermediary one.
+    if (EndWrite(0) != MOJO_RESULT_OK) {
       controller->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
       return;
     }
-    if (!defer) {
-      if (static_cast<size_t>(buffer->size()) >= kMinAllocationSize) {
-        *buf = buffer_ = buffer;
-        *buf_size = buffer_->size();
-        controller->Resume();
-        return;
-      }
-
-      // The allocated buffer is too small.
-      if (EndWrite(0) != MOJO_RESULT_OK) {
-        controller->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
-        return;
-      }
-    }
     DCHECK(!is_using_io_buffer_not_from_writer_);
     is_using_io_buffer_not_from_writer_ = true;
     buffer_ = new net::IOBufferWithSize(kMinAllocationSize);
   }
 
-  DCHECK_EQ(0u, buffer_offset_);
   *buf = buffer_;
   *buf_size = buffer_->size();
   controller->Resume();
 }
 
 void MojoAsyncResourceHandler::OnReadCompleted(
     int bytes_read,
     std::unique_ptr<ResourceController> controller) {
   DCHECK(!has_controller());
   DCHECK_GE(bytes_read, 0);
   DCHECK(buffer_);
 
-  if (!bytes_read) {
+  if (bytes_read == 0) {
+    // Note that |buffer_| is not cleared here, which will cause a DCHECK on
+    // subsequent OnWillRead calls.

[Randy Smith (Not in Mondays), 2017/03/17 20:06:06]

Confirming: It seems what this means is that you're overloading that DCHECK() to
also mean "No reads after the EOF signal"; is that accurate?  If so, just to be
extra pedantic, a note next to the DCHECK that it also means that, to aid in
hopefully never to happen debugging?

[mmenke, 2017/03/17 22:29:39]

It's mostly because a buffer_ = nullptr here didn't seem worth the effort here,
and then when I ran into DCHECK on existing tests doing weird things without it,
I decided I liked the new DCHECK condition.

(Note that in practice, we'll always delete synchronously after this, since we
don't delay destruction on normal reads - only WriteToFileResourceHandler defers
teardown after sending 0-byte-reads downstream, I believe, and when it's in use,
we won't be calling OnReadCompleted)

Anyhow, I've added a comment.

     controller->Resume();
     return;
   }
 
   const ResourceRequestInfoImpl* info = GetRequestInfo();
   if (info->ShouldReportRawHeaders()) {
     auto transfer_size_diff = CalculateRecentlyReceivedBytes();
     if (transfer_size_diff > 0)
       url_loader_client_->OnTransferSizeUpdated(transfer_size_diff);
   }
 
   if (response_body_consumer_handle_.is_valid()) {
     // Send the data pipe on the first OnReadCompleted call.
     url_loader_client_->OnStartLoadingResponseBody(
         std::move(response_body_consumer_handle_));
     response_body_consumer_handle_.reset();
   }
 
   if (is_using_io_buffer_not_from_writer_) {
-    // Couldn't allocate a buffer on the data pipe in OnWillRead.
+    // Couldn't allocate a large enough buffer on the data pipe in OnWillRead.
     DCHECK_EQ(0u, buffer_bytes_read_);
     buffer_bytes_read_ = bytes_read;
     bool defer = false;
     if (!CopyReadDataToDataPipe(&defer)) {
-      controller->Cancel();
+      controller->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
       return;
     }
     if (defer) {
       request()->LogBlockedBy("MojoAsyncResourceHandler");
       did_defer_on_writing_ = true;
       HoldController(std::move(controller));
       return;
     }
     controller->Resume();
     return;
   }
 
   if (EndWrite(bytes_read) != MOJO_RESULT_OK) {
     controller->Cancel();
     return;
   }
-  // Allocate a buffer for the next OnWillRead call here, because OnWillRead
-  // doesn't have |defer| parameter.
-  bool defer = false;
-  if (!AllocateWriterIOBuffer(&buffer_, &defer)) {
-    controller->Cancel();
-    return;
-  }
-  if (defer) {
-    request()->LogBlockedBy("MojoAsyncResourceHandler");
-    did_defer_on_writing_ = true;
-    HoldController(std::move(controller));
-    return;
-  }
 
+  buffer_ = nullptr;
   controller->Resume();
 }
 
 void MojoAsyncResourceHandler::OnDataDownloaded(int bytes_downloaded) {
   url_loader_client_->OnDataDownloaded(bytes_downloaded,
                                        CalculateRecentlyReceivedBytes());
 }
 
 void MojoAsyncResourceHandler::FollowRedirect() {
   if (!request()->status().is_success()) {
     DVLOG(1) << "FollowRedirect for invalid request";
     return;
   }
   if (!did_defer_on_redirect_) {
     DVLOG(1) << "Malformed FollowRedirect request";
     ReportBadMessage("Malformed FollowRedirect request");
     return;
   }
 
+  DCHECK(!did_defer_on_will_read_);
   DCHECK(!did_defer_on_writing_);
   did_defer_on_redirect_ = false;
   request()->LogUnblocked();
   Resume();
 }
 
 void MojoAsyncResourceHandler::SetPriority(net::RequestPriority priority,
                                            int32_t intra_priority_value) {
   ResourceDispatcherHostImpl::Get()->scheduler()->ReprioritizeRequest(
       request(), priority, intra_priority_value);
@@ skipping 66 matching lines @@
   request_complete_data.completion_time = base::TimeTicks::Now();
   request_complete_data.encoded_data_length =
       request()->GetTotalReceivedBytes();
   request_complete_data.encoded_body_length = request()->GetRawBodyBytes();
 
   url_loader_client_->OnComplete(request_complete_data);
   controller->Resume();
 }
 
 bool MojoAsyncResourceHandler::CopyReadDataToDataPipe(bool* defer) {
-  while (true) {
+  while (buffer_bytes_read_ > 0) {
     scoped_refptr<net::IOBufferWithSize> dest;
     if (!AllocateWriterIOBuffer(&dest, defer))
       return false;
     if (*defer)
       return true;
-    if (buffer_bytes_read_ == 0) {
-      // All bytes are copied. Save the buffer for the next OnWillRead call.
-      buffer_ = std::move(dest);
-      return true;
-    }
 
     size_t copied_size =
         std::min(buffer_bytes_read_, static_cast<size_t>(dest->size()));
     memcpy(dest->data(), buffer_->data() + buffer_offset_, copied_size);
     buffer_offset_ += copied_size;
     buffer_bytes_read_ -= copied_size;
     if (EndWrite(copied_size) != MOJO_RESULT_OK)
       return false;
+  }
 
-    if (buffer_bytes_read_ == 0) {
-      // All bytes are copied.
-      buffer_offset_ = 0;
-      is_using_io_buffer_not_from_writer_ = false;
-    }
-  }
+  // All bytes are copied.
+  buffer_ = nullptr;
+  buffer_offset_ = 0;
+  is_using_io_buffer_not_from_writer_ = false;
+  return true;
 }
 
 bool MojoAsyncResourceHandler::AllocateWriterIOBuffer(
     scoped_refptr<net::IOBufferWithSize>* buf,
     bool* defer) {
   void* data = nullptr;
   uint32_t available = 0;
   MojoResult result = BeginWrite(&data, &available);
   if (result == MOJO_RESULT_SHOULD_WAIT) {
     *defer = true;
     return true;
   }
   if (result != MOJO_RESULT_OK)
     return false;
+  DCHECK_GT(available, 0u);
   *buf = new WriterIOBuffer(shared_writer_, data, available);
   return true;
 }
 
 bool MojoAsyncResourceHandler::CheckForSufficientResource() {
   if (has_checked_for_sufficient_resources_)
     return true;
   has_checked_for_sufficient_resources_ = true;
 
   if (rdh_->HasSufficientResourcesForRequest(request()))
     return true;
 
   return false;
 }
 
 void MojoAsyncResourceHandler::OnWritable(MojoResult result) {
+  if (did_defer_on_will_read_) {
+    DCHECK(has_controller());
+    DCHECK(!did_defer_on_writing_);
+    DCHECK(!did_defer_on_redirect_);
+
+    did_defer_on_will_read_ = false;
+
+    scoped_refptr<net::IOBuffer>* parent_buffer = parent_buffer_;
+    parent_buffer_ = nullptr;
+    int* parent_buffer_size = parent_buffer_size_;
+    parent_buffer_size_ = nullptr;
+    OnWillRead(parent_buffer, parent_buffer_size, ReleaseController());
+    return;
+  }
+
   if (!did_defer_on_writing_)
     return;
   DCHECK(has_controller());
   DCHECK(!did_defer_on_redirect_);
   did_defer_on_writing_ = false;
 
-  if (is_using_io_buffer_not_from_writer_) {
-    // |buffer_| is set to a net::IOBufferWithSize. Write the buffer contents
-    // to the data pipe.
-    DCHECK_GT(buffer_bytes_read_, 0u);
-    if (!CopyReadDataToDataPipe(&did_defer_on_writing_)) {
-      CancelWithError(net::ERR_FAILED);
-      return;
-    }
-  } else {
-    // Allocate a buffer for the next OnWillRead call here.
-    if (!AllocateWriterIOBuffer(&buffer_, &did_defer_on_writing_)) {
-      CancelWithError(net::ERR_FAILED);
-      return;
-    }
+  DCHECK(is_using_io_buffer_not_from_writer_);
+  // |buffer_| is set to a net::IOBufferWithSize. Write the buffer contents
+  // to the data pipe.
+  DCHECK_GT(buffer_bytes_read_, 0u);
+  if (!CopyReadDataToDataPipe(&did_defer_on_writing_)) {
+    CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
+    return;
   }
 
   if (did_defer_on_writing_) {
     // Continue waiting.
     return;
   }
   request()->LogUnblocked();
   Resume();
 }
 
@@ skipping 41 matching lines @@
       base::Bind(&MojoAsyncResourceHandler::OnUploadProgressACK,
                  weak_factory_.GetWeakPtr()));
 }
 
 void MojoAsyncResourceHandler::OnUploadProgressACK() {
   if (upload_progress_tracker_)
     upload_progress_tracker_->OnAckReceived();
 }
 
 }  // namespace content