Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to ResourceFetcher, and changes the ordering of some steps to better match other callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=

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

third_party/WebKit/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-expected.txt

 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi, main document URL http://127.0.0.1:8000/security/XFrameOptions/x-frame-options-deny.html, http method GET> redirectResponse (null)
-CONSOLE ERROR: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi, http status code 200>
+CONSOLE ERROR: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi - didFinishLoading
 CONSOLE MESSAGE: line 14: PASS: Access to contentWindow.location.href threw an exception.
 There should be no content in the iframe below

third_party/WebKit/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-invalid-expected.txt

 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi, main document URL http://127.0.0.1:8000/security/XFrameOptions/x-frame-options-invalid.html, http method GET> redirectResponse (null)
-CONSOLE ERROR: Invalid 'X-Frame-Options' header encountered when loading 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi': 'INVALID INVALID INVALID' is not a recognized directive. The header will be ignored.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi, http status code 200>
+CONSOLE ERROR: Invalid 'X-Frame-Options' header encountered when loading 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi': 'INVALID INVALID INVALID' is not a recognized directive. The header will be ignored.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-invalid.cgi - didFinishLoading
 The frame below should load, and a console message should be generated that notes the invalid header.
 

third_party/WebKit/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict-expected.txt

 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi, main document URL http://127.0.0.1:8000/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi, http status code 200>
 CONSOLE ERROR: Multiple 'X-Frame-Options' headers with conflicting values ('ALLOWALL, DENY') encountered when loading 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi'. Falling back to 'DENY'.
 CONSOLE ERROR: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi' in a frame because it set 'X-Frame-Options' to 'ALLOWALL, DENY'.
-http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi, http status code 200>
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi - didFinishLoading
 The frame below should not load, and a console message should be generated that notes the invalid header.
 

third_party/WebKit/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-none-expected.txt

 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi, main document URL http://127.0.0.1:8000/security/XFrameOptions/x-frame-options-none.html, http method GET> redirectResponse (null)
-CONSOLE ERROR: Invalid 'X-Frame-Options' header encountered when loading 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi': '' is not a recognized directive. The header will be ignored.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi, http status code 200>
+CONSOLE ERROR: Invalid 'X-Frame-Options' header encountered when loading 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi': '' is not a recognized directive. The header will be ignored.
 http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-none.cgi - didFinishLoading
 The frame below should load, and a console message should be generated that notes the invalid header.
 

third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp

@@ -239,25 +239,28 @@ Resource* ResourceFetcher::cachedResource(const KURL& resourceURL) const
     return resource.get();
 }
 
