Refactor ResourceHandler API.

content/browser/loader/intercepting_resource_handler.cc

Index: content/browser/loader/intercepting_resource_handler.cc
diff --git a/content/browser/loader/intercepting_resource_handler.cc b/content/browser/loader/intercepting_resource_handler.cc
index 7c8f4a998641b711b9f5917198011a99269cad02..d49e8ef23ad07c1f84cde72e2df8056700d06e0d 100644
--- a/content/browser/loader/intercepting_resource_handler.cc
+++ b/content/browser/loader/intercepting_resource_handler.cc
@@ -5,55 +5,83 @@
 #include "content/browser/loader/intercepting_resource_handler.h"
 
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
+#include "content/browser/loader/null_resource_controller.h"
+#include "content/public/browser/resource_controller.h"
 #include "content/public/common/resource_response.h"
 #include "net/base/io_buffer.h"
 #include "net/url_request/url_request.h"
 
 namespace content {
 
+class InterceptingResourceHandler::Controller : public ResourceController {
+ public:
+  explicit Controller(InterceptingResourceHandler* mime_handler)
+      : intercepting_handler_(mime_handler) {}
+
+  void Resume() override {
+    DCHECK(!used_);
+    used_ = true;
+    intercepting_handler_->ResumeInternal();
+  }
+
+  void Cancel() override {
+    DCHECK(!used_);
+    used_ = true;
+    intercepting_handler_->Cancel();
+  }
+
+  void CancelAndIgnore() override {
+    DCHECK(!used_);
+    used_ = true;
+    intercepting_handler_->CancelAndIgnore();
+  }
+
+  void CancelWithError(int error_code) override {
+    DCHECK(!used_);
+    used_ = true;
+    intercepting_handler_->CancelWithError(error_code);
+  }
+
+ private:
+  InterceptingResourceHandler* intercepting_handler_;
+  bool used_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(Controller);
+};
+
 InterceptingResourceHandler::InterceptingResourceHandler(
     std::unique_ptr<ResourceHandler> next_handler,
     net::URLRequest* request)
     : LayeredResourceHandler(request, std::move(next_handler)) {
-  next_handler_->SetController(this);
 }
 
 InterceptingResourceHandler::~InterceptingResourceHandler() {}
 
-void InterceptingResourceHandler::SetController(
-    ResourceController* controller) {
-  if (state_ == State::PASS_THROUGH)
-    return LayeredResourceHandler::SetController(controller);
-  ResourceHandler::SetController(controller);
-}
-
-bool InterceptingResourceHandler::OnResponseStarted(ResourceResponse* response,
-                                                    bool* defer) {
+void InterceptingResourceHandler::OnResponseStarted(
+    ResourceResponse* response,
+    std::unique_ptr<ResourceController> controller) {
   // If there's no need to switch handlers, just start acting as a blind
   // pass-through ResourceHandler.
   if (!new_handler_) {
     state_ = State::PASS_THROUGH;
-    next_handler_->SetController(controller());
-    return next_handler_->OnResponseStarted(response, defer);
+    next_handler_->OnResponseStarted(response, std::move(controller));
+    return;
   }
 
   DCHECK_EQ(state_, State::STARTING);
-  // Otherwise, switch handlers. First, inform the original ResourceHandler
-  // that this will be handled entirely by the new ResourceHandler.
-  bool defer_ignored = false;
-  if (!next_handler_->OnResponseStarted(response, &defer_ignored))
-    return false;
-
-  // Although deferring OnResponseStarted is legal, the only downstream handler
-  // which does so is CrossSiteResourceHandler. Cross-site transitions should
-  // not trigger when switching handlers.
-  DCHECK(!defer_ignored);
 
   // TODO(yhirano): Retaining ownership from a raw pointer is bad.

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

A sense a disturbance in the force ... as if several kittens died when the CL
containing this line landed ....

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

We were already doing it in the MimeBlahResourceHandler, so it's more their
corpses were dug up and buried elsewhere.

   response_ = response;
+
+  // Otherwise, switch handlers. First, inform the original ResourceHandler
+  // that this will be handled entirely by the new ResourceHandler.
+  set_controller(std::move(controller));
   state_ = State::SENDING_PAYLOAD_TO_OLD_HANDLER;
-  return DoLoop(defer);
+  next_handler_->OnResponseStarted(response,
+                                   base::MakeUnique<Controller>(this));
+  return;
 }
 
 bool InterceptingResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
@@ -75,7 +103,11 @@ bool InterceptingResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
   return true;
 }
 
-bool InterceptingResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+void InterceptingResourceHandler::OnReadCompleted(
+    int bytes_read,
+    std::unique_ptr<ResourceController> controller) {
+  DCHECK(!has_controller());
+
   DCHECK_GE(bytes_read, 0);
   if (state_ == State::PASS_THROUGH) {
     if (first_read_buffer_double_) {
@@ -86,197 +118,174 @@ bool InterceptingResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
       first_read_buffer_ = nullptr;
       first_read_buffer_double_ = nullptr;
     }
-    return next_handler_->OnReadCompleted(bytes_read, defer);
+    next_handler_->OnReadCompleted(bytes_read, std::move(controller));
+    return;
   }
 
   DCHECK_EQ(State::WAITING_FOR_ON_READ_COMPLETED, state_);
   first_read_buffer_bytes_read_ = bytes_read;
   state_ = State::SENDING_BUFFER_TO_NEW_HANDLER;
-  return DoLoop(defer);
+  set_controller(std::move(controller));
+  DoLoop();
 }
 
 void InterceptingResourceHandler::OnResponseCompleted(
     const net::URLRequestStatus& status,
-    bool* defer) {
+    std::unique_ptr<ResourceController> controller) {
   if (state_ == State::PASS_THROUGH) {
-    LayeredResourceHandler::OnResponseCompleted(status, defer);
+    LayeredResourceHandler::OnResponseCompleted(status, std::move(controller));
     return;
   }
   if (!new_handler_) {
     // Therer is only one ResourceHandler in this InterceptingResourceHandler.
     state_ = State::PASS_THROUGH;
     first_read_buffer_double_ = nullptr;
-    next_handler_->SetController(controller());
-    next_handler_->OnResponseCompleted(status, defer);
+    next_handler_->OnResponseCompleted(status, std::move(controller));
     return;
   }
 
   // There are two ResourceHandlers in this InterceptingResourceHandler.
   // |next_handler_| is the old handler and |new_handler_| is the new handler.
   // As written in the class comment, this class assumes that the old handler
-  // will not set |*defer| in OnResponseCompleted.
-  next_handler_->SetController(controller());
-  next_handler_->OnResponseCompleted(status, defer);
-  DCHECK(!*defer);
+  // will immediately call Resume() in OnResponseCompleted.
+  bool was_resumed = false;
+  next_handler_->OnResponseCompleted(
+      status, base::MakeUnique<NullResourceController>(&was_resumed));
+  DCHECK(was_resumed);
 
   state_ = State::PASS_THROUGH;
   first_read_buffer_double_ = nullptr;
-  new_handler_->SetController(controller());
   next_handler_ = std::move(new_handler_);
-  next_handler_->OnResponseCompleted(status, defer);
+  next_handler_->OnResponseCompleted(status, std::move(controller));
 }
 
-void InterceptingResourceHandler::Cancel() {
-  DCHECK_NE(State::PASS_THROUGH, state_);
-  controller()->Cancel();
-}
-
-void InterceptingResourceHandler::CancelAndIgnore() {
-  DCHECK_NE(State::PASS_THROUGH, state_);
-  controller()->CancelAndIgnore();
-}
-
-void InterceptingResourceHandler::CancelWithError(int error_code) {
-  DCHECK_NE(State::PASS_THROUGH, state_);
-  controller()->CancelWithError(error_code);
-}
-
-void InterceptingResourceHandler::Resume() {
-  DCHECK_NE(State::PASS_THROUGH, state_);
+void InterceptingResourceHandler::ResumeInternal() {
+  DCHECK(has_controller());
   if (state_ == State::STARTING ||
-      state_ == State::WAITING_FOR_ON_READ_COMPLETED) {
+      state_ == State::WAITING_FOR_ON_READ_COMPLETED ||
+      state_ == State::PASS_THROUGH) {
     // Uninteresting Resume: just delegate to the original resource controller.
-    controller()->Resume();
-    return;
-  }
-  bool defer = false;
-  if (!DoLoop(&defer)) {
-    controller()->Cancel();
+    Resume();
     return;
   }
 
-  if (!defer)
-    controller()->Resume();
+  DoLoop();
 }
 
 void InterceptingResourceHandler::UseNewHandler(
     std::unique_ptr<ResourceHandler> new_handler,
     const std::string& payload_for_old_handler) {
   new_handler_ = std::move(new_handler);
-  new_handler_->SetController(this);
   payload_for_old_handler_ = payload_for_old_handler;
 }
 
-bool InterceptingResourceHandler::DoLoop(bool* defer) {
-  bool result = true;
-  do {
-    switch (state_) {
-      case State::STARTING:
-      case State::WAITING_FOR_ON_READ_COMPLETED:
-      case State::PASS_THROUGH:
-        NOTREACHED();
-        break;
-      case State::SENDING_ON_WILL_START_TO_NEW_HANDLER:
-        result = SendOnResponseStartedToNewHandler(defer);
-        break;
-      case State::SENDING_ON_RESPONSE_STARTED_TO_NEW_HANDLER:
-        if (first_read_buffer_double_) {
-          // OnWillRead has been called, so copying the data from
-          // |first_read_buffer_double_| to |first_read_buffer_| will be needed
-          // when OnReadCompleted is called.
-          state_ = State::WAITING_FOR_ON_READ_COMPLETED;
-        } else {
-          // OnWillRead has not been called, so no special handling will be
-          // needed from now on.
-          state_ = State::PASS_THROUGH;
-          next_handler_->SetController(controller());
-        }
-        break;
-      case State::SENDING_PAYLOAD_TO_OLD_HANDLER:
-        result = SendPayloadToOldHandler(defer);
-        break;
-      case State::SENDING_BUFFER_TO_NEW_HANDLER:
-        result = SendFirstReadBufferToNewHandler(defer);
-        break;
-    }
-  } while (result && !*defer &&
-           state_ != State::WAITING_FOR_ON_READ_COMPLETED &&
-           state_ != State::PASS_THROUGH);
-  return result;
+void InterceptingResourceHandler::DoLoop() {
+  switch (state_) {
+    case State::STARTING:
+    case State::WAITING_FOR_ON_READ_COMPLETED:
+    case State::PASS_THROUGH:
+      NOTREACHED();
+      break;
+    case State::SENDING_ON_WILL_START_TO_NEW_HANDLER:
+      SendOnResponseStartedToNewHandler();
+      break;
+    case State::SENDING_ON_RESPONSE_STARTED_TO_NEW_HANDLER:
+      if (first_read_buffer_double_) {
+        // OnWillRead has been called, so copying the data from
+        // |first_read_buffer_double_| to |first_read_buffer_| will be needed
+        // when OnReadCompleted is called.
+        state_ = State::WAITING_FOR_ON_READ_COMPLETED;
+        ResumeInternal();
+      } else {
+        // OnWillRead has not been called, so no special handling will be
+        // needed from now on.
+        state_ = State::PASS_THROUGH;
+        ResumeInternal();
+      }
+      break;
+    case State::SENDING_PAYLOAD_TO_OLD_HANDLER:
+      SendPayloadToOldHandler();
+      break;
+    case State::SENDING_BUFFER_TO_NEW_HANDLER:
+      SendFirstReadBufferToNewHandler();
+      break;
+  }
 }
 
-bool InterceptingResourceHandler::SendPayloadToOldHandler(bool* defer) {
+void InterceptingResourceHandler::SendPayloadToOldHandler() {
   DCHECK_EQ(State::SENDING_PAYLOAD_TO_OLD_HANDLER, state_);
-  while (payload_bytes_written_ < payload_for_old_handler_.size()) {
-    scoped_refptr<net::IOBuffer> buffer;
-    int size = 0;
-    if (first_read_buffer_) {
-      // |first_read_buffer_| is a buffer gotten from |next_handler_| via
-      // OnWillRead. Use the buffer.
-      buffer = first_read_buffer_;
-      size = first_read_buffer_size_;
-
-      first_read_buffer_ = nullptr;
-      first_read_buffer_size_ = 0;
-    } else {
-      if (!next_handler_->OnWillRead(&buffer, &size, -1))
-        return false;
+  if (payload_bytes_written_ == payload_for_old_handler_.size()) {
+    net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0);
+    if (payload_for_old_handler_.empty()) {
+      // If there is no payload, just finalize the request on the old handler.
+      status = net::URLRequestStatus::FromError(net::ERR_ABORTED);
     }
-
-    size = std::min(size, static_cast<int>(payload_for_old_handler_.size() -
-                                           payload_bytes_written_));
-    memcpy(buffer->data(),
-           payload_for_old_handler_.data() + payload_bytes_written_, size);
-    if (!next_handler_->OnReadCompleted(size, defer))
-      return false;
-    payload_bytes_written_ += size;
-    if (*defer)
-      return true;
+    bool was_resumed = false;
+    next_handler_->OnResponseCompleted(
+        status, base::MakeUnique<NullResourceController>(&was_resumed));
+    DCHECK(was_resumed);
+
+    next_handler_ = std::move(new_handler_);
+    state_ = State::SENDING_ON_WILL_START_TO_NEW_HANDLER;
+    next_handler_->OnWillStart(request()->url(),
+                               base::MakeUnique<Controller>(this));
+    return;
   }
 
-  net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0);
-  if (payload_for_old_handler_.empty()) {
-    // If there is no payload, just finalize the request on the old handler.
-    status = net::URLRequestStatus::FromError(net::ERR_ABORTED);
+  scoped_refptr<net::IOBuffer> buffer;
+  int size = 0;
+  if (first_read_buffer_) {
+    // |first_read_buffer_| is a buffer gotten from |next_handler_| via
+    // OnWillRead. Use the buffer.
+    buffer = first_read_buffer_;
+    size = first_read_buffer_size_;
+
+    first_read_buffer_ = nullptr;
+    first_read_buffer_size_ = 0;
+  } else {
+    if (!next_handler_->OnWillRead(&buffer, &size, -1)) {
+      Cancel();
+      return;
+    }
   }
-  next_handler_->OnResponseCompleted(status, defer);
-  DCHECK(!*defer);
 
-  next_handler_ = std::move(new_handler_);
-  state_ = State::SENDING_ON_WILL_START_TO_NEW_HANDLER;
-  return next_handler_->OnWillStart(request()->url(), defer);
+  size = std::min(size, static_cast<int>(payload_for_old_handler_.size() -
+                                         payload_bytes_written_));
+  memcpy(buffer->data(),
+         payload_for_old_handler_.data() + payload_bytes_written_, size);
+  payload_bytes_written_ += size;
+  next_handler_->OnReadCompleted(size, base::MakeUnique<Controller>(this));
 }
 
-bool InterceptingResourceHandler::SendOnResponseStartedToNewHandler(
-    bool* defer) {
+void InterceptingResourceHandler::SendOnResponseStartedToNewHandler() {
   state_ = State::SENDING_ON_RESPONSE_STARTED_TO_NEW_HANDLER;
-  return next_handler_->OnResponseStarted(response_.get(), defer);
+  next_handler_->OnResponseStarted(response_.get(),
+                                   base::MakeUnique<Controller>(this));
 }
 
-bool InterceptingResourceHandler::SendFirstReadBufferToNewHandler(bool* defer) {
+void InterceptingResourceHandler::SendFirstReadBufferToNewHandler() {
   DCHECK_EQ(state_, State::SENDING_BUFFER_TO_NEW_HANDLER);
 
-  while (first_read_buffer_bytes_written_ < first_read_buffer_bytes_read_) {
-    scoped_refptr<net::IOBuffer> buf;
-    int size = 0;
-    if (!next_handler_->OnWillRead(&buf, &size, -1))
-      return false;
-    size = std::min(size, static_cast<int>(first_read_buffer_bytes_read_ -
-                                           first_read_buffer_bytes_written_));
-    memcpy(buf->data(),
-           first_read_buffer_double_->data() + first_read_buffer_bytes_written_,
-           size);
-    if (!next_handler_->OnReadCompleted(size, defer))
-      return false;
-    first_read_buffer_bytes_written_ += size;
-    if (*defer)
-      return true;
+  if (first_read_buffer_bytes_written_ == first_read_buffer_bytes_read_) {
+    state_ = State::PASS_THROUGH;
+    first_read_buffer_double_ = nullptr;
+    ResumeInternal();
+    return;
   }
 
-  state_ = State::PASS_THROUGH;
-  first_read_buffer_double_ = nullptr;
-  next_handler_->SetController(controller());
-  return true;
+  scoped_refptr<net::IOBuffer> buf;
+  int size = 0;
+  if (!next_handler_->OnWillRead(&buf, &size, -1)) {
+    Cancel();
+    return;
+  }
+  size = std::min(size, static_cast<int>(first_read_buffer_bytes_read_ -
+                                         first_read_buffer_bytes_written_));
+  memcpy(buf->data(),
+         first_read_buffer_double_->data() + first_read_buffer_bytes_written_,
+         size);
+  first_read_buffer_bytes_written_ += size;
+  next_handler_->OnReadCompleted(size, base::MakeUnique<Controller>(this));
 }
 
 }  // namespace content