Make ResourceHandler::OnWillRead able to complete asynchronously.

content/browser/loader/mime_sniffing_resource_handler.cc

 // Copyright (c) 2012 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/mime_sniffing_resource_handler.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 107 matching lines @@
     : LayeredResourceHandler(request, std::move(next_handler)),
       state_(STATE_STARTING),
       host_(host),
 #if BUILDFLAG(ENABLE_PLUGINS)
       plugin_service_(plugin_service),
 #endif
       must_download_(false),
       must_download_is_set_(false),
       read_buffer_size_(0),
       bytes_read_(0),
+      parent_read_buffer_(nullptr),
+      parent_read_buffer_size_(nullptr),
       intercepting_handler_(intercepting_handler),
       request_context_type_(request_context_type),
       in_state_loop_(false),
       advance_state_(false),
       weak_ptr_factory_(this) {
 }
 
 MimeSniffingResourceHandler::~MimeSniffingResourceHandler() {}
 
 void MimeSniffingResourceHandler::OnWillStart(
@@ skipping 70 matching lines @@
     if (response_->head.mime_type == "application/rss+xml" ||
         response_->head.mime_type == "application/atom+xml") {
       response_->head.mime_type.assign("text/plain");
     }
   }
 
   HoldController(std::move(controller));
   AdvanceState();
 }
 
-bool MimeSniffingResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
-                                             int* buf_size) {
-  if (state_ == STATE_STREAMING)
-    return next_handler_->OnWillRead(buf, buf_size);
+void MimeSniffingResourceHandler::OnWillRead(
+    scoped_refptr<net::IOBuffer>* buf,
+    int* buf_size,
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(buf);
+  DCHECK(buf_size);
+  DCHECK(!parent_read_buffer_);
+  DCHECK(!parent_read_buffer_size_);
+
+  if (state_ == STATE_STREAMING) {
+    next_handler_->OnWillRead(buf, buf_size, std::move(controller));
+    return;
+  }
+
+  DCHECK_EQ(State::STATE_BUFFERING, state_);
 
   if (read_buffer_.get()) {
     CHECK_LT(bytes_read_, read_buffer_size_);
     *buf = new DependentIOBuffer(read_buffer_.get(), bytes_read_);
     *buf_size = read_buffer_size_ - bytes_read_;
-  } else {
-    if (!next_handler_->OnWillRead(buf, buf_size))
-      return false;
+    controller->Resume();
+    return;
+  }
 
-    read_buffer_ = *buf;
-    read_buffer_size_ = *buf_size;
-    DCHECK_GE(read_buffer_size_, net::kMaxBytesToSniff * 2);
-  }
-  return true;
+  DCHECK(!read_buffer_size_);
+
+  parent_read_buffer_ = buf;
+  parent_read_buffer_size_ = buf_size;
+
+  HoldController(std::move(controller));
+
+  // Have to go through AdvanceState here so that if OnWillRead completes
+  // synchronously, won't post a task.
+  state_ = State::STATE_CALLING_ON_WILL_READ;
+  AdvanceState();
 }
 
 void MimeSniffingResourceHandler::OnReadCompleted(
     int bytes_read,
     std::unique_ptr<ResourceController> controller) {
   DCHECK(!has_controller());
 
   if (state_ == STATE_STREAMING) {
     next_handler_->OnReadCompleted(bytes_read, std::move(controller));
     return;
@@ skipping 63 matching lines @@
 
   base::AutoReset<bool> auto_in_state_loop(&in_state_loop_, true);
   advance_state_ = true;
   while (advance_state_) {
     advance_state_ = false;
 
     switch (state_) {
       case STATE_BUFFERING:
         MaybeIntercept();
         break;
+      case STATE_CALLING_ON_WILL_READ:
+        CallOnWillRead();
+        break;
+      case STATE_WAITING_FOR_BUFFER:
+        BufferReceived();
+        break;
       case STATE_INTERCEPTION_CHECK_DONE:
         ReplayResponseReceived();
         break;
       case STATE_REPLAYING_RESPONSE_RECEIVED:
         ReplayReadCompleted();
         break;
       case STATE_STARTING:
       case STATE_STREAMING:
-        in_state_loop_ = false;
         Resume();
         return;
       default:
         NOTREACHED();
         break;
     }
   }
 
   DCHECK(in_state_loop_);
   in_state_loop_ = false;
 }
 
 void MimeSniffingResourceHandler::MaybeIntercept() {
   DCHECK_EQ(STATE_BUFFERING, state_);
   // If a request that can be intercepted failed the check for interception
   // step, it should be canceled.
   if (!MaybeStartInterception())
     return;
 
   state_ = STATE_INTERCEPTION_CHECK_DONE;
   ResumeInternal();
 }
 
+void MimeSniffingResourceHandler::CallOnWillRead() {
+  DCHECK_EQ(STATE_CALLING_ON_WILL_READ, state_);
+
+  state_ = STATE_WAITING_FOR_BUFFER;
+  next_handler_->OnWillRead(&read_buffer_, &read_buffer_size_,
+                            base::MakeUnique<Controller>(this));
+}
+
+void MimeSniffingResourceHandler::BufferReceived() {
+  DCHECK_EQ(STATE_WAITING_FOR_BUFFER, state_);
+
+  DCHECK(read_buffer_);
+  DCHECK(parent_read_buffer_);
+  DCHECK(parent_read_buffer_size_);
+  DCHECK_GE(read_buffer_size_, net::kMaxBytesToSniff * 2);
+
+  *parent_read_buffer_ = read_buffer_;

[Charlie Harrison, 2017/02/16 21:25:04]

I think you should std::move() here to condense this operation and the = nullptr
operation, without causing ref churn (here and in other files).

[mmenke, 2017/03/08 19:16:07]

That wouldn't be correct - we need the buffer pointer, so we can consume any
data we receive.  The nullptr lines are not nulling out the scoped_refptrs, but
the pointers to the parent's scoped_refptrs.

[Charlie Harrison, 2017/03/08 21:12:46]

You're completely right, apologies.

+  *parent_read_buffer_size_ = read_buffer_size_;
+
+  parent_read_buffer_ = nullptr;
+  parent_read_buffer_size_ = nullptr;
+
+  state_ = State::STATE_BUFFERING;
+  Resume();
+}
+
 void MimeSniffingResourceHandler::ReplayResponseReceived() {
   DCHECK_EQ(STATE_INTERCEPTION_CHECK_DONE, state_);
   state_ = STATE_REPLAYING_RESPONSE_RECEIVED;
   next_handler_->OnResponseStarted(response_.get(),
                                    base::MakeUnique<Controller>(this));
 }
 
 void MimeSniffingResourceHandler::ReplayReadCompleted() {
   DCHECK_EQ(STATE_REPLAYING_RESPONSE_RECEIVED, state_);
 
@@ skipping 194 matching lines @@
 
 void MimeSniffingResourceHandler::OnPluginsLoaded(
     const std::vector<WebPluginInfo>& plugins) {
   // No longer blocking on the plugins being loaded.
   request()->LogUnblocked();
   if (state_ == STATE_BUFFERING)
     AdvanceState();
 }
 
 }  // namespace content