[NetworkService] Attach "accept" header appropriately

Share the logic with MimeSniffingResourceHandler.

Bug: 778721
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I4a90beee86a5182b0a8d1c6225c4aaa711abed7d
Reviewed-on: https://chromium-review.googlesource.com/745661
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512776}

content/browser/loader/mime_sniffing_resource_handler.cc

@@ -45,14 +45,6 @@ namespace content {
 
 namespace {
 
-const char kAcceptHeader[] = "Accept";
-const char kFrameAcceptHeader[] =
-    "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,"
-    "image/apng,*/*;q=0.8";
-const char kStylesheetAcceptHeader[] = "text/css,*/*;q=0.1";
-const char kImageAcceptHeader[] = "image/webp,image/apng,image/*,*/*;q=0.8";
-const char kDefaultAcceptHeader[] = "*/*";
-
 // Used to write into an existing IOBuffer at a given offset. This is
 // very similar to DependentIOBufferForRedirectToFile and
 // DependentIOBufferForAsyncLoading but not identical.
@@ -139,42 +131,7 @@ void MimeSniffingResourceHandler::OnWillStart(
     std::unique_ptr<ResourceController> controller) {
   DCHECK(!has_controller());
 
-  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_FAVICON:
-    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_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;
-  }
-
-  // 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);
+  AttachAcceptHeader(GetRequestInfo()->GetResourceType(), request());
   next_handler_->OnWillStart(url, std::move(controller));
 }
 

content/common/loader_util.cc

@@ -16,6 +16,16 @@
 
 namespace content {
 
+namespace {
+constexpr char kAcceptHeader[] = "Accept";
+constexpr char kFrameAcceptHeader[] =
+    "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,"
+    "image/apng,*/*;q=0.8";
+constexpr char kStylesheetAcceptHeader[] = "text/css,*/*;q=0.1";
+constexpr char kImageAcceptHeader[] = "image/webp,image/apng,image/*,*/*;q=0.8";
+constexpr char kDefaultAcceptHeader[] = "*/*";
+}  //  namespace
+
 bool ShouldSniffContent(net::URLRequest* url_request,
                         ResourceResponse* response) {
   const std::string& mime_type = response->head.mime_type;
@@ -87,4 +97,42 @@ scoped_refptr<ResourceDevToolsInfo> BuildDevToolsInfo(
   return info;
 }
 
+void AttachAcceptHeader(ResourceType type, net::URLRequest* request) {
+  const char* accept_value = nullptr;
+  switch (type) {
+    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_FAVICON:
+    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_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;
+  }
+  // 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);
+}
+
 }  // namespace content

content/common/loader_util.h

@@ -6,6 +6,7 @@
 #define CONTENT_COMMON_LOADER_UTIL_H_
 
 #include "base/memory/ref_counted.h"
+#include "content/public/common/resource_type.h"
 
 namespace net {
 class HttpRawRequestHeaders;
@@ -31,6 +32,8 @@ scoped_refptr<ResourceDevToolsInfo> BuildDevToolsInfo(
     const net::HttpRawRequestHeaders& raw_request_headers,
     const net::HttpResponseHeaders* raw_response_headers);
 
+void AttachAcceptHeader(ResourceType type, net::URLRequest* request);
+
 }  // namespace content
 
 #endif  // CONTENT_COMMON_LOADER_UTIL_H_

content/network/url_loader_impl.cc

@@ -322,6 +322,9 @@ URLLoaderImpl::URLLoaderImpl(
     url_request_->SetResponseHeadersCallback(base::Bind(
         &URLLoaderImpl::SetRawResponseHeaders, base::Unretained(this)));
   }
+
+  AttachAcceptHeader(request.resource_type, url_request_.get());
+
   url_request_->Start();
 }
 

content/network/url_loader_unittest.cc

@@ -162,6 +162,9 @@ class URLLoaderImplTest : public testing::Test {
   void SetUp() override {
     test_server_.AddDefaultHandlers(
         base::FilePath(FILE_PATH_LITERAL("content/test/data")));
+    // This Unretained is safe because test_server_ is owned by |this|.
+    test_server_.RegisterRequestMonitor(
+        base::Bind(&URLLoaderImplTest::Monitor, base::Unretained(this)));
     ASSERT_TRUE(test_server_.Start());
   }
 
@@ -174,13 +177,14 @@ class URLLoaderImplTest : public testing::Test {
     DCHECK(!ran_);
     mojom::URLLoaderPtr loader;
 
-    ResourceRequest request =
-        CreateResourceRequest("GET", RESOURCE_TYPE_MAIN_FRAME, url);
+    ResourceRequest request = CreateResourceRequest("GET", resource_type_, url);
     uint32_t options = mojom::kURLLoadOptionNone;
     if (send_ssl_)
       options |= mojom::kURLLoadOptionSendSSLInfo;
     if (sniff_)
       options |= mojom::kURLLoadOptionSniffMimeType;
+    if (add_custom_accept_header_)
+      request.headers.SetHeader("accept", "custom/*");
 
     URLLoaderImpl loader_impl(context(), mojo::MakeRequest(&loader), options,
                               request, false, client_.CreateInterfacePtr(),
@@ -281,6 +285,14 @@ class URLLoaderImplTest : public testing::Test {
     DCHECK(!ran_);
     send_ssl_ = true;
   }
+  void set_add_custom_accept_header() {
+    DCHECK(!ran_);
+    add_custom_accept_header_ = true;
+  }
+  void set_resource_type(ResourceType type) {
+    DCHECK(!ran_);
+    resource_type_ = type;
+  }
 
   // Convenience methods after calling Load();
   std::string mime_type() const {
@@ -354,15 +366,26 @@ class URLLoaderImplTest : public testing::Test {
     return std::string(buffer.data(), buffer.size());
   }
 
+  const net::test_server::HttpRequest& sent_request() const {
+    return sent_request_;
+  }
+
  private:
+  void Monitor(const net::test_server::HttpRequest& request) {
+    sent_request_ = request;
+  }
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   net::EmbeddedTestServer test_server_;
   std::unique_ptr<NetworkContext> context_;
   bool sniff_ = false;
   bool send_ssl_ = false;
+  bool add_custom_accept_header_ = false;
+  ResourceType resource_type_ = RESOURCE_TYPE_MAIN_FRAME;
   // Used to ensure that methods are called either before or after a request is
   // made, since the test fixture is meant to be used only once.
   bool ran_ = false;
+  net::test_server::HttpRequest sent_request_;
   TestURLLoaderClient client_;
 };
 
@@ -853,4 +876,35 @@ TEST_F(URLLoaderImplTest, MultiplePauseResumeReadingBodyFromNet) {
             ReadBody());
 }
 
+TEST_F(URLLoaderImplTest, AttachAcceptHeaderForStyleSheet) {
+  set_resource_type(RESOURCE_TYPE_STYLESHEET);
+  EXPECT_EQ(net::OK,
+            Load(test_server()->GetURL("/content-sniffer-test0.html")));
+
+  auto it = sent_request().headers.find("accept");
+  ASSERT_NE(it, sent_request().headers.end());
+  EXPECT_EQ(it->second, "text/css,*/*;q=0.1");
+}
+
+TEST_F(URLLoaderImplTest, AttachAcceptHeaderForXHR) {
+  set_resource_type(RESOURCE_TYPE_XHR);
+  EXPECT_EQ(net::OK,
+            Load(test_server()->GetURL("/content-sniffer-test0.html")));
+
+  auto it = sent_request().headers.find("accept");
+  ASSERT_NE(it, sent_request().headers.end());
+  EXPECT_EQ(it->second, "*/*");
+}
+
+TEST_F(URLLoaderImplTest, DoNotOverrideAcceptHeader) {
+  set_resource_type(RESOURCE_TYPE_XHR);
+  set_add_custom_accept_header();
+  EXPECT_EQ(net::OK,
+            Load(test_server()->GetURL("/content-sniffer-test0.html")));
+
+  auto it = sent_request().headers.find("accept");
+  ASSERT_NE(it, sent_request().headers.end());
+  EXPECT_EQ(it->second, "custom/*");
+}
+
 }  // namespace content

third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

@@ -9,7 +9,6 @@ Bug(none) virtual/mojo-loading [ Skip ]
 
 Bug(none) external/wpt/clear-site-data/storage.https.html [ Failure ]
 Bug(none) external/wpt/XMLHttpRequest/overridemimetype-blob.html [ Failure ]
-Bug(none) external/wpt/XMLHttpRequest/send-accept.htm [ Failure ]
 Bug(none) external/wpt/beacon/headers/header-content-type.html [ Timeout ]
 Bug(none) external/wpt/clear-site-data/navigation.https.html [ Timeout ]
 Bug(none) external/wpt/content-security-policy/generic/filesystem-urls-match-filesystem.sub.html [ Failure ]
@@ -18,8 +17,6 @@ Bug(none) external/wpt/content-security-policy/inside-worker/dedicated-script.ht
 Bug(none) external/wpt/cookies/secure/set-from-wss.https.sub.html [ Failure ]
 Bug(none) external/wpt/cssom-view/scrolling-quirks-vs-nonquirks.html [ Failure Timeout Crash ]
 Bug(none) external/wpt/cssom-view/scrollingElement.html [ Failure Timeout Crash ]
-Bug(none) external/wpt/fetch/api/basic/accept-header.any.html [ Failure ]
-Bug(none) external/wpt/fetch/api/basic/accept-header.any.worker.html [ Failure ]
 Bug(none) external/wpt/fetch/api/cors/cors-cookies.any.html [ Failure Pass ]
 Bug(none) external/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-weird.html [ Failure Timeout Failure ]
 Bug(none) external/wpt/html/browsers/offline/appcache/workers/appcache-worker.html [ Timeout ]
@@ -126,7 +123,6 @@ Bug(none) http/tests/local/fileapi/send-sliced-dragged-file.html [ Failure Timeo
 Bug(none) http/tests/local/serviceworker/fetch-request-body-file.html [ Crash Timeout ]
 Bug(none) http/tests/media/video-buffered.html [ Timeout ]
 Bug(none) http/tests/misc/embed-image-load-outlives-gc-without-crashing.html [ Failure Pass ]
-Bug(none) http/tests/misc/image-checks-for-accept.html [ Failure Pass ]
 Bug(none) http/tests/misc/image-input-type-outlives-gc-without-crashing.html [ Failure Pass ]
 Bug(none) http/tests/misc/image-load-outlives-gc-without-crashing.html [ Failure Pass ]
 Bug(none) http/tests/misc/redirect-to-about-blank.html [ Timeout ]