Simplify FetchManager's CORS preflight flag related logic

The logic to determine whether the CORS-preflight fetch is needed for
the given request is duplicated in FetchManager and
DocumentThreadableLoader. Remove one in FetchManager. This is safe
because:
- One in FetchManager::Loader::Start() sets the CORS preflight flag for
  the forbidden header names while one in DocumentThreadableLoader
  doesn't.
- But since all the headers with a forbidden header name are removed
  by the Headers guard, FetchManager::Loader::Start() never encounter
  any header with a forbidden header name.

Due to Request::Create() on the given |input| in GlobalFetch::Fetch(),
UnsafeRequestFlag() is always true in FetchManager::Loader::Start().
So, this in the condition had no effect. Remove it.

Bug: 727596
Change-Id: I7f2ddd49b619db067f68128227c7bb42ba16cd23
Reviewed-on: https://chromium-review.googlesource.com/553097
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490750}

third_party/WebKit/Source/modules/fetch/FetchManager.cpp

@@ -273,7 +273,7 @@ class FetchManager::Loader final
 
   void PerformSchemeFetch();
   void PerformNetworkError(const String& message);
-  void PerformHTTPFetch(bool cors_flag, bool cors_preflight_flag);
+  void PerformHTTPFetch();
   void PerformDataFetch();
   void Failed(const String& message);
   void NotifyFinished();
@@ -630,29 +630,12 @@ void FetchManager::Loader::Start() {
     return;
   }
 
-  // "- |request|'s mode is |CORS-with-forced-preflight|."
-  // "- |request|'s unsafe request flag is set and either |request|'s method
-  // is not a CORS-safelisted method or a header in |request|'s header list is
-  // not a CORS-safelisted header"
-  if (request_->Mode() ==
-          WebURLRequest::kFetchRequestModeCORSWithForcedPreflight ||
-      (request_->UnsafeRequestFlag() &&
-       (!FetchUtils::IsCORSSafelistedMethod(request_->Method()) ||
-        request_->HeaderList()->ContainsNonCORSSafelistedHeader()))) {
-    // "Set |request|'s response tainting to |CORS|."
-    request_->SetResponseTainting(FetchRequestData::kCORSTainting);
-    // "The result of performing an HTTP fetch using |request| with the
-    // |CORS flag| and |CORS preflight flag| set."
-    PerformHTTPFetch(true, true);
-    return;
-  }
-
-  // "- Otherwise
-  //     Set |request|'s response tainting to |CORS|."
+  // "Set |request|'s response tainting to |CORS|."
   request_->SetResponseTainting(FetchRequestData::kCORSTainting);
+
   // "The result of performing an HTTP fetch using |request| with the
   // |CORS flag| set."
