OOR-CORS: Simplify DocumentThreadableLoader's preflight request creation code

Now DocumentThreadableLoader uses Blink API types to construct a
CORS-preflight request, but this isn't necessary.

Also, this change prefer using std::unique_ptr for ResourceRequest
so to respect the direction of crbug.com/787704.

Bug: 803766, 787704
Change-Id: I897b17b7ea20e47316c05436c7d36438e03f2d7d
Reviewed-on: https://chromium-review.googlesource.com/964074
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546062}

third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp

@@ -74,7 +74,8 @@ namespace blink {
 namespace {
 
 // Fetch API Spec: https://fetch.spec.whatwg.org/#cors-preflight-fetch-0
-String CreateAccessControlRequestHeadersHeader(const HTTPHeaderMap& headers) {
+AtomicString CreateAccessControlRequestHeadersHeader(
+    const HTTPHeaderMap& headers) {
   Vector<String> filtered_headers;
   for (const auto& header : headers) {
     // Exclude CORS-safelisted headers.
@@ -102,7 +103,7 @@ String CreateAccessControlRequestHeadersHeader(const HTTPHeaderMap& headers) {
     header_buffer.Append(header);
   }
 
-  return header_buffer.ToString();
+  return header_buffer.ToAtomicString();
 }
 
 class EmptyDataHandle final : public WebDataConsumerHandle {
@@ -165,45 +166,51 @@ void DocumentThreadableLoader::LoadResourceSynchronously(
 }
 
 // static
-WebURLRequest DocumentThreadableLoader::CreateAccessControlPreflightRequest(
-    const WebURLRequest& request) {
+std::unique_ptr<ResourceRequest>
+DocumentThreadableLoader::CreateAccessControlPreflightRequest(
+    const ResourceRequest& request,
+    const SecurityOrigin* origin) {
   const KURL& request_url = request.Url();
 
   DCHECK(request_url.User().IsEmpty());
   DCHECK(request_url.Pass().IsEmpty());
 
-  WebURLRequest preflight_request(request_url);
-  preflight_request.SetHTTPMethod(HTTPNames::OPTIONS);
-  preflight_request.SetHTTPHeaderField(HTTPNames::Access_Control_Request_Method,
-                                       request.HttpMethod());
-  preflight_request.SetPriority(request.GetPriority());
-  preflight_request.SetRequestContext(request.GetRequestContext());
-  preflight_request.SetFetchCredentialsMode(
+  std::unique_ptr<ResourceRequest> preflight_request =
+      std::make_unique<ResourceRequest>(request_url);
+  preflight_request->SetHTTPMethod(HTTPNames::OPTIONS);
+  preflight_request->SetHTTPHeaderField(
+      HTTPNames::Access_Control_Request_Method, request.HttpMethod());
+  preflight_request->SetPriority(request.Priority());
+  preflight_request->SetRequestContext(request.GetRequestContext());
+  preflight_request->SetFetchCredentialsMode(
       network::mojom::FetchCredentialsMode::kOmit);
-  preflight_request.SetSkipServiceWorker(true);
-  preflight_request.SetHTTPReferrer(request.HttpHeaderField(HTTPNames::Referer),
-                                    request.GetReferrerPolicy());
+  preflight_request->SetSkipServiceWorker(true);
+  preflight_request->SetHTTPReferrer(
+      Referrer(request.HttpReferrer(), request.GetReferrerPolicy()));
 
   if (request.IsExternalRequest()) {
-    preflight_request.SetHTTPHeaderField(
+    preflight_request->SetHTTPHeaderField(
         HTTPNames::Access_Control_Request_External, "true");
   }
 
-  String request_headers = CreateAccessControlRequestHeadersHeader(
-      request.ToResourceRequest().HttpHeaderFields());
+  const AtomicString request_headers =
+      CreateAccessControlRequestHeadersHeader(request.HttpHeaderFields());
   if (request_headers != g_null_atom) {
-    preflight_request.SetHTTPHeaderField(
+    preflight_request->SetHTTPHeaderField(
         HTTPNames::Access_Control_Request_Headers, request_headers);
   }
 
+  if (origin)
+    preflight_request->SetHTTPOrigin(origin);
+
   return preflight_request;
 }
 
 // static
-WebURLRequest
+std::unique_ptr<ResourceRequest>
 DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
-    const WebURLRequest& request) {
-  return CreateAccessControlPreflightRequest(request);
+    const ResourceRequest& request) {
+  return CreateAccessControlPreflightRequest(request, nullptr);
 }
 
 // static
@@ -456,16 +463,8 @@ void DocumentThreadableLoader::PrepareCrossOriginRequest(
 void DocumentThreadableLoader::LoadPreflightRequest(
     const ResourceRequest& actual_request,
     const ResourceLoaderOptions& actual_options) {
-  WebURLRequest web_url_request = CreateAccessControlPreflightRequest(
-      WrappedResourceRequest(actual_request));
-
-  ResourceRequest& preflight_request =
-      web_url_request.ToMutableResourceRequest();
-
-  // TODO(tyoshino): Call PrepareCrossOriginRequest(preflight_request) to also
-  // set the referrer header.
-  if (GetSecurityOrigin())
-    preflight_request.SetHTTPOrigin(GetSecurityOrigin());
+  std::unique_ptr<ResourceRequest> preflight_request =
+      CreateAccessControlPreflightRequest(actual_request, GetSecurityOrigin());
 
   actual_request_ = actual_request;
   actual_options_ = actual_options;
@@ -479,7 +478,7 @@ void DocumentThreadableLoader::LoadPreflightRequest(
   // Create a ResourceLoaderOptions for preflight.
   ResourceLoaderOptions preflight_options = actual_options;
 
-  LoadRequest(preflight_request, preflight_options);
+  LoadRequest(*preflight_request, preflight_options);
 }
 
 void DocumentThreadableLoader::MakeCrossOriginAccessRequest(
@@ -1345,10 +1344,10 @@ void DocumentThreadableLoader::LoadRequest(
     case network::mojom::FetchCredentialsMode::kOmit:
       break;
     case network::mojom::FetchCredentialsMode::kSameOrigin:
-      // TODO(tyoshino): It's wrong to use |cors_flag| here. Fix it to use the
+      // TODO(toyoshim): It's wrong to use |cors_flag| here. Fix it to use the
       // response tainting.
       //
-      // TODO(tyoshino): The credentials mode must work even when the "no-cors"
+      // TODO(toyoshim): The credentials mode must work even when the "no-cors"
       // mode is in use. See the following issues:
       // - https://github.com/whatwg/fetch/issues/130
       // - https://github.com/whatwg/fetch/issues/169

third_party/WebKit/Source/core/loader/DocumentThreadableLoader.h

@@ -68,8 +68,8 @@ class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader,
                                         const ResourceLoaderOptions&);
 
   // Exposed for testing. Code outside this class should not call this function.
-  static WebURLRequest CreateAccessControlPreflightRequestForTesting(
-      const WebURLRequest&);
+  static std::unique_ptr<ResourceRequest>
+  CreateAccessControlPreflightRequestForTesting(const ResourceRequest&);
 
   static DocumentThreadableLoader* Create(ThreadableLoadingContext&,
                                           ThreadableLoaderClient*,
@@ -90,8 +90,9 @@ class CORE_EXPORT DocumentThreadableLoader final : public ThreadableLoader,
  private:
   enum BlockingBehavior { kLoadSynchronously, kLoadAsynchronously };
 
-  static WebURLRequest CreateAccessControlPreflightRequest(
-      const WebURLRequest&);
+  static std::unique_ptr<ResourceRequest> CreateAccessControlPreflightRequest(
+      const ResourceRequest&,
+      const SecurityOrigin*);
 
   DocumentThreadableLoader(ThreadableLoadingContext&,
                            ThreadableLoaderClient*,

third_party/WebKit/Source/core/loader/DocumentThreadableLoaderTest.cpp

@@ -14,77 +14,77 @@ namespace blink {
 namespace {
 
 TEST(DocumentThreadableLoaderCreatePreflightRequestTest, LexicographicalOrder) {
-  WebURLRequest request;
+  ResourceRequest request;
   request.AddHTTPHeaderField("Orange", "Orange");
   request.AddHTTPHeaderField("Apple", "Red");
   request.AddHTTPHeaderField("Kiwifruit", "Green");
   request.AddHTTPHeaderField("Content-Type", "application/octet-stream");
   request.AddHTTPHeaderField("Strawberry", "Red");
 
-  WebURLRequest preflight =
+  std::unique_ptr<ResourceRequest> preflight =
       DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
           request);
 
   EXPECT_EQ("apple,content-type,kiwifruit,orange,strawberry",
-            preflight.HttpHeaderField("Access-Control-Request-Headers"));
+            preflight->HttpHeaderField("Access-Control-Request-Headers"));
 }
 
 TEST(DocumentThreadableLoaderCreatePreflightRequestTest, ExcludeSimpleHeaders) {
-  WebURLRequest request;
+  ResourceRequest request;
   request.AddHTTPHeaderField("Accept", "everything");
   request.AddHTTPHeaderField("Accept-Language", "everything");
   request.AddHTTPHeaderField("Content-Language", "everything");
   request.AddHTTPHeaderField("Save-Data", "on");
 
-  WebURLRequest preflight =
+  std::unique_ptr<ResourceRequest> preflight =
       DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
           request);
 
   // Do not emit empty-valued headers; an empty list of non-"CORS safelisted"
   // request headers should cause "Access-Control-Request-Headers:" to be
   // left out in the preflight request.
-  EXPECT_EQ(WebString(g_null_atom),
-            preflight.HttpHeaderField("Access-Control-Request-Headers"));
+  EXPECT_EQ(g_null_atom,
+            preflight->HttpHeaderField("Access-Control-Request-Headers"));
 }
 
 TEST(DocumentThreadableLoaderCreatePreflightRequestTest,
      ExcludeSimpleContentTypeHeader) {
-  WebURLRequest request;
+  ResourceRequest request;
   request.AddHTTPHeaderField("Content-Type", "text/plain");
 
-  WebURLRequest preflight =
+  std::unique_ptr<ResourceRequest> preflight =
       DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
           request);
 
   // Empty list also; see comment in test above.
-  EXPECT_EQ(WebString(g_null_atom),
-            preflight.HttpHeaderField("Access-Control-Request-Headers"));
+  EXPECT_EQ(g_null_atom,
+            preflight->HttpHeaderField("Access-Control-Request-Headers"));
 }
 
 TEST(DocumentThreadableLoaderCreatePreflightRequestTest,
      IncludeNonSimpleHeader) {
-  WebURLRequest request;
+  ResourceRequest request;
   request.AddHTTPHeaderField("X-Custom-Header", "foobar");
 
-  WebURLRequest preflight =
+  std::unique_ptr<ResourceRequest> preflight =
       DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
           request);
 
   EXPECT_EQ("x-custom-header",
-            preflight.HttpHeaderField("Access-Control-Request-Headers"));
+            preflight->HttpHeaderField("Access-Control-Request-Headers"));
 }
 
 TEST(DocumentThreadableLoaderCreatePreflightRequestTest,
      IncludeNonSimpleContentTypeHeader) {
-  WebURLRequest request;
+  ResourceRequest request;
   request.AddHTTPHeaderField("Content-Type", "application/octet-stream");
 
-  WebURLRequest preflight =
+  std::unique_ptr<ResourceRequest> preflight =
       DocumentThreadableLoader::CreateAccessControlPreflightRequestForTesting(
           request);
 
   EXPECT_EQ("content-type",
-            preflight.HttpHeaderField("Access-Control-Request-Headers"));
+            preflight->HttpHeaderField("Access-Control-Request-Headers"));
 }
 
 }  // namespace