Fix foreign fetch intercepting CORS preflighted requests if preflight was in cache.

This moves setting of the skip service worker flag on CORS preflighted requests
to happen earlier so that even non-simple requests for which we don't end up
sending a preflight request still skip any service workers.

BUG=674370

Review-Url: https://codereview.chromium.org/2582833002
Cr-Commit-Position: refs/heads/master@{#439863}

third_party/WebKit/LayoutTests/http/tests/serviceworker/foreign-fetch-basics.html

@@ -234,6 +234,10 @@ promise_test(t => {
     var remote_url =
         host_info.HTTPS_REMOTE_ORIGIN + '/serviceworker/resources/' + scope;
     return install_cross_origin_worker(t, worker_for_scopes(['']), scope)
+      .then(() => fetch(remote_url, {method: 'SPECIAL'}))
+      .then(response => response.text())
+      .then(response_text => assert_true(response_text.startsWith('report(')))
+      // Do the whole thing twice to test CORS preflight cache behavior.
       .then(() => fetch(remote_url, {method: 'SPECIAL'}))
       .then(response => response.text())
       .then(response_text => assert_true(response_text.startsWith('report(')));

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

@@ -369,6 +369,16 @@ void DocumentThreadableLoader::makeCrossOriginAccessRequest(
     prepareCrossOriginRequest(crossOriginRequest);
     loadRequest(crossOriginRequest, crossOriginOptions);
   } else {
+    // Explicitly set the SkipServiceWorker flag here. Although the page is not
+    // controlled by a SW at this point, a new SW may be controlling the page
+    // when this request gets sent later. We should not send the actual request
+    // to the SW. https://crbug.com/604583
+    // Similarly we don't want any requests that could involve a CORS preflight
+    // to get intercepted by a foreign fetch service worker, even if we have the
+    // result of the preflight cached already. https://crbug.com/674370
+    crossOriginRequest.setSkipServiceWorker(
+        WebURLRequest::SkipServiceWorker::All);
+
     bool shouldForcePreflight =
         request.isExternalRequest() ||
         InspectorInstrumentation::shouldForceCORSPreflight(m_document);
@@ -916,12 +926,6 @@ void DocumentThreadableLoader::loadActualRequest() {
 
   clearResource();
 
-  // Explicitly set the SkipServiceWorker flag here. Even if the page was not
-  // controlled by a SW when the preflight request was sent, a new SW may be
-  // controlling the page now by calling clients.claim(). We should not send
-  // the actual request to the SW. https://crbug.com/604583
-  actualRequest.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::All);
-
   prepareCrossOriginRequest(actualRequest);
   loadRequest(actualRequest, actualOptions);
 }