Refactoring starting a resource load

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 4e1613a4a65ffb4344db104bd9657fa5be36555b..393518de6d39ac85e8206be733d5b9601f5a9875 100644
--- a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
@@ -204,14 +204,6 @@ ResourceFetcher::~ResourceFetcher()
 {
 }
 
-WebTaskRunner* ResourceFetcher::loadingTaskRunner()
-{
-    if (!m_context)
-        return nullptr;
-
-    return m_context->loadingTaskRunner();
-}
-
 Resource* ResourceFetcher::cachedResource(const KURL& resourceURL) const
 {
     KURL url = MemoryCache::removeFragmentIdentifierIfNeeded(resourceURL);
@@ -263,10 +255,10 @@ bool ResourceFetcher::resourceNeedsLoad(Resource* resource, const FetchRequest&
 // http://crbug.com/52411
 static const int kMaxValidatedURLsSize = 10000;
 
-void ResourceFetcher::requestLoadStarted(Resource* resource, const FetchRequest& request, ResourceLoadStartType type, bool isStaticData)
+void ResourceFetcher::requestLoadStarted(unsigned long identifier, Resource* resource, const FetchRequest& request, ResourceLoadStartType type, bool isStaticData)
 {
     if (type == ResourceLoadingFromCache && resource->getStatus() == Resource::Cached && !m_validatedURLs.contains(resource->url()))
-        context().dispatchDidLoadResourceFromMemoryCache(resource, request.resourceRequest().frameType(), request.resourceRequest().requestContext());
+        context().dispatchDidLoadResourceFromMemoryCache(identifier, resource, request.resourceRequest().frameType(), request.resourceRequest().requestContext());
 
     if (isStaticData)
         return;
@@ -397,16 +389,20 @@ Resource* ResourceFetcher::requestResource(FetchRequest& request, const Resource
     context().addClientHintsIfNecessary(request);
     context().addCSPHeaderIfNecessary(factory.type(), request);
 
-    KURL url = request.resourceRequest().url();
-    KURL urlWithoutFragment = MemoryCache::removeFragmentIdentifierIfNeeded(url);
-    TRACE_EVENT1("blink", "ResourceFetcher::requestResource", "url", urlForTraceEvent(url));
+    TRACE_EVENT1("blink", "ResourceFetcher::requestResource", "url", urlForTraceEvent(request.url()));
 
-    WTF_LOG(ResourceLoading, "ResourceFetcher::requestResource '%s', charset '%s', priority=%d, forPreload=%u, type=%s", url.elidedString().latin1().data(), request.charset().latin1().data(), request.priority(), request.forPreload(), Resource::resourceTypeName(factory.type()));
+    if (!request.url().isValid())
+        return nullptr;
 
-    if (!url.isValid())
+    if (!context().canRequest(factory.type(), request.resourceRequest(), MemoryCache::removeFragmentIdentifierIfNeeded(request.url()), request.options(), request.forPreload(), request.getOriginRestriction()))
         return nullptr;
 
-    if (!context().canRequest(factory.type(), request.resourceRequest(), urlWithoutFragment, request.options(), request.forPreload(), request.getOriginRestriction()))
+    unsigned long identifier = createUniqueIdentifier();
+    request.setPriority(loadPriority(factory.type(), request, ResourcePriority::NotVisible));
+    request.mutableResourceRequest().setPriority(request.priority());
+    initializeResourceRequest(request.mutableResourceRequest(), factory.type(), request.defer());
+    context().willStartLoadingResource(identifier, request.mutableResourceRequest(), factory.type());
+    if (!request.url().isValid())
         return nullptr;
 
     if (!request.forPreload()) {
@@ -419,7 +415,7 @@ Resource* ResourceFetcher::requestResource(FetchRequest& request, const Resource
         if (activityLogger) {
             Vector<String> argv;
             argv.append(Resource::resourceTypeToString(factory.type(), request.options().initiatorInfo));
-            argv.append(url);
+            argv.append(request.url());
             activityLogger->logEvent("blinkRequestResource", argv.size(), argv.data());
         }
     }
@@ -429,7 +425,7 @@ Resource* ResourceFetcher::requestResource(FetchRequest& request, const Resource
     if (isStaticData)
         resource = resourceForStaticData(request, factory, substituteData);
     if (!resource)
-        resource = memoryCache()->resourceForURL(url, getCacheIdentifier());
+        resource = memoryCache()->resourceForURL(request.url(), getCacheIdentifier());
 
     // See if we can use an existing resource from the cache. If so, we need to move it to be load blocking.
     moveCachedNonBlockingResourceToBlocking(resource, request);
@@ -438,7 +434,9 @@ Resource* ResourceFetcher::requestResource(FetchRequest& request, const Resource
 
     updateMemoryCacheStats(resource, policy, request, factory, isStaticData);
 
-    initializeResourceRequest(request.mutableResourceRequest(), factory.type(), request.defer());
+    if (policy != Use)
+        willSendRequest(identifier, request.mutableResourceRequest(), ResourceResponse(), request.options());
+
     switch (policy) {
     case Reload:
         memoryCache()->remove(resource);
@@ -465,34 +463,28 @@ Resource* ResourceFetcher::requestResource(FetchRequest& request, const Resource
         m_deadStatsRecorder.update(policy);
 
     if (policy != Use)
-        resource->setIdentifier(createUniqueIdentifier());
+        resource->setIdentifier(identifier);
 
     if (!request.forPreload() || policy != Use) {
-        ResourceLoadPriority priority = loadPriority(factory.type(), request, ResourcePriority::NotVisible);
         // When issuing another request for a resource that is already in-flight make
         // sure to not demote the priority of the in-flight request. If the new request
         // isn't at the same priority as the in-flight request, only allow promotions.
         // This can happen when a visible image's priority is increased and then another
         // reference to the image is parsed (which would be at a lower priority).
-        if (priority > resource->resourceRequest().priority())
-            resource->didChangePriority(priority, 0);
+        if (request.priority() > resource->resourceRequest().priority())
+            resource->didChangePriority(request.priority(), 0);
     }
 
     // If only the fragment identifiers differ, it is the same resource.
-    ASSERT(equalIgnoringFragmentIdentifier(resource->url(), url));
-    requestLoadStarted(resource, request, policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork, isStaticData);
-    m_documentResources.set(urlWithoutFragment, resource);
+    DCHECK(equalIgnoringFragmentIdentifier(resource->url(), request.url()));
+    requestLoadStarted(identifier, resource, request, policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork, isStaticData);
+    m_documentResources.set(MemoryCache::removeFragmentIdentifierIfNeeded(request.url()), resource);
 
     if (!resourceNeedsLoad(resource, request, policy))
         return resource;
 
-    if (!context().shouldLoadNewResource(factory.type())) {
-        if (memoryCache()->contains(resource))
-            memoryCache()->remove(resource);
+    if (!startLoad(resource))
         return nullptr;
-    }
-
-    resource->load(this);
     ASSERT(!resource->errorOccurred() || request.options().synchronousPolicy == RequestSynchronously);
     return resource;
 }
@@ -529,7 +521,7 @@ void ResourceFetcher::initializeResourceRequest(ResourceRequest& request, Resour
     context().addAdditionalRequestHeaders(request, (type == Resource::MainResource) ? FetchMainResource : FetchSubresource);
 }
 
-void ResourceFetcher::initializeRevalidation(const FetchRequest& request, Resource* resource)
+void ResourceFetcher::initializeRevalidation(FetchRequest& request, Resource* resource)
 {
     ASSERT(resource);
     ASSERT(memoryCache()->contains(resource));
@@ -538,7 +530,7 @@ void ResourceFetcher::initializeRevalidation(const FetchRequest& request, Resour
     ASSERT(!resource->isCacheValidator());
     ASSERT(!context().isControlledByServiceWorker());
 
-    ResourceRequest revalidatingRequest(resource->resourceRequest());
+    ResourceRequest& revalidatingRequest = request.mutableResourceRequest();
     revalidatingRequest.clearHTTPReferrer();
     initializeResourceRequest(revalidatingRequest, resource->getType(), request.defer());
 
@@ -688,9 +680,6 @@ ResourceFetcher::RevalidationPolicy ResourceFetcher::determineRevalidationPolicy
     if (m_allowStaleResources)
         return Use;
 
-    if (request.getCachePolicy() == WebCachePolicy::BypassingCache)
-        return Reload;
-
     if (!fetchRequest.options().canReuseRequest(existingResource->options()))
         return Reload;
 
@@ -732,6 +721,9 @@ ResourceFetcher::RevalidationPolicy ResourceFetcher::determineRevalidationPolicy
             return Use;
     }
 
+    if (request.getCachePolicy() == WebCachePolicy::BypassingCache)

[Nate Chapin, 2016/05/18 23:56:05]

Turns out initializeResourceRequest() is called too late on trunk, and the cache
policy doesn't fallback to FetchContext::resourceRequestCachePolicy() until
*after* we call determineRevalidationPolicy(). Moving
initializeResourceRequest() to its proper earlier time means that this clause
needs to move to a lower priority: it was blocking even a preload from being
reused, which is not good. This makes it equivalent priority to
CachePolicyReload, which is a very similar concept.

+        return Reload;
+
     // CachePolicyReload always reloads
     if (cachePolicy == CachePolicyReload) {
         WTF_LOG(ResourceLoading, "ResourceFetcher::determineRevalidationPolicy reloading due to CachePolicyReload.");
@@ -810,7 +802,7 @@ void ResourceFetcher::reloadImagesIfNotDeferred()
 {
     for (Resource* resource : m_documentResources.values()) {
         if (resource->getType() == Resource::Image && resource->stillNeedsLoad() && !shouldDeferImageLoad(resource->url()))
-            const_cast<Resource*>(resource)->load(this);
+            startLoad(resource);
     }
 }
 
@@ -952,8 +944,15 @@ void ResourceFetcher::moveResourceLoaderToNonBlocking(ResourceLoader* loader)
     m_loaders->remove(loader);
 }
 
-void ResourceFetcher::willStartLoadingResource(Resource* resource, ResourceLoader* loader, ResourceRequest& request)
+bool ResourceFetcher::startLoad(Resource* resource)
 {
+    DCHECK(resource && resource->stillNeedsLoad());
+    if (!context().shouldLoadNewResource(resource->getType())) {
+        memoryCache()->remove(resource);
+        return false;
+    }
+
+    ResourceLoader* loader = ResourceLoader::create(this, resource);
     if (resource->shouldBlockLoadEvent()) {
         if (!m_loaders)
             m_loaders = ResourceLoaderSet::create();
@@ -964,10 +963,10 @@ void ResourceFetcher::willStartLoadingResource(Resource* resource, ResourceLoade
         m_nonBlockingLoaders->add(loader);
     }
 
-    context().willStartLoadingResource(resource, request);
     storeResourceTimingInitiatorInformation(resource);
-
-    context().dispatchWillSendRequest(resource->identifier(), request, ResourceResponse(), resource->options().initiatorInfo);
+    resource->setFetcherSecurityOrigin(context().getSecurityOrigin());
+    loader->start(resource->requestForLoadStart(), context().loadingTaskRunner(), context().defersLoading());
+    return true;
 }
 
 void ResourceFetcher::removeResourceLoader(ResourceLoader* loader)
@@ -1036,10 +1035,16 @@ bool ResourceFetcher::willFollowRedirect(Resource* resource, ResourceRequest& ne
     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);
+    willSendRequest(resource->identifier(), newRequest, redirectResponse, resource->options());
     return true;
 }
 
+void ResourceFetcher::willSendRequest(unsigned long identifier, ResourceRequest& newRequest, const ResourceResponse& redirectResponse, const ResourceLoaderOptions& options)
+{
+    newRequest.setAllowStoredCredentials(options.allowCredentials == AllowStoredCredentials);
+    context().dispatchWillSendRequest(identifier, newRequest, redirectResponse, options.initiatorInfo);
+}
+
 void ResourceFetcher::updateAllImageResourcePriorities()
 {
     TRACE_EVENT0("blink", "ResourceLoadPriorityOptimizer::updateAllImageResourcePriorities");