-bool ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin* sourceOrigin, const KURL& url) const
+bool ResourceFetcher::canAccessResponse(Resource* resource, const ResourceResponse& response) const
 {
     // Redirects can change the response URL different from one of request.
     bool forPreload = resource->isUnusedPreload();
-    if (!context().canRequest(resource->getType(), resource->resourceRequest(), url, resource->options(), forPreload, FetchRequest::UseDefaultOriginRestrictionForType))
+    if (!context().canRequest(resource->getType(), resource->resourceRequest(), response.url(), resource->options(), forPreload, FetchRequest::UseDefaultOriginRestrictionForType))
         return false;
 
+    SecurityOrigin* sourceOrigin = resource->options().securityOrigin.get();
     if (!sourceOrigin)
         sourceOrigin = context().getSecurityOrigin();
 
-    if (sourceOrigin->canRequestNoSuborigin(url))
+    if (sourceOrigin->canRequestNoSuborigin(response.url()))
         return true;
 
+    // Use the original response instead of the 304 response for a successful revaldiation.
+    const ResourceResponse& responseForAccessControl = (resource->isCacheValidator() && response.httpStatusCode() == 304) ? resource->response() : response;
     String errorDescription;
-    if (!resource->passesAccessControlCheck(sourceOrigin, errorDescription)) {
+    if (!passesAccessControlCheck(responseForAccessControl, resource->options().allowCredentials, sourceOrigin, errorDescription, resource->lastResourceRequest().requestContext())) {
         resource->setCORSFailed();
         if (!forPreload) {
             String resourceType = Resource::resourceTypeToString(resource->getType(), resource->options().initiatorInfo);
-            context().addConsoleMessage(resourceType + " from origin '" + SecurityOrigin::create(url)->toString() + "' has been blocked from loading by Cross-Origin Resource Sharing policy: " + errorDescription);
+            context().addConsoleMessage(resourceType + " from origin '" + SecurityOrigin::create(response.url())->toString() + "' has been blocked from loading by Cross-Origin Resource Sharing policy: " + errorDescription);
         }
         return false;
     }
@@ -945,21 +948,44 @@ void ResourceFetcher::didFailLoading(Resource* resource, const ResourceError& er
     context().didLoadResource(resource);
 }
 
-void ResourceFetcher::didReceiveResponse(Resource* resource, const ResourceResponse& response)
+void ResourceFetcher::didReceiveResponse(Resource* resource, const ResourceResponse& response, WebDataConsumerHandle* rawHandle)
 {
-    // If the response is fetched via ServiceWorker, the original URL of the response could be different from the URL of the request.
-    // We check the URL not to load the resources which are forbidden by the page CSP.
-    // https://w3c.github.io/webappsec-csp/#should-block-response
+    // |rawHandle|'s ownership is transferred to the callee.
+    std::unique_ptr<WebDataConsumerHandle> handle = wrapUnique(rawHandle);
+
     if (response.wasFetchedViaServiceWorker()) {
+        if (resource->options().corsEnabled == IsCORSEnabled && response.wasFallbackRequiredByServiceWorker()) {
+            ResourceRequest request = resource->lastResourceRequest();
+            DCHECK_EQ(request.skipServiceWorker(), WebURLRequest::SkipServiceWorker::None);
+            // This code handles the case when a regular controlling service worker
+            // doesn't handle a cross origin request. When this happens we still
+            // want to give foreign fetch a chance to handle the request, so
+            // only skip the controlling service worker for the fallback request.
+            // This is currently safe because of http://crbug.com/604084 the
+            // wasFallbackRequiredByServiceWorker flag is never set when foreign fetch
+            // handled a request.
+            request.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::Controlling);
+            resource->loader()->restartForServiceWorkerFallback(request);
+            return;
+        }
+
+        // If the response is fetched via ServiceWorker, the original URL of the response could be different from the URL of the request.
+        // We check the URL not to load the resources which are forbidden by the page CSP.
+        // https://w3c.github.io/webappsec-csp/#should-block-response
         const KURL& originalURL = response.originalURLViaServiceWorker();
         if (!originalURL.isEmpty() && !context().allowResponse(resource->getType(), resource->resourceRequest(), originalURL, resource->options())) {
-            resource->loader()->cancel();
-            bool isInternalRequest = resource->options().initiatorInfo.name == FetchInitiatorTypeNames::internal;
-            context().dispatchDidFail(resource->identifier(), ResourceError(errorDomainBlinkInternal, 0, originalURL.getString(), "Unsafe attempt to load URL " + originalURL.elidedString() + " fetched by a ServiceWorker."), isInternalRequest);
+            resource->loader()->didFail(nullptr, ResourceError::cancelledDueToAccessCheckError(originalURL));
             return;
         }
+    } else if (resource->options().corsEnabled == IsCORSEnabled && !canAccessResponse(resource, response)) {
+        resource->loader()->didFail(nullptr, ResourceError::cancelledDueToAccessCheckError(response.url()));
+        return;
     }
+
     context().dispatchDidReceiveResponse(resource->identifier(), response, resource->resourceRequest().frameType(), resource->resourceRequest().requestContext(), resource);
+    resource->responseReceived(response, std::move(handle));
+    if (resource->loader() && response.httpStatusCode() >= 400 && !resource->shouldIgnoreHTTPStatusCodeErrors())
+        resource->loader()->didFail(nullptr, ResourceError::cancelledError(response.url()));
 }
 
 void ResourceFetcher::didReceiveData(const Resource* resource, const char* data, int dataLength, int encodedDataLength)

third_party/WebKit/Source/core/fetch/ResourceFetcher.h

@@ -114,12 +114,10 @@ public:
     };
     void didFinishLoading(Resource*, double finishTime, int64_t encodedDataLength, DidFinishLoadingReason);
     void didFailLoading(Resource*, const ResourceError&);
-    void didReceiveResponse(Resource*, const ResourceResponse&);
+    void didReceiveResponse(Resource*, const ResourceResponse&, WebDataConsumerHandle*);
     void didReceiveData(const Resource*, const char* data, int dataLength, int encodedDataLength);
     void didDownloadData(const Resource*, int dataLength, int encodedDataLength);
     bool defersLoading() const;
-
-    bool canAccessResource(Resource*, SecurityOrigin*, const KURL&) const;
     bool isControlledByServiceWorker() const;
 
     void acceptDataFromThreadedReceiver(unsigned long identifier, const char* data, int dataLength, int encodedDataLength);
@@ -166,6 +164,7 @@ private:
 
     void initializeResourceRequest(ResourceRequest&, Resource::Type, FetchRequest::DeferOption);
     void willSendRequest(unsigned long identifier, ResourceRequest&, const ResourceResponse&, const ResourceLoaderOptions&);
+    bool canAccessResponse(Resource*, const ResourceResponse&) const;
 
     bool resourceNeedsLoad(Resource*, const FetchRequest&, RevalidationPolicy);
     bool shouldDeferImageLoad(const KURL&) const;

