Move MimeTypeResourceHandler before ThrottlingResourceHandler

content/browser/loader/intercepting_resource_handler.cc

Index: content/browser/loader/intercepting_resource_handler.cc
diff --git a/content/browser/loader/mime_type_resource_handler.cc b/content/browser/loader/intercepting_resource_handler.cc
similarity index 48%
copy from content/browser/loader/mime_type_resource_handler.cc
copy to content/browser/loader/intercepting_resource_handler.cc
index ab4fca3a8dbdc4780fb66e01878e11c1a5aa31eb..05e3c378d53e1e7cbb224eb64225584a14ac72d4 100644
--- a/content/browser/loader/mime_type_resource_handler.cc
+++ b/content/browser/loader/intercepting_resource_handler.cc
@@ -1,8 +1,8 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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/mime_type_resource_handler.h"
+#include "content/browser/loader/intercepting_resource_handler.h"
 
 #include <utility>
 #include <vector>
@@ -30,294 +30,90 @@
 #include "content/public/common/resource_response.h"
 #include "content/public/common/webplugininfo.h"
 #include "net/base/io_buffer.h"
-#include "net/base/mime_sniffer.h"
 #include "net/base/mime_util.h"
-#include "net/base/net_errors.h"
 #include "net/http/http_content_disposition.h"
 #include "net/http/http_response_headers.h"
