Move MimeTypeResourceHandler before ThrottlingResourceHandler

content/browser/loader/intercepting_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/intercepting_resource_handler.h"
+
+#include <utility>
+#include <vector>

[mmenke, 2016/07/18 18:50:42]

Not used

[clamy, 2016/07/19 14:24:54]

Done.

+
+#include "base/logging.h"
+#include "base/strings/string_util.h"
+#include "content/public/common/resource_response.h"
+#include "net/base/io_buffer.h"
+
+namespace content {
+
+InterceptingResourceHandler::InterceptingResourceHandler(
+    std::unique_ptr<ResourceHandler> next_handler,
+    net::URLRequest* request)
+    : LayeredResourceHandler(request, std::move(next_handler)),
+      state_(State::STARTING),
+      bytes_read_(0) {}
+
+InterceptingResourceHandler::~InterceptingResourceHandler() {}
+
+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(controller());
+  payload_for_old_handler_ = payload_for_old_handler;
+}
+
+bool InterceptingResourceHandler::OnResponseStarted(ResourceResponse* response,
+                                                    bool* defer) {
+  // If there's no need to switch handlers, just start acting as a blind
+  // pass-through ResourceHandler.
+  if (!new_handler_) {
+    state_ = State::DONE;
+    return next_handler_->OnResponseStarted(response, defer);
+  }
+
+  // Otherwise, switch handlers. First, inform the original ResourceHandler
+  // that this will be handled entirely by the new ResourceHandler.
+  // TODO(clamy): We will probably need to check the return values of these for
+  // PlzNavigate.
+  bool defer_ignored = false;
+  next_handler_->OnResponseStarted(response, &defer_ignored);
+  // 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);
+
+  // Send the payload to the old handler.
+  SendPayloadToOldHandler();
+
+  // This is now handled entirely within the new ResourceHandler, so just reset
+  // the original ResourceHandler.
+  next_handler_ = std::move(new_handler_);
+
+  state_ = read_buffer_ ? State::WAITING_FOR_BUFFER_COPY : State::DONE;
+
+  return next_handler_->OnResponseStarted(response, defer);
+}
+
+bool InterceptingResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
+                                             int* buf_size,
+                                             int min_size) {
+  if (state_ == State::DONE)
+    return next_handler_->OnWillRead(buf, buf_size, min_size);
+
+  DCHECK_EQ(State::STARTING, state_);
+  DCHECK_EQ(-1, min_size);
+
+  if (!next_handler_->OnWillRead(buf, buf_size, min_size))
+    return false;
+
+  read_buffer_ = *buf;
+  return true;
+}
+
+bool InterceptingResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+  if (state_ == State::DONE)
+    return next_handler_->OnReadCompleted(bytes_read, defer);
+
+  DCHECK_EQ(State::WAITING_FOR_BUFFER_COPY, state_);
+  bytes_read_ += bytes_read;

[mmenke, 2016/07/18 18:50:42]

Seems like bytes_read_ isn't needed, given that this method is only called once.
 Can just pass it directly to CopyReadBufferToNextHandler, right?

[clamy, 2016/07/19 14:24:54]

Done.

+  if (!CopyReadBufferToNextHandler())
+    return false;
+
+  return next_handler_->OnReadCompleted(bytes_read, defer);
+}
+
+void InterceptingResourceHandler::SendPayloadToOldHandler() {
+  bool defer_ignored = false;
+  if (payload_for_old_handler_.empty()) {
+    // If there is no payload, just finalize the request on the old handler.
+    net::URLRequestStatus status(net::URLRequestStatus::CANCELED,
+                                 net::ERR_ABORTED);
+    next_handler_->OnResponseCompleted(status, std::string(), &defer_ignored);
+    DCHECK(!defer_ignored);
+    return;
+  }
+
+  // Complete the read operation that may have created the IOBuffer.  Note that
+  // this InterceptingResourceHandler keeps a pointer to the IOBuffer, so it
+  // won't actually be reset, since the data inside it needs to be sent to the
+  // new next handler.
+  if (read_buffer_) {
+    next_handler_->OnReadCompleted(0, &defer_ignored);
+    DCHECK(!defer_ignored);

[mmenke, 2016/07/18 18:50:42]

I don't understand this - why are we completing the read with 0 bytes, only to
create a new buffer, when we presumably could have just use read_buffer_?

[clamy, 2016/07/19 14:24:54]

Because we may already have read something in read_buffer_ that we want to pass
to the new handler, so we don't want to override it with the payload for the old
handler. Rewrote the comment to make it clearer.

+  }
+
+  // Get a new buffer iand use it to send |payload_for_old_handler_| to the

[mmenke, 2016/07/18 18:50:42]

nit:  iand -> and

[clamy, 2016/07/19 14:24:54]

Done.

+  // next ResourceHandler.

[mmenke, 2016/07/18 18:50:42]

"next ResourceHandler" is extremely confusing - this is the original
ResourceHandler.

[clamy, 2016/07/19 14:24:54]

Done.

+  scoped_refptr<net::IOBuffer> buf;
+  int size = 0;
+  next_handler_->OnWillRead(&buf, &size, -1);
+  CHECK_GE(size, static_cast<int>(payload_for_old_handler_.length()));
+  memcpy(buf->data(), payload_for_old_handler_.c_str(),
+         payload_for_old_handler_.length());
+  next_handler_->OnReadCompleted(payload_for_old_handler_.length(),
+                                 &defer_ignored);
+  DCHECK(!defer_ignored);
+
+  // Finalize the request.
+  net::URLRequestStatus status(net::URLRequestStatus::SUCCESS, 0);
+  next_handler_->OnResponseCompleted(status, std::string(), &defer_ignored);
+  DCHECK(!defer_ignored);
+}
+
+bool InterceptingResourceHandler::CopyReadBufferToNextHandler() {

[mmenke, 2016/07/18 18:50:42]

Suggest just inlining this.  The method it's called from is short enough, and
the "state_ = State::DONE" transition seems a bit unexpected in here.  THink
it's clearer just to inline.

[clamy, 2016/07/19 14:24:54]

Done.

+  DCHECK(read_buffer_.get());
+  scoped_refptr<net::IOBuffer> buf;
+  int buf_len = 0;
+  if (!next_handler_->OnWillRead(&buf, &buf_len, bytes_read_))
+    return false;
+
+  CHECK((buf_len >= bytes_read_) && (bytes_read_ >= 0));
+  memcpy(buf->data(), read_buffer_->data(), bytes_read_);
+  state_ = State::DONE;

[mmenke, 2016/07/18 18:50:42]

Suppose it doesn't really matter what we do if OnWillRead fails, but seems most
consistent, and least likely to cause big colorful explosions, if we move this
line above the return false.

[clamy, 2016/07/19 14:24:54]

Done.

+  read_buffer_ = nullptr;
+  return true;
+}
+
+}  // namespace content