third_party/WebKit/Source/core/fetch/ResourceLoader.cpp

@@ -93,6 +93,14 @@ void ResourceLoader::start(const ResourceRequest& request, WebTaskRunner* loadin
         m_loader->loadAsynchronously(WrappedResourceRequest(request), this);
 }
 
+void ResourceLoader::restartForServiceWorkerFallback(const ResourceRequest& request)
+{
+    m_loader.reset();
+    m_loader = wrapUnique(Platform::current()->createURLLoader());
+    DCHECK(m_loader);
+    m_loader->loadAsynchronously(WrappedResourceRequest(request), this);
+}
+
 void ResourceLoader::setDefersLoading(bool defers)
 {
     ASSERT(m_loader);
@@ -144,60 +152,10 @@ void ResourceLoader::didSendData(WebURLLoader*, unsigned long long bytesSent, un
     m_resource->didSendData(bytesSent, totalBytesToBeSent);
 }
 
-bool ResourceLoader::responseNeedsAccessControlCheck() const
-{
-    // If the fetch was (potentially) CORS enabled, an access control check of the response is required.
-    return m_resource->options().corsEnabled == IsCORSEnabled;
-}
-
-void ResourceLoader::didReceiveResponse(WebURLLoader*, const WebURLResponse& response, WebDataConsumerHandle* rawHandle)
+void ResourceLoader::didReceiveResponse(WebURLLoader*, const WebURLResponse& response, WebDataConsumerHandle* handle)
 {
     DCHECK(!response.isNull());
-    // |rawHandle|'s ownership is transferred to the callee.
-    std::unique_ptr<WebDataConsumerHandle> handle = wrapUnique(rawHandle);
-    const ResourceResponse& resourceResponse = response.toResourceResponse();
-
-    if (responseNeedsAccessControlCheck()) {
-        if (response.wasFetchedViaServiceWorker()) {
-            if (response.wasFallbackRequiredByServiceWorker()) {
-                m_loader.reset();
-                m_loader = wrapUnique(Platform::current()->createURLLoader());
-                DCHECK(m_loader);
-                ResourceRequest request = m_resource->lastResourceRequest();
-                DCHECK_EQ(request.skipServiceWorker(), WebURLRequest::SkipServiceWorker::None);
-                // This code handles the case when a regular controlling service worker
-                // doesn't handle a cross origin request. When this happens we still
-                // want to give foreign fetch a chance to handle the request, so
-                // only skip the controlling service worker for the fallback request.
-                // This is currently safe because of http://crbug.com/604084 the
-                // wasFallbackRequiredByServiceWorker flag is never set when foreign fetch
-                // handled a request.
-                request.setSkipServiceWorker(WebURLRequest::SkipServiceWorker::Controlling);
-                m_loader->loadAsynchronously(WrappedResourceRequest(request), this);
-                return;
-            }
-        } else {
-            if (!m_resource->isCacheValidator() || resourceResponse.httpStatusCode() != 304)
-                m_resource->setResponse(resourceResponse);
-            if (!m_fetcher->canAccessResource(m_resource.get(), m_resource->options().securityOrigin.get(), response.url())) {
-                m_fetcher->didReceiveResponse(m_resource.get(), resourceResponse);
-                didFail(nullptr, ResourceError::cancelledDueToAccessCheckError(KURL(response.url())));
-                return;
-            }
-        }
-    }
-
-    m_resource->responseReceived(resourceResponse, std::move(handle));
-    if (!m_loader)
-        return;
-
-    m_fetcher->didReceiveResponse(m_resource.get(), resourceResponse);
-    if (!m_loader)
-        return;
-
-    if (m_resource->response().httpStatusCode() < 400 || m_resource->shouldIgnoreHTTPStatusCodeErrors())
-        return;
-    didFail(nullptr, ResourceError::cancelledError(resourceResponse.url()));
+    m_fetcher->didReceiveResponse(m_resource.get(), response.toResourceResponse(), handle);
 }
 
 void ResourceLoader::didReceiveResponse(WebURLLoader* loader, const WebURLResponse& response)

third_party/WebKit/Source/core/fetch/ResourceLoader.h

@@ -50,6 +50,7 @@ public:
     DECLARE_TRACE();
 
     void start(const ResourceRequest&, WebTaskRunner* loadingTaskRunner, bool defersLoading);
+    void restartForServiceWorkerFallback(const ResourceRequest&);
     void cancel();
 
     void setDefersLoading(bool);
@@ -86,8 +87,6 @@ private:
 
     void requestSynchronously(const ResourceRequest&);
 
-    bool responseNeedsAccessControlCheck() const;
-
     std::unique_ptr<WebURLLoader> m_loader;
     Member<ResourceFetcher> m_fetcher;
     Member<Resource> m_resource;