Skip foreign fetch when the skipServiceWorker flag is set on a request.

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

Index: third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
index a366b38b3696fee964b23fdc6283b8508601b4c8..fce33b053886377d9478c2a7d7a5be8b02706e13 100644
--- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -97,7 +97,7 @@ private:
 
 // No-CORS requests are allowed for all these contexts, and plugin contexts with
 // private permission when we set skipServiceWorker flag in PepperURLLoaderHost.
-bool IsNoCORSAllowedContext(WebURLRequest::RequestContext context, bool skipServiceWorker)
+bool IsNoCORSAllowedContext(WebURLRequest::RequestContext context, WebURLRequest::SkipServiceWorker skipServiceWorker)
 {
     switch (context) {
     case WebURLRequest::RequestContextAudio:
@@ -108,7 +108,7 @@ bool IsNoCORSAllowedContext(WebURLRequest::RequestContext context, bool skipServ
     case WebURLRequest::RequestContextScript:
         return true;
     case WebURLRequest::RequestContextPlugin:
-        return skipServiceWorker;
+        return skipServiceWorker == WebURLRequest::SkipServiceWorker::All;
     default:
         return false;
     }
@@ -238,10 +238,10 @@ void DocumentThreadableLoader::start(const ResourceRequest& request)
 
     // We assume that ServiceWorker is skipped for sync requests and unsupported
     // protocol requests by content/ code.
-    if (m_async && !request.skipServiceWorker() && SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers(request.url().protocol()) && m_document->fetcher()->isControlledByServiceWorker()) {
+    if (m_async && request.skipServiceWorker() == WebURLRequest::SkipServiceWorker::None && SchemeRegistry::shouldTreatURLSchemeAsAllowingServiceWorkers(request.url().protocol()) && m_document->fetcher()->isControlledByServiceWorker()) {
         if (newRequest.fetchRequestMode() == WebURLRequest::FetchRequestModeCORS || newRequest.fetchRequestMode() == WebURLRequest::FetchRequestModeCORSWithForcedPreflight) {
             m_fallbackRequestForServiceWorker = ResourceRequest(request);
-            m_fallbackRequestForServiceWorker.setSkipServiceWorker(true);
+            m_fallbackRequestForServiceWorker.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::Controlling);

[horo, 2016/06/28 12:10:02]

|m_fallbackRequestForServiceWorker| is used when a CORS request was send to the
SW but respondWith() is not called.
In this case, we should skip all SW.

[Marijn Kruisselbrink, 2016/06/28 17:06:13]

Actually that is precisely the case where I don't want to skip foreign fetch
service workers. CORS requests in general are exactly the requests foreign fetch
wants to intercept, even if made from a controlled page. It's only CORS requests
that involve preflights that for now foreign fetch won't be able to handle; so
the preflights and requests after that are separately set as skipping all
service workers (in CrossOriginAccessControl.cpp for the preflight, and below in
loadActualRequest for the actual request). Requests that don't end up with a
preflight still need to go through foreign fetch.

[horo, 2016/06/29 04:06:14]

Ah got it.
Please write comments about it and that currently the foreign fetch event
fallback response's wasFallbackRequiredByServiceWorker flag is not set
(http://crbug.com/604084).

[Marijn Kruisselbrink, 2016/06/29 17:12:15]

Done

         }
         loadRequest(newRequest, m_resourceLoaderOptions);
         // |this| may be dead here.
@@ -829,7 +829,7 @@ void DocumentThreadableLoader::loadActualRequest()
     // 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(true);
+    actualRequest.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::All);
 
     loadRequest(actualRequest, actualOptions);
     // |this| may be dead here in async mode.