Don't duplicate ResourceRequests and ResourceLoaderOptions on ResourceLoader

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

Index: third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
diff --git a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
index e18a9c95abc30e15dbf5eeff60fa8a504fa870d7..f6d32dc7d40d976ac51fd23e6d4ecebf82a63943 100644
--- a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
@@ -767,6 +767,10 @@ ResourceFetcher::RevalidationPolicy ResourceFetcher::determineRevalidationPolicy
             return Use;
     }
 
+    // Don't try to reuse an in-progress async request for a new sync request.
+    if (fetchRequest.options().synchronousPolicy == RequestSynchronously && existingResource->isLoading())

[hiroshige, 2016/03/10 00:24:38]

This doesn't Reload in non-RawResource cases, because Use might be returned at
Line 767.
XSLStyleSheetResource::fetchSynchronously() seems the only case? where
RequestSynchronously is used with XSLStyleSheet.
Shouldn't this case have higher (or highest) priority, because ResourceLoader
otherwise fails by ASSERT()?

[Nate Chapin, 2016/03/10 18:46:01]

Bumped priority up so that the only higher priority Use cases are preloading
(not relevant for XSL stylesheets or raw resources) and static data (where no
ResourceLoader is created).

+        return Reload;
+
     // CachePolicyReload always reloads
     if (cachePolicy == CachePolicyReload) {
         WTF_LOG(ResourceLoading, "ResourceFetcher::determineRevalidationPolicy reloading due to CachePolicyReload.");
@@ -858,13 +862,6 @@ void ResourceFetcher::reloadImagesIfNotDeferred()
     }
 }
 
