Use net::HttpRequestHeaders in content::ResourceRequest

Today content::ResourceRequest (and mojom::URLRequest) sends extra
HTTP headers as a \r\n-delimited string, but this means that everytime
we go through URLLoader layer we need to serialize and deserialize
headers, which feels a bit error-prone and inefficient.

This CL tries to commonly use net::HttpRequestHeaders (which is basically a
simple vector of key,value struct), and adds IPC support code as following:

- content/public/common/common_param_traits.{h,cc} for legacy IPC
- services/network/public/interfaces/http_request_headers.* for mojo

Bug: 
Change-Id: Id84ea93d9c14edd2629e4a874ef022ff8d3b5b0a
Reviewed-on: https://chromium-review.googlesource.com/633316
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499187}

chrome/browser/net/network_context_configuration_browsertest.cc

@@ -266,7 +266,7 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, PRE_DiskCache) {
   // Make a request whose response should be cached.
   content::ResourceRequest request;
   request.url = test_url;
-  request.headers = "foo: foopity foo\r\n\r\n";
+  request.headers.SetHeader("foo", "foopity foo");
   simple_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
       request, loader_factory(), TRAFFIC_ANNOTATION_FOR_TESTS,
       simple_loader_helper.GetCallback());

components/safe_browsing/DEPS