-#include "net/url_request/url_request.h"
 
 namespace content {
 
-namespace {
-
-const char kAcceptHeader[] = "Accept";
-const char kFrameAcceptHeader[] =
-    "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,"
-    "*/*;q=0.8";
-const char kStylesheetAcceptHeader[] = "text/css,*/*;q=0.1";
-const char kImageAcceptHeader[] = "image/webp,image/*,*/*;q=0.8";
-const char kDefaultAcceptHeader[] = "*/*";
-
-// Used to write into an existing IOBuffer at a given offset.
-class DependentIOBuffer : public net::WrappedIOBuffer {
- public:
-  DependentIOBuffer(net::IOBuffer* buf, int offset)
-      : net::WrappedIOBuffer(buf->data() + offset),
-        buf_(buf) {
-  }
-
- private:
-  ~DependentIOBuffer() override {}
-
-  scoped_refptr<net::IOBuffer> buf_;
-};
-
-}  // namespace
-
-MimeTypeResourceHandler::MimeTypeResourceHandler(
+InterceptingResourceHandler::InterceptingResourceHandler(
     std::unique_ptr<ResourceHandler> next_handler,
     ResourceDispatcherHostImpl* host,
     PluginService* plugin_service,
     net::URLRequest* request)
     : LayeredResourceHandler(request, std::move(next_handler)),
-      state_(STATE_STARTING),
+      state_(State::STARTING),
       host_(host),
 #if defined(ENABLE_PLUGINS)
       plugin_service_(plugin_service),
 #endif
-      read_buffer_size_(0),
-      bytes_read_(0),
+      switched_handler_(false),
       must_download_(false),
       must_download_is_set_(false),
+      bytes_read_(0),
       weak_ptr_factory_(this) {
 }
 
-MimeTypeResourceHandler::~MimeTypeResourceHandler() {
-}
-
-void MimeTypeResourceHandler::SetController(ResourceController* controller) {
-  ResourceHandler::SetController(controller);
-
-  // Downstream handlers see us as their ResourceController, which allows us to
-  // consume part or all of the resource response, and then later replay it to
-  // downstream handler.
-  DCHECK(next_handler_.get());
-  next_handler_->SetController(this);
-}
-
-bool MimeTypeResourceHandler::OnResponseStarted(ResourceResponse* response,
-                                                bool* defer) {
-  response_ = response;
+InterceptingResourceHandler::~InterceptingResourceHandler() {}
 
-  // A 304 response should not contain a Content-Type header (RFC 7232 section
-  // 4.1). The following code may incorrectly attempt to add a Content-Type to
-  // the response, and so must be skipped for 304 responses.
-  if (!(response_->head.headers.get() &&
-        response_->head.headers->response_code() == 304)) {
-    if (ShouldSniffContent()) {
-      state_ = STATE_BUFFERING;
-      return true;
-    }
-
-    if (response_->head.mime_type.empty()) {
-      // Ugg.  The server told us not to sniff the content but didn't give us
-      // a mime type.  What's a browser to do?  Turns out, we're supposed to
-      // treat the response as "text/plain".  This is the most secure option.
-      response_->head.mime_type.assign("text/plain");
-    }
-
-    // Treat feed types as text/plain.
-    if (response_->head.mime_type == "application/rss+xml" ||
-        response_->head.mime_type == "application/atom+xml") {
-      response_->head.mime_type.assign("text/plain");
-    }
+bool InterceptingResourceHandler::OnResponseStarted(ResourceResponse* response,
+                                                    bool* defer) {
+  if (response->head.headers.get() &&
+      response->head.headers->response_code() == 304) {

[asanka, 2016/06/22 20:26:53]

For HTTP(-like) responses, this class does the following:

  304: Short-circuit everything and avoid any interception.

  2xx: Intercept if necessary and swap the handler with the new one.

  everything else: Create a ResourceHandler for interception, but destroy it
immediately (see UseAlternateResourceHandler below).

Shall we centralize the interception policy calculation? Doing so would allow
this class to decide whether an interception is allowed or not *before* creating
any resource handlers.

That would also indirectly fix the bug below where
ResourceRequestInfo::is_download is set for a resource that's not a download if
UseAlternateResourceHandler decides not to allow interception after all.

[mmenke, 2016/06/22 20:31:18]

If this is an <a download> link, but we get a 4xx error, don't we need to create
a ResourceHandler to have a download error appear on the download shelf?  At
least I assumed that was the purpose of the "Everything else" behavior.

[asanka, 2016/06/23 02:26:17]

<a download>s are explicit downloads and don't use the resource handler
switching mechanism (previously known as BufferedResourceHandler and
MimeTypeResourceHandler). Instead, for explicit downloads,
ResourceDispatcherHostImpl constructs the ResourceHandler chain with
DownloadResourceHandler already attached at the end and has to handle all
responses including 404s. Hence for explicit downloads, the
DownloadResourceHandler always gets called for error responses. (well, with the
exception of any interference by one of the throttles on
ThrottlingResourceHandler).

The mechanism in the "Everything else" category above destroys the pending
ResourceHandler chain without calling OnResponseStarted/OnResponseComplete. I.e.
those handlers don't see a response.

[clamy, 2016/06/28 16:20:41]

I've moved the interception choice case to the MimeSniffingResourceHandler so
that the ResourceRequestInfo is properly updated (is_stream and is_download).
There, I've put a specific method to check whether we should attempt
interception or not at all.

[clamy, 2016/06/29 13:35:57]

It appears that one of teh unit tests is (ForbiddenDownload) is enforcing the
behavior of canceling with error if we get a non 2* response on a request we
went to intercept. I put that back in place.

+    state_ = State::DONE;
+    return next_handler_->OnResponseStarted(response, defer);
   }
 
-  state_ = STATE_PROCESSING;
-  return ProcessResponse(defer);
-}
-
-bool MimeTypeResourceHandler::OnWillStart(const GURL& url, bool* defer) {
-  const char* accept_value = nullptr;
-  switch (GetRequestInfo()->GetResourceType()) {
-    case RESOURCE_TYPE_MAIN_FRAME:
-    case RESOURCE_TYPE_SUB_FRAME:
-      accept_value = kFrameAcceptHeader;
-      break;
-    case RESOURCE_TYPE_STYLESHEET:
-      accept_value = kStylesheetAcceptHeader;
-      break;
-    case RESOURCE_TYPE_IMAGE:
-      accept_value = kImageAcceptHeader;
-      break;
-    case RESOURCE_TYPE_SCRIPT:
-    case RESOURCE_TYPE_FONT_RESOURCE:
-    case RESOURCE_TYPE_SUB_RESOURCE:
-    case RESOURCE_TYPE_OBJECT:
-    case RESOURCE_TYPE_MEDIA:
-    case RESOURCE_TYPE_WORKER:
-    case RESOURCE_TYPE_SHARED_WORKER:
-    case RESOURCE_TYPE_PREFETCH:
-    case RESOURCE_TYPE_FAVICON:
-    case RESOURCE_TYPE_XHR:
-    case RESOURCE_TYPE_PING:
-    case RESOURCE_TYPE_SERVICE_WORKER:
-    case RESOURCE_TYPE_CSP_REPORT:
-    case RESOURCE_TYPE_PLUGIN_RESOURCE:
-      accept_value = kDefaultAcceptHeader;
-      break;
-    case RESOURCE_TYPE_LAST_TYPE:
-      NOTREACHED();
-      break;
-  }
+  response_ = response;
 
-  // The false parameter prevents overwriting an existing accept header value,
-  // which is needed because JS can manually set an accept header on an XHR.
-  request()->SetExtraRequestHeaderByName(kAcceptHeader, accept_value, false);
-  return next_handler_->OnWillStart(url, defer);
+  return MakeHandlerChoice(defer);
 }
 
-bool MimeTypeResourceHandler::OnWillRead(scoped_refptr<net::IOBuffer>* buf,
-                                         int* buf_size,
-                                         int min_size) {
-  if (state_ == STATE_STREAMING)
+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 (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, min_size))
-      return false;
+  if (!next_handler_->OnWillRead(buf, buf_size, min_size))
+    return false;
 
-    read_buffer_ = *buf;
-    read_buffer_size_ = *buf_size;
-    DCHECK_GE(read_buffer_size_, net::kMaxBytesToSniff * 2);
-  }
+  read_buffer_ = *buf;
   return true;
 }
 
-bool MimeTypeResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
-  if (state_ == STATE_STREAMING)
+bool InterceptingResourceHandler::OnReadCompleted(int bytes_read, bool* defer) {
+  if (state_ == State::DONE)
     return next_handler_->OnReadCompleted(bytes_read, defer);
 
-  DCHECK_EQ(state_, STATE_BUFFERING);
+  DCHECK_EQ(State::WAITING_FOR_BUFFER_COPY, state_);
   bytes_read_ += bytes_read;
+  if (!CopyReadBufferToNextHandler())
+    return false;
 
-  if (!DetermineMimeType() && (bytes_read > 0))
-    return true;  // Needs more data, so keep buffering.
-
-  state_ = STATE_PROCESSING;
-  return ProcessResponse(defer);
-}
-
-void MimeTypeResourceHandler::OnResponseCompleted(
-    const net::URLRequestStatus& status,
-    const std::string& security_info,
-    bool* defer) {
-  // Upon completion, act like a pass-through handler in case the downstream
-  // handler defers OnResponseCompleted.
-  state_ = STATE_STREAMING;
-
-  next_handler_->OnResponseCompleted(status, security_info, defer);
-}
-
-void MimeTypeResourceHandler::Resume() {
-  switch (state_) {
-    case STATE_BUFFERING:
-    case STATE_PROCESSING:
-      NOTREACHED();
-      break;
-    case STATE_REPLAYING:
-      base::ThreadTaskRunnerHandle::Get()->PostTask(
-          FROM_HERE,
-          base::Bind(&MimeTypeResourceHandler::CallReplayReadCompleted,
-                     weak_ptr_factory_.GetWeakPtr()));
-      break;
-    case STATE_STARTING:
-    case STATE_STREAMING:
-      controller()->Resume();
-      break;
-  }
-}
-
-void MimeTypeResourceHandler::Cancel() {
-  controller()->Cancel();
-}
-
-void MimeTypeResourceHandler::CancelAndIgnore() {
-  controller()->CancelAndIgnore();
-}
-
-void MimeTypeResourceHandler::CancelWithError(int error_code) {
-  controller()->CancelWithError(error_code);
+  return next_handler_->OnReadCompleted(bytes_read, defer);
 }
 
-bool MimeTypeResourceHandler::ProcessResponse(bool* defer) {
-  DCHECK_EQ(STATE_PROCESSING, state_);
-
-  // TODO(darin): Stop special-casing 304 responses.
-  if (!(response_->head.headers.get() &&
-        response_->head.headers->response_code() == 304)) {
-    if (!SelectNextHandler(defer))
-      return false;
-    if (*defer)
-      return true;
-  }
-
-  state_ = STATE_REPLAYING;
-
-  if (!next_handler_->OnResponseStarted(response_.get(), defer))
+bool InterceptingResourceHandler::MakeHandlerChoice(bool* defer) {
+  if (!SelectNextHandler(defer))
     return false;
 
-  if (!read_buffer_.get()) {
-    state_ = STATE_STREAMING;
+  if (*defer)
     return true;
-  }
 
-  if (!*defer)
-    return ReplayReadCompleted(defer);
+  if (!next_handler_->OnResponseStarted(response_.get(), defer))
+    return false;
 
+  state_ = read_buffer_ && switched_handler_ ? State::WAITING_FOR_BUFFER_COPY
+                                             : State::DONE;
   return true;
 }
 
-bool MimeTypeResourceHandler::ShouldSniffContent() {
-  const std::string& mime_type = response_->head.mime_type;
-
-  std::string content_type_options;
-  request()->GetResponseHeaderByName("x-content-type-options",
-                                     &content_type_options);
-
-  bool sniffing_blocked =
-      base::LowerCaseEqualsASCII(content_type_options, "nosniff");
-  bool we_would_like_to_sniff =
-      net::ShouldSniffMimeType(request()->url(), mime_type);
-
-  if (!sniffing_blocked && we_would_like_to_sniff) {
-    // We're going to look at the data before deciding what the content type
-    // is.  That means we need to delay sending the ResponseStarted message
-    // over the IPC channel.
-    VLOG(1) << "To buffer: " << request()->url().spec();
-    return true;
-  }
-
-  return false;
-}
-
-bool MimeTypeResourceHandler::DetermineMimeType() {
-  DCHECK_EQ(STATE_BUFFERING, state_);
-
-  const std::string& type_hint = response_->head.mime_type;
-
-  std::string new_type;
-  bool made_final_decision =
-      net::SniffMimeType(read_buffer_->data(), bytes_read_, request()->url(),
-                         type_hint, &new_type);
-
-  // SniffMimeType() returns false if there is not enough data to determine
-  // the mime type. However, even if it returns false, it returns a new type
-  // that is probably better than the current one.
-  response_->head.mime_type.assign(new_type);
-
-  return made_final_decision;
-}
-
-bool MimeTypeResourceHandler::SelectPluginHandler(bool* defer,
-                                                  bool* handled_by_plugin) {
+bool InterceptingResourceHandler::SelectPluginHandler(bool* defer,
+                                                      bool* handled_by_plugin) {
   *handled_by_plugin = false;
 #if defined(ENABLE_PLUGINS)
   ResourceRequestInfoImpl* info = GetRequestInfo();
@@ -330,11 +126,13 @@ bool MimeTypeResourceHandler::SelectPluginHandler(bool* defer,
       &stale, &plugin, NULL);
 
   if (stale) {
+    state_ = State::WAITING_FOR_PLUGINS;
+
     // Refresh the plugins asynchronously.
     plugin_service_->GetPlugins(
-        base::Bind(&MimeTypeResourceHandler::OnPluginsLoaded,
+        base::Bind(&InterceptingResourceHandler::OnPluginsLoaded,
                    weak_ptr_factory_.GetWeakPtr()));
-    request()->LogBlockedBy("MimeTypeResourceHandler");
+    request()->LogBlockedBy("InterceptingResourceHandler");
     *defer = true;
     return true;
   }
@@ -359,7 +157,7 @@ bool MimeTypeResourceHandler::SelectPluginHandler(bool* defer,
   return true;
 }
 
-bool MimeTypeResourceHandler::SelectNextHandler(bool* defer) {
+bool InterceptingResourceHandler::SelectNextHandler(bool* defer) {
   DCHECK(!response_->head.mime_type.empty());
 
   ResourceRequestInfoImpl* info = GetRequestInfo();
@@ -416,7 +214,7 @@ bool MimeTypeResourceHandler::SelectNextHandler(bool* defer) {
   return UseAlternateNextHandler(std::move(handler), std::string());
 }
 
-bool MimeTypeResourceHandler::UseAlternateNextHandler(
+bool InterceptingResourceHandler::UseAlternateNextHandler(
     std::unique_ptr<ResourceHandler> new_handler,
     const std::string& payload_for_old_handler) {
   if (response_->head.headers.get() &&  // Can be NULL if FTP.
@@ -447,13 +245,10 @@ bool MimeTypeResourceHandler::UseAlternateNextHandler(
   } else {
     scoped_refptr<net::IOBuffer> buf;
     int size = 0;
-
     next_handler_->OnWillRead(&buf, &size, -1);

[asanka, 2016/06/22 20:26:53]

Is this correct? It seems that the old handler could see a sequence like the
following:

    ->OnWillRead(...)  // Due to MimeSniffingResourceHandler passing through
OnWillRead() before OnResponseStarted()

    ->OnResponseStarted(...) //  L235 above.
    ->OnWillRead(...)  // L248 above.
    ->OnReadCompleted(...) // L252 below

Are ResourceHandlers in general expected to be resilient to multiple
OnWillRead() calls without intervening OnReadCompleted()?

[clamy, 2016/06/28 16:20:41]

I'm now doing a OnReadCompleted of size 0 so that we do
OnWillRead
OnResponseStarted
OnReadCompleted
OnWillReda
OnReadCompleted.

     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);
@@ -466,36 +261,14 @@ bool MimeTypeResourceHandler::UseAlternateNextHandler(
   // This is handled entirely within the new ResourceHandler, so just reset the
   // original ResourceHandler.
   next_handler_ = std::move(new_handler);
-  next_handler_->SetController(this);
-
-  return CopyReadBufferToNextHandler();
-}
-
-bool MimeTypeResourceHandler::ReplayReadCompleted(bool* defer) {
-  DCHECK(read_buffer_.get());
-
-  bool result = next_handler_->OnReadCompleted(bytes_read_, defer);
-
-  read_buffer_ = NULL;
-  read_buffer_size_ = 0;
-  bytes_read_ = 0;
+  next_handler_->SetController(controller());
 
-  state_ = STATE_STREAMING;
+  switched_handler_ = true;
 
-  return result;
-}
-
-void MimeTypeResourceHandler::CallReplayReadCompleted() {
-  bool defer = false;
-  if (!ReplayReadCompleted(&defer)) {
-    controller()->Cancel();
-  } else if (!defer) {
-    state_ = STATE_STREAMING;
-    controller()->Resume();
-  }
+  return true;
 }
 
-bool MimeTypeResourceHandler::MustDownload() {
+bool InterceptingResourceHandler::MustDownload() {
   if (must_download_is_set_)
     return must_download_;
 
@@ -517,10 +290,19 @@ bool MimeTypeResourceHandler::MustDownload() {
   return must_download_;
 }
 
-bool MimeTypeResourceHandler::CopyReadBufferToNextHandler() {
-  if (!read_buffer_.get())
-    return true;
+void InterceptingResourceHandler::OnPluginsLoaded(
+    const std::vector<WebPluginInfo>& plugins) {
+  request()->LogUnblocked();
+  bool defer = false;
+  if (!MakeHandlerChoice(&defer)) {
+    controller()->Cancel();
+  } else if (!defer) {
+    controller()->Resume();
+  }
+}
 
+bool InterceptingResourceHandler::CopyReadBufferToNextHandler() {
+  DCHECK(read_buffer_.get());
   scoped_refptr<net::IOBuffer> buf;
   int buf_len = 0;
   if (!next_handler_->OnWillRead(&buf, &buf_len, bytes_read_))
@@ -528,18 +310,8 @@ bool MimeTypeResourceHandler::CopyReadBufferToNextHandler() {
 
   CHECK((buf_len >= bytes_read_) && (bytes_read_ >= 0));
   memcpy(buf->data(), read_buffer_->data(), bytes_read_);
+  state_ = State::DONE;
   return true;
 }
 
-void MimeTypeResourceHandler::OnPluginsLoaded(
-    const std::vector<WebPluginInfo>& plugins) {
-  request()->LogUnblocked();
-  bool defer = false;
-  if (!ProcessResponse(&defer)) {
-    controller()->Cancel();
-  } else if (!defer) {
-    controller()->Resume();
-  }
-}
-
 }  // namespace content