Skip the Service Worker CORS fallback for same origin requests. [1/2 blink]

Source/core/loader/DocumentThreadableLoader.cpp

 /*
  * Copyright (C) 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2013, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 434 matching lines @@
             // when the ServiceWorker does not call respondWith().)
             ASSERT(m_fallbackRequestForServiceWorker);
             loadFallbackRequestForServiceWorker();
             return;
         }
         m_fallbackRequestForServiceWorker = nullptr;
         m_client->didReceiveResponse(identifier, response, handle);
         return;
     }
 
-    ASSERT(!m_fallbackRequestForServiceWorker);
+    // Even if the request met the conditions to get handled by a Service Worker
+    // in the constructor of this class (and therefore
+    // |m_fallbackRequestForServiceWorker| is set), the Service Worker may skip
+    // processing the request. Only if the request is same origin, the skipped
+    // response may come here (wasFetchedViaServiceWorker() returns false) since
+    // such a request doesn't have to go through the CORS algorithm by calling
+    // loadFallbackRequestForServiceWorker().
+    // FIXME: We should use |m_sameOriginRequest| when we will support
+    // Suborigins (crbug.com/336894) for Service Worker.
+    ASSERT(!m_fallbackRequestForServiceWorker || securityOrigin()->canRequest(m_fallbackRequestForServiceWorker->url()));

[tyoshino (SeeGerritForStatus), 2015/07/23 05:33:16]

have you investigated carefully that this is consistent with the condition used
in
https://codereview.chromium.org/1242953007/diff/40001/content/browser/service...
?

For example, m_universalAccess which is set when the --disable-web-security flag
is specified.

[horo, 2015/07/23 07:59:44]

I don't think these conditions (in blink and content) must be strictly
consistent.
Whenever "provider_host_->document_url().GetOrigin() ==
request()->url().GetOrigin()" is true,
"securityOrigin()->canRequest(m_fallbackRequestForServiceWorker->url()" must be
true.

[tyoshino (SeeGerritForStatus), 2015/07/23 08:09:42]

Ah, yeah, actually it's enough that condition holds.

How about unique origins?

[horo, 2015/07/23 09:42:00]

The document which origin is an unique origin can't be controlled by Service
Worker.

[tyoshino (SeeGerritForStatus), 2015/07/23 09:50:13]

Thanks. Please write that in the CL description.

+    m_fallbackRequestForServiceWorker = nullptr;
 
     if (!m_sameOriginRequest && m_options.crossOriginRequestPolicy == UseAccessControl) {
         String accessControlErrorDescription;
         if (!passesAccessControlCheck(response, effectiveAllowCredentials(), securityOrigin(), accessControlErrorDescription)) {
             reportResponseReceived(identifier, response);
             m_client->didFailAccessControlCheck(ResourceError(errorDomainBlinkInternal, 0, response.url().string(), accessControlErrorDescription));
             return;
         }
     }
 
@@ skipping 198 matching lines @@
         return DoNotAllowStoredCredentials;
     return m_resourceLoaderOptions.allowCredentials;
 }
 
 SecurityOrigin* DocumentThreadableLoader::securityOrigin() const
 {
     return m_securityOrigin ? m_securityOrigin.get() : m_document.securityOrigin();
 }
 
 } // namespace blink