@@ -8,6 +8,7 @@ include_rules = [
   "+google_apis",
   "+mojo/public/cpp",
   "+net/base",
+  "+net/http",
   "+net/log",
   "+net/traffic_annotation",
   "+net/url_request",

components/safe_browsing/browser/base_parallel_resource_throttle.cc

@@ -48,8 +48,8 @@ void BaseParallelResourceThrottle::WillStartRequest(bool* defer) {
 
   net::HttpRequestHeaders full_headers;
   resource_request.headers = request_->GetFullRequestHeaders(&full_headers)
-                                 ? full_headers.ToString()
-                                 : request_->extra_request_headers().ToString();
+                                 ? full_headers
+                                 : request_->extra_request_headers();
 
   resource_request.load_flags = request_->load_flags();
   resource_request.resource_type = resource_type_;

components/safe_browsing/browser/mojo_safe_browsing_impl.cc

@@ -83,7 +83,7 @@ void MojoSafeBrowsingImpl::CreateCheckerAndCheck(
     mojom::SafeBrowsingUrlCheckerRequest request,
     const GURL& url,
     const std::string& method,
-    const std::string& headers,
+    const net::HttpRequestHeaders& headers,
     int32_t load_flags,
     content::ResourceType resource_type,
     bool has_user_gesture,

components/safe_browsing/browser/mojo_safe_browsing_impl.h

@@ -33,7 +33,7 @@ class MojoSafeBrowsingImpl : public mojom::SafeBrowsing {
                              mojom::SafeBrowsingUrlCheckerRequest request,
                              const GURL& url,
                              const std::string& method,
-                             const std::string& headers,
+                             const net::HttpRequestHeaders& headers,
                              int32_t load_flags,
                              content::ResourceType resource_type,
                              bool has_user_gesture,

components/safe_browsing/browser/safe_browsing_url_checker_impl.cc

@@ -37,7 +37,7 @@ SafeBrowsingUrlCheckerImpl::UrlInfo::UrlInfo(UrlInfo&& other) = default;
 SafeBrowsingUrlCheckerImpl::UrlInfo::~UrlInfo() = default;
 
 SafeBrowsingUrlCheckerImpl::SafeBrowsingUrlCheckerImpl(
-    const std::string& headers,
+    const net::HttpRequestHeaders& headers,
     int load_flags,
     content::ResourceType resource_type,
     bool has_user_gesture,
@@ -142,12 +142,9 @@ void SafeBrowsingUrlCheckerImpl::OnCheckBrowseUrlResult(
   resource.web_contents_getter = web_contents_getter_;
   resource.threat_source = database_manager_->GetThreatSource();
 
-  net::HttpRequestHeaders headers;
-  headers.AddHeadersFromString(headers_);
-
   state_ = STATE_DISPLAYING_BLOCKING_PAGE;
   url_checker_delegate_->StartDisplayingBlockingPageHelper(
-      resource, urls_[next_index_].method, headers,
+      resource, urls_[next_index_].method, headers_,
       resource_type_ == content::RESOURCE_TYPE_MAIN_FRAME, has_user_gesture_);
 }
 

components/safe_browsing/browser/safe_browsing_url_checker_impl.h

@@ -14,6 +14,7 @@
 #include "components/safe_browsing/common/safe_browsing.mojom.h"
 #include "components/safe_browsing_db/database_manager.h"
 #include "content/public/common/resource_type.h"
+#include "net/http/http_request_headers.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -34,7 +35,7 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
                                    public SafeBrowsingDatabaseManager::Client {
  public:
   SafeBrowsingUrlCheckerImpl(
-      const std::string& headers,
+      const net::HttpRequestHeaders& headers,
       int load_flags,
       content::ResourceType resource_type,
       bool has_user_gesture,
@@ -102,7 +103,7 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
     CheckUrlCallback callback;
   };
 
-  const std::string headers_;
+  const net::HttpRequestHeaders headers_;
   const int load_flags_;
   const content::ResourceType resource_type_;
   const bool has_user_gesture_;

components/safe_browsing/common/BUILD.gn

@@ -63,6 +63,7 @@ mojom("interfaces") {
 
   public_deps = [
     "//content/public/common:resource_type_bindings",
+    "//services/network/public/interfaces",
     "//url/mojo:url_mojom_gurl",
   ]
 }

components/safe_browsing/common/safe_browsing.mojom

@@ -5,6 +5,7 @@
 module safe_browsing.mojom;
 
 import "content/public/common/resource_type.mojom";
+import "services/network/public/interfaces/http_request_headers.mojom";
 import "url/mojo/url.mojom";
 
 // Provided by the browser and used by renderers to perform SafeBrowsing URL
@@ -32,7 +33,7 @@ interface SafeBrowsing {
       SafeBrowsingUrlChecker& request,
       url.mojom.Url url,
       string method,
-      string headers,
+      network.mojom.HttpRequestHeaders headers,
       int32 load_flags,
       content.mojom.ResourceType resource_type,
       bool has_user_gesture) => (bool proceed, bool showed_interstitial);

components/safe_browsing/renderer/renderer_url_loader_throttle.cc

@@ -31,10 +31,12 @@ void RendererURLLoaderThrottle::WillStartRequest(
   pending_checks_++;
   // Use a weak pointer to self because |safe_browsing_| is not owned by this
   // object.
+  net::HttpRequestHeaders headers;
+  headers.CopyFrom(request.headers);
   safe_browsing_->CreateCheckerAndCheck(
       render_frame_id_, mojo::MakeRequest(&url_checker_), request.url,
-      request.method, request.headers, request.load_flags,
-      request.resource_type, request.has_user_gesture,
+      request.method, headers, request.load_flags, request.resource_type,
+      request.has_user_gesture,
       base::BindOnce(&RendererURLLoaderThrottle::OnCheckUrlResult,
                      weak_factory_.GetWeakPtr()));
   safe_browsing_ = nullptr;

components/safe_browsing/renderer/websocket_sb_handshake_throttle.cc

@@ -15,6 +15,7 @@
 #include "content/public/renderer/render_frame.h"
 #include "ipc/ipc_message.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
+#include "net/http/http_request_headers.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURL.h"
 
@@ -60,7 +61,8 @@ void WebSocketSBHandshakeThrottle::ThrottleHandshake(
   start_time_ = base::TimeTicks::Now();
   safe_browsing_->CreateCheckerAndCheck(
       render_frame_id, mojo::MakeRequest(&url_checker_), url, "GET",
-      std::string(), load_flags, content::RESOURCE_TYPE_SUB_RESOURCE, false,
+      net::HttpRequestHeaders(), load_flags,
+      content::RESOURCE_TYPE_SUB_RESOURCE, false,
       base::BindOnce(&WebSocketSBHandshakeThrottle::OnCheckResult,
                      weak_factory_.GetWeakPtr()));
 

components/safe_browsing/renderer/websocket_sb_handshake_throttle_unittest.cc

@@ -14,6 +14,7 @@
 #include "content/public/common/resource_type.h"
 #include "ipc/ipc_message.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "net/http/http_request_headers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebString.h"
@@ -37,7 +38,7 @@ class FakeSafeBrowsing : public mojom::SafeBrowsing {
                              mojom::SafeBrowsingUrlCheckerRequest request,
                              const GURL& url,
                              const std::string& method,
-                             const std::string& headers,
+                             const net::HttpRequestHeaders& headers,
                              int32_t load_flags,
                              content::ResourceType resource_type,
                              bool has_user_gesture,
@@ -60,7 +61,7 @@ class FakeSafeBrowsing : public mojom::SafeBrowsing {
   mojom::SafeBrowsingUrlCheckerRequest request_;
   GURL url_;
   std::string method_;
-  std::string headers_;
+  net::HttpRequestHeaders headers_;
   int32_t load_flags_;
   content::ResourceType resource_type_;
   bool has_user_gesture_;
@@ -119,7 +120,7 @@ TEST_F(WebSocketSBHandshakeThrottleTest, CheckArguments) {
   EXPECT_EQ(MSG_ROUTING_NONE, safe_browsing_.render_frame_id_);
   EXPECT_EQ(GURL(kTestUrl), safe_browsing_.url_);
   EXPECT_EQ("GET", safe_browsing_.method_);
-  EXPECT_TRUE(safe_browsing_.headers_.empty());
+  EXPECT_TRUE(safe_browsing_.headers_.GetHeaderVector().empty());
   EXPECT_EQ(0, safe_browsing_.load_flags_);
   EXPECT_EQ(content::RESOURCE_TYPE_SUB_RESOURCE, safe_browsing_.resource_type_);
   EXPECT_FALSE(safe_browsing_.has_user_gesture_);

content/browser/appcache/appcache_update_job_unittest.cc

@@ -608,9 +608,7 @@ class MockURLLoaderFactory : public mojom::URLLoaderFactory {
       return;
     }
 
-    net::HttpRequestHeaders request_headers;
-    request_headers.AddHeadersFromString(url_request.headers);
-    HttpHeadersRequestTestJob::ValidateExtraHeaders(request_headers);
+    HttpHeadersRequestTestJob::ValidateExtraHeaders(url_request.headers);
 
     std::string headers;
     std::string body;

content/browser/appcache/appcache_update_url_loader_request.cc

@@ -30,7 +30,7 @@ void AppCacheUpdateJob::UpdateURLLoaderRequest::Start() {
 
 void AppCacheUpdateJob::UpdateURLLoaderRequest::SetExtraRequestHeaders(
     const net::HttpRequestHeaders& headers) {
-  request_.headers = headers.ToString();
+  request_.headers = headers;
 }
 
 GURL AppCacheUpdateJob::UpdateURLLoaderRequest::GetURL() const {

content/browser/appcache/appcache_url_loader_job.cc

@@ -59,9 +59,7 @@ void AppCacheURLLoaderJob::DeliverAppCachedResponse(const GURL& manifest_url,
   is_fallback_ = is_fallback;
 
   // Handle range requests.
-  net::HttpRequestHeaders headers;
-  headers.AddHeadersFromString(request_.headers);
-  InitializeRangeRequestInfo(headers);
+  InitializeRangeRequestInfo(request_.headers);
 
   // TODO(ananta)
   // Implement the AppCacheServiceImpl::Observer interface or add weak pointer

content/browser/blob_storage/blob_url_loader_factory.cc

@@ -76,10 +76,8 @@ class BlobURLLoader : public storage::MojoBlobReader::Delegate,
       return;
     }
 
-    net::HttpRequestHeaders request_headers;
-    request_headers.AddHeadersFromString(request.headers);
     std::string range_header;
-    if (request_headers.GetHeader(net::HttpRequestHeaders::kRange,
+    if (request.headers.GetHeader(net::HttpRequestHeaders::kRange,
                                   &range_header)) {
       // We only care about "Range" header here.
       std::vector<net::HttpByteRange> ranges;

content/browser/blob_storage/blob_url_unittest.cc

@@ -277,8 +277,7 @@ class BlobURLRequestJobTest : public testing::TestWithParam<bool> {
       ResourceRequest request;
       request.url = url;
       request.method = method;
-      if (!extra_headers.IsEmpty())
-        request.headers = extra_headers.ToString();
+      request.headers = extra_headers;
 
       mojom::URLLoaderPtr url_loader;
       TestURLLoaderClient url_loader_client;

content/browser/devtools/protocol/network_handler.cc

@@ -788,9 +788,7 @@ void NetworkHandler::NavigationPreloadRequestSent(
     return;
   const std::string version_id(base::IntToString(worker_version_id));
   std::unique_ptr<DictionaryValue> headers_dict(DictionaryValue::create());
-  net::HttpRequestHeaders headers;
-  headers.AddHeadersFromString(request.headers);
-  for (net::HttpRequestHeaders::Iterator it(headers); it.GetNext();)
+  for (net::HttpRequestHeaders::Iterator it(request.headers); it.GetNext();)
     headers_dict->setString(it.name(), it.value());
   frontend_->RequestWillBeSent(
       request_id, "" /* loader_id */, request.url.spec(),

content/browser/download/download_utils.cc

@@ -212,8 +212,7 @@ std::unique_ptr<ResourceRequest> CreateResourceRequest(
   // Add additional request headers.
   std::unique_ptr<net::HttpRequestHeaders> headers =
       GetAdditionalRequestHeaders(params);
-  if (!headers->IsEmpty())
-    request->headers = headers->ToString();
+  request->headers.Swap(headers.get());
 
   return request;
 }

content/browser/loader/navigation_url_loader_network_service.cc

@@ -436,7 +436,7 @@ NavigationURLLoaderNetworkService::NavigationURLLoaderNetworkService(
   new_request->request_initiator = request_info->begin_params.initiator_origin;
   new_request->referrer = request_info->common_params.referrer.url;
   new_request->referrer_policy = request_info->common_params.referrer.policy;
-  new_request->headers = request_info->begin_params.headers;
+  new_request->headers.AddHeadersFromString(request_info->begin_params.headers);
 
   int load_flags = request_info->begin_params.load_flags;
   load_flags |= net::LOAD_VERIFY_EV_CERT;

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -1166,12 +1166,9 @@ void ResourceDispatcherHostImpl::BeginRequest(
 
   // Parse the headers before calling ShouldServiceRequest, so that they are
   // available to be validated.
-  net::HttpRequestHeaders headers;
-  headers.AddHeadersFromString(request_data.headers);
-
   if (is_shutdown_ ||
-      !ShouldServiceRequest(child_id, request_data, headers, requester_info,
-                            resource_context)) {
+      !ShouldServiceRequest(child_id, request_data, request_data.headers,
+                            requester_info, resource_context)) {
     AbortRequestBeforeItStarts(requester_info->filter(), sync_result_handler,
                                request_id, std::move(url_loader_client));
     return;
@@ -1204,7 +1201,8 @@ void ResourceDispatcherHostImpl::BeginRequest(
     // yes then we need to mark the current request as pending and wait for the
     // interceptor to invoke the callback with a status code indicating whether
     // the request needs to be aborted or continued.
-    for (net::HttpRequestHeaders::Iterator it(headers); it.GetNext();) {
+    for (net::HttpRequestHeaders::Iterator it(request_data.headers);
+         it.GetNext();) {
       HeaderInterceptorMap::iterator index =
           http_header_interceptor_map_.find(it.name());
       if (index != http_header_interceptor_map_.end()) {
@@ -1223,19 +1221,21 @@ void ResourceDispatcherHostImpl::BeginRequest(
                   &ResourceDispatcherHostImpl::ContinuePendingBeginRequest,
                   base::Unretained(this), make_scoped_refptr(requester_info),
                   request_id, request_data, is_sync_load, sync_result_handler,
-                  route_id, headers, base::Passed(std::move(mojo_request)),
+                  route_id, request_data.headers,
+                  base::Passed(std::move(mojo_request)),
                   base::Passed(std::move(url_loader_client)),
                   base::Passed(std::move(blob_handles)), traffic_annotation));
           return;
         }
       }
     }
   }
-  ContinuePendingBeginRequest(
-      requester_info, request_id, request_data, is_sync_load,
-      sync_result_handler, route_id, headers, std::move(mojo_request),
-      std::move(url_loader_client), std::move(blob_handles), traffic_annotation,
-      HeaderInterceptorResult::CONTINUE);
+  ContinuePendingBeginRequest(requester_info, request_id, request_data,
+                              is_sync_load, sync_result_handler, route_id,
+                              request_data.headers, std::move(mojo_request),
+                              std::move(url_loader_client),
+                              std::move(blob_handles), traffic_annotation,
+                              HeaderInterceptorResult::CONTINUE);
 }
 
 void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(

content/browser/security_exploit_browsertest.cc

@@ -139,7 +139,7 @@ ResourceRequest CreateXHRRequest(const char* url) {
 ResourceRequest CreateXHRRequestWithOrigin(const char* origin) {
   ResourceRequest request = CreateXHRRequest("http://bar.com/simple_page.html");
   request.site_for_cookies = GURL(origin);
-  request.headers = base::StringPrintf("Origin: %s\r\n", origin);
+  request.headers.SetHeader(net::HttpRequestHeaders::kOrigin, origin);
   return request;
 }
 

content/browser/service_worker/service_worker_controllee_request_handler.cc

@@ -213,9 +213,7 @@ void ServiceWorkerControlleeRequestHandler::MaybeCreateLoader(
 #if BUILDFLAG(ENABLE_OFFLINE_PAGES)
   // Fall back for the subsequent offline page interceptor to load the offline
   // snapshot of the page if required.
-  net::HttpRequestHeaders extra_request_headers;
-  extra_request_headers.AddHeadersFromString(resource_request.headers);
-  if (ShouldFallbackToLoadOfflinePage(extra_request_headers)) {
+  if (ShouldFallbackToLoadOfflinePage(resource_request.headers)) {
     std::move(callback).Run(StartLoaderCallback());
     return;
   }

content/browser/service_worker/service_worker_fetch_dispatcher.cc

@@ -654,9 +654,9 @@ bool ServiceWorkerFetchDispatcher::MaybeStartNavigationPreload(
       version_->navigation_preload_state().header));
   ServiceWorkerMetrics::RecordNavigationPreloadRequestHeaderSize(
       version_->navigation_preload_state().header.length());
-  request.headers = "Service-Worker-Navigation-Preload: " +
-                    version_->navigation_preload_state().header + "\r\n" +
-                    original_request->extra_request_headers().ToString();
+  request.headers.CopyFrom(original_request->extra_request_headers());
+  request.headers.SetHeader("Service-Worker-Navigation-Preload",
+                            version_->navigation_preload_state().header);
 
   const int request_id = ResourceDispatcherHostImpl::Get()->MakeRequestID();
   DCHECK_LT(request_id, -1);

content/browser/webui/web_ui_url_loader_factory.cc

@@ -156,10 +156,8 @@ void StartURLLoader(const ResourceRequest& request,
   std::string path;
   URLDataManagerBackend::URLToRequestPath(request.url, &path);
 
-  net::HttpRequestHeaders request_headers;
-  request_headers.AddHeadersFromString(request.headers);
   std::string origin_header;
-  request_headers.GetHeader(net::HttpRequestHeaders::kOrigin, &origin_header);
+  request.headers.GetHeader(net::HttpRequestHeaders::kOrigin, &origin_header);
 
   scoped_refptr<net::HttpResponseHeaders> headers =
       URLDataManagerBackend::GetHeaders(source, path, origin_header);