-  PerformHTTPFetch(true, false);
+  PerformHTTPFetch();
 }
 
 void FetchManager::Loader::Dispose() {
@@ -670,13 +653,12 @@ void FetchManager::Loader::PerformSchemeFetch() {
   // "To perform a scheme fetch using |request|, switch on |request|'s url's
   // scheme, and run the associated steps:"
   if (SchemeRegistry::ShouldTreatURLSchemeAsSupportingFetchAPI(
-          request_->Url().Protocol())) {
+          request_->Url().Protocol()) ||
+      request_->Url().ProtocolIs("blob")) {
     // "Return the result of performing an HTTP fetch using |request|."
-    PerformHTTPFetch(false, false);
+    PerformHTTPFetch();
   } else if (request_->Url().ProtocolIsData()) {
     PerformDataFetch();
-  } else if (request_->Url().ProtocolIs("blob")) {
-    PerformHTTPFetch(false, false);
   } else {
     // FIXME: implement other protocols.
     PerformNetworkError("Fetch API cannot load " + request_->Url().GetString() +
@@ -689,12 +671,7 @@ void FetchManager::Loader::PerformNetworkError(const String& message) {
   Failed(message);
 }
 
-void FetchManager::Loader::PerformHTTPFetch(bool cors_flag,
-                                            bool cors_preflight_flag) {
-  DCHECK(SchemeRegistry::ShouldTreatURLSchemeAsSupportingFetchAPI(
-             request_->Url().Protocol()) ||
-         (request_->Url().ProtocolIs("blob") && !cors_flag &&
-          !cors_preflight_flag));
+void FetchManager::Loader::PerformHTTPFetch() {
   // CORS preflight fetch procedure is implemented inside
   // DocumentThreadableLoader.
 
@@ -709,19 +686,9 @@ void FetchManager::Loader::PerformHTTPFetch(bool cors_flag,
   switch (request_->Mode()) {
     case WebURLRequest::kFetchRequestModeSameOrigin:
     case WebURLRequest::kFetchRequestModeNoCORS:
-      request.SetFetchRequestMode(request_->Mode());
-      break;
     case WebURLRequest::kFetchRequestModeCORS:
     case WebURLRequest::kFetchRequestModeCORSWithForcedPreflight:
-      // TODO(tyoshino): Use only the flag or the mode enum inside the
-      // FetchManager. Currently both are used due to ongoing refactoring.
-      // See http://crbug.com/727596.
-      if (cors_preflight_flag) {
-        request.SetFetchRequestMode(
-            WebURLRequest::kFetchRequestModeCORSWithForcedPreflight);
-      } else {
-        request.SetFetchRequestMode(WebURLRequest::kFetchRequestModeCORS);
-      }
+      request.SetFetchRequestMode(request_->Mode());
       break;
     case WebURLRequest::kFetchRequestModeNavigate:
       // Using kFetchRequestModeSameOrigin here to reduce the security risk.
@@ -732,6 +699,11 @@ void FetchManager::Loader::PerformHTTPFetch(bool cors_flag,
 
   request.SetFetchCredentialsMode(request_->Credentials());
   for (const auto& header : request_->HeaderList()->List()) {
+    // Since |request_|'s headers are populated with either of the "request"
+    // guard or "request-no-cors" guard, we can assume that none of the headers
+    // have a name listed in the forbidden header names.
+    DCHECK(!FetchUtils::IsForbiddenHeaderName(header.first));
+
     request.AddHTTPHeaderField(AtomicString(header.first),
                                AtomicString(header.second));
   }

third_party/WebKit/Source/modules/fetch/FetchRequestData.cpp

@@ -57,7 +57,6 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
   request->url_ = url_;
   request->method_ = method_;
   request->header_list_ = header_list_->Clone();
-  request->unsafe_request_flag_ = unsafe_request_flag_;
   request->origin_ = origin_;
   request->same_origin_data_url_flag_ = same_origin_data_url_flag_;
   request->context_ = context_;
@@ -100,7 +99,6 @@ FetchRequestData::~FetchRequestData() {}
 FetchRequestData::FetchRequestData()
     : method_(HTTPNames::GET),
       header_list_(FetchHeaderList::Create()),
-      unsafe_request_flag_(false),
       context_(WebURLRequest::kRequestContextUnspecified),
       same_origin_data_url_flag_(false),
       referrer_(Referrer(ClientReferrerString(), kReferrerPolicyDefault)),

third_party/WebKit/Source/modules/fetch/FetchRequestData.h

@@ -42,8 +42,6 @@ class FetchRequestData final
   const AtomicString& Method() const { return method_; }
   void SetURL(const KURL& url) { url_ = url; }
   const KURL& Url() const { return url_; }
-  bool UnsafeRequestFlag() const { return unsafe_request_flag_; }
-  void SetUnsafeRequestFlag(bool flag) { unsafe_request_flag_ = flag; }
   WebURLRequest::RequestContext Context() const { return context_; }
   void SetContext(WebURLRequest::RequestContext context) { context_ = context; }
   PassRefPtr<SecurityOrigin> Origin() { return origin_; }
@@ -110,7 +108,6 @@ class FetchRequestData final
   AtomicString method_;
   KURL url_;
   Member<FetchHeaderList> header_list_;
-  bool unsafe_request_flag_;
   // FIXME: Support m_skipServiceWorkerFlag;
   WebURLRequest::RequestContext context_;
   RefPtr<SecurityOrigin> origin_;

third_party/WebKit/Source/modules/fetch/Request.cpp

@@ -32,7 +32,6 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch(
   request->SetURL(original->Url());
   request->SetMethod(original->Method());
   request->SetHeaderList(original->HeaderList()->Clone());
-  request->SetUnsafeRequestFlag(true);
   // FIXME: Set client.
   DOMWrapperWorld& world = script_state->World();
   if (world.IsIsolatedWorld()) {