-void ResourceFetcher::redirectReceived(Resource* resource, const ResourceResponse& redirectResponse)
-{
-    ResourceTimingInfoMap::iterator it = m_resourceTimingInfoMap.find(resource);
-    if (it != m_resourceTimingInfoMap.end())
-        it->value->addRedirect(redirectResponse);
-}
-
 void ResourceFetcher::didLoadResource(Resource* resource)
 {
     context().didLoadResource(resource);
@@ -978,11 +975,6 @@ void ResourceFetcher::didFailLoading(const Resource* resource, const ResourceErr
     context().dispatchDidFail(resource->identifier(), error, isInternalRequest);
 }
 
-void ResourceFetcher::willSendRequest(unsigned long identifier, ResourceRequest& request, const ResourceResponse& redirectResponse, const FetchInitiatorInfo& initiatorInfo)
-{
-    context().dispatchWillSendRequest(identifier, request, redirectResponse, initiatorInfo);
-}
-
 void ResourceFetcher::didReceiveResponse(const Resource* resource, const ResourceResponse& response)
 {
     // If the response is fetched via ServiceWorker, the original URL of the response could be different from the URL of the request.
@@ -1024,7 +1016,7 @@ void ResourceFetcher::subresourceLoaderFinishedLoadingOnePart(ResourceLoader* lo
     didLoadResource(loader->cachedResource());
 }
 
-void ResourceFetcher::didInitializeResourceLoader(ResourceLoader* loader)
+void ResourceFetcher::willStartLoadingResource(Resource* resource, ResourceLoader* loader, ResourceRequest& request)
 {
     if (loader->cachedResource()->shouldBlockLoadEvent()) {
         if (!m_loaders)
@@ -1035,6 +1027,12 @@ void ResourceFetcher::didInitializeResourceLoader(ResourceLoader* loader)
             m_nonBlockingLoaders = ResourceLoaderSet::create();
         m_nonBlockingLoaders->add(loader);
     }
+
+    context().willStartLoadingResource(request);
+    storeResourceTimingInitiatorInformation(resource);
+    TRACE_EVENT_ASYNC_BEGIN2("blink.net", "Resource", resource, "url", resource->url().getString().ascii(), "priority", resource->resourceRequest().priority());
+
+    context().dispatchWillSendRequest(resource->identifier(), request, ResourceResponse(), resource->options().initiatorInfo);
 }
 
 void ResourceFetcher::willTerminateResourceLoader(ResourceLoader* loader)
@@ -1047,13 +1045,6 @@ void ResourceFetcher::willTerminateResourceLoader(ResourceLoader* loader)
         ASSERT_NOT_REACHED();
 }
 
-void ResourceFetcher::willStartLoadingResource(Resource* resource, ResourceRequest& request)
-{
-    context().willStartLoadingResource(request);
-    storeResourceTimingInitiatorInformation(resource);
-    TRACE_EVENT_ASYNC_BEGIN2("blink.net", "Resource", resource, "url", resource->url().getString().ascii(), "priority", resource->resourceRequest().priority());
-}
-
 void ResourceFetcher::stopFetching()
 {
     if (m_nonBlockingLoaders)
@@ -1080,25 +1071,37 @@ bool ResourceFetcher::defersLoading() const
     return context().defersLoading();
 }
 
-bool ResourceFetcher::canAccessRedirect(Resource* resource, ResourceRequest& newRequest, const ResourceResponse& redirectResponse, ResourceLoaderOptions& options)

[Nate Chapin, 2016/03/09 22:35:54]

I'm not sure whether it's better to merge canAccessRedirect() in to
willFollowRedirect() or to leave it as a private helper.

+static bool isManualRedirectFetchRequest(const ResourceRequest& request)
 {
-    if (!context().canRequest(resource->getType(), newRequest, newRequest.url(), options, resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType))
-        return false;
-    if (options.corsEnabled == IsCORSEnabled) {
-        SecurityOrigin* sourceOrigin = options.securityOrigin.get();
-        if (!sourceOrigin)
-            sourceOrigin = context().securityOrigin();
-
-        String errorMessage;
-        StoredCredentials withCredentials = resource->lastResourceRequest().allowStoredCredentials() ? AllowStoredCredentials : DoNotAllowStoredCredentials;
-        if (!CrossOriginAccessControl::handleRedirect(sourceOrigin, newRequest, redirectResponse, withCredentials, options, errorMessage)) {
-            resource->setCORSFailed();
-            context().addConsoleMessage(errorMessage);
+    return request.fetchRedirectMode() == WebURLRequest::FetchRedirectModeManual && request.requestContext() == WebURLRequest::RequestContextFetch;
+}
+
+bool ResourceFetcher::willFollowRedirect(Resource* resource, ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
+{
+    if (!isManualRedirectFetchRequest(resource->resourceRequest())) {
+        if (!context().canRequest(resource->getType(), newRequest, newRequest.url(), resource->options(), resource->isUnusedPreload(), FetchRequest::UseDefaultOriginRestrictionForType))
             return false;
+        if (resource->options().corsEnabled == IsCORSEnabled) {
+            SecurityOrigin* sourceOrigin = resource->options().securityOrigin.get();
+            if (!sourceOrigin)
+                sourceOrigin = context().securityOrigin();
+
+            String errorMessage;
+            StoredCredentials withCredentials = resource->lastResourceRequest().allowStoredCredentials() ? AllowStoredCredentials : DoNotAllowStoredCredentials;
+            if (!CrossOriginAccessControl::handleRedirect(sourceOrigin, newRequest, redirectResponse, withCredentials, resource->mutableOptions(), errorMessage)) {
+                resource->setCORSFailed();
+                context().addConsoleMessage(errorMessage);
+                return false;
+            }
         }
+        if (resource->getType() == Resource::Image && shouldDeferImageLoad(newRequest.url()))
+            return false;
     }
-    if (resource->getType() == Resource::Image && shouldDeferImageLoad(newRequest.url()))
-        return false;
+
+    ResourceTimingInfoMap::iterator it = m_resourceTimingInfoMap.find(resource);
+    if (it != m_resourceTimingInfoMap.end())
+        it->value->addRedirect(redirectResponse);
+    context().dispatchWillSendRequest(resource->identifier(), newRequest, redirectResponse, resource->options().initiatorInfo);
     return true;
 }