Unify Resource loading status tracking

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

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ skipping 244 matching lines @@
 
 bool ResourceFetcher::isControlledByServiceWorker() const
 {
     return context().isControlledByServiceWorker();
 }
 
 bool ResourceFetcher::resourceNeedsLoad(Resource* resource, const FetchRequest& request, RevalidationPolicy policy)
 {
     if (FetchRequest::DeferredByClient == request.defer())
         return false;
+    if (resource->isImage() && shouldDeferImageLoad(resource->url()))
+        return false;
     return policy != Use || resource->stillNeedsLoad();
 }
 
 // Limit the number of URLs in m_validatedURLs to avoid memory bloat.
 // http://crbug.com/52411
 static const int kMaxValidatedURLsSize = 10000;
 
 void ResourceFetcher::requestLoadStarted(Resource* resource, const FetchRequest& request, ResourceLoadStartType type, bool isStaticData)
 {
     if (type == ResourceLoadingFromCache && resource->getStatus() == Resource::Cached && !m_validatedURLs.contains(resource->url()))
@@ skipping 380 matching lines @@
     // If the same URL has been loaded as a different type, we need to reload.
     if (existingResource->getType() != type) {
         // FIXME: If existingResource is a Preload and the new type is LinkPrefetch
         // We really should discard the new prefetch since the preload has more
         // specific type information! crbug.com/379893
         // fast/dom/HTMLLinkElement/link-and-subresource-test hits this case.
         WTF_LOG(ResourceLoading, "ResourceFetcher::determineRevalidationPolicy reloading due to type mismatch.");
         return Reload;
     }
 
-    // Do not load from cache if images are not enabled. The load for this image will be blocked
-    // in ImageResource::load.
+    // Do not load from cache if images are not enabled.
     if (FetchRequest::DeferredByClient == fetchRequest.defer())
         return Reload;
+    if (existingResource->isImage() && !context().allowImage(m_imagesEnabled, existingResource->url()))

[Nate Chapin, 2016/03/18 21:24:06]

shouldDeferImageLoad(), if used here, blocks SVGImages from loading inline data:
images. It'd be nice make it shouldDeferImageLoad() here, too, for consistency.

[hiroshige, 2016/03/18 23:21:20]

Offline discussion memo: can we |m_autoLoadImages| to true only for
ResourceFetcher for SVGs and use shouldDeferImageLoad() here?

[Nate Chapin, 2016/03/21 22:04:01]

https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/La...
fails with this change.

It appears that m_imagesEnabled and m_autoLoadImages are intentionally subtly
different. m_imagesEnabled being false blocks *everything*, while
m_autoLoadImages being false blocks network requests only (but not data: urls or
cache hits). It would be nice if we could unify this behavior, but I don't think
this CL is the place to do it.

[hiroshige, 2016/03/22 18:21:33]

I see.
So, if allowImage() is false,
(1) we reload Resource here, just to make all Resource to be deferred by
shouldDeferImageLoad() in resourceNeedsLoad(), and
(2) we apply this intentionally to data URLs (right?), and thus this if
statement has higher priority than |if(isStaticData)| at Line 679.
Please add comments here to describe these two points.

[Nate Chapin, 2016/03/22 19:51:23]

Done.

+        return Reload;
 
     // Never use cache entries for downloadToFile / useStreamOnResponse
     // requests. The data will be delivered through other paths.
     if (request.downloadToFile() || request.useStreamOnResponse())
         return Reload;
 
     // If resource was populated from a SubstituteData load or data: url, use it.
     if (isStaticData)
         return Use;
 
@@ skipping 130 matching lines @@
         return;
 
     m_imagesEnabled = enable;
 
     if (!m_imagesEnabled)
         return;
 
     reloadImagesIfNotDeferred();
 }
 
-bool ResourceFetcher::clientDefersImage(const KURL& url) const
-{
-    return !context().allowImage(m_imagesEnabled, url);
-}
-
 bool ResourceFetcher::shouldDeferImageLoad(const KURL& url) const
 {
-    return clientDefersImage(url) || !m_autoLoadImages;
+    return !context().allowImage(m_imagesEnabled, url) || !m_autoLoadImages;
 }
 
 void ResourceFetcher::reloadImagesIfNotDeferred()
 {
     // TODO(japhet): Once oilpan ships, the const auto&
     // can be replaced with a Resource*. Also, null checking
     // the resource probably won't be necesssary.
     for (const auto& documentResource : m_documentResources) {
         Resource* resource = documentResource.value.get();
-        if (resource && resource->getType() == Resource::Image && resource->stillNeedsLoad() && !clientDefersImage(resource->url()))
+        if (resource && resource->getType() == Resource::Image && resource->stillNeedsLoad() && !shouldDeferImageLoad(resource->url()))
             const_cast<Resource*>(resource)->load(this, defaultResourceOptions());
     }
 }
 
 void ResourceFetcher::didLoadResource(Resource* resource)
 {
     context().didLoadResource(resource);
 }
 
 int ResourceFetcher::requestCount() const
@@ skipping 61 matching lines @@
 
     if (!m_archive)
         return false;
 
     ArchiveResource* archiveResource = m_archive->subresourceForURL(request.url());
     if (!archiveResource) {
         resource->error(Resource::LoadError);
         return false;
     }
 
-    resource->setLoading(true);
+    resource->setStatus(Resource::Pending);
     resource->responseReceived(archiveResource->response(), nullptr);
     SharedBuffer* data = archiveResource->data();
     if (data)
         resource->appendData(data->data(), data->size());
     resource->finish();
     return true;
 }
 
 void ResourceFetcher::didFinishLoading(Resource* resource, double finishTime, int64_t encodedDataLength)
 {
@@ skipping 290 matching lines @@
     visitor->trace(m_loaders);
     visitor->trace(m_nonBlockingLoaders);
 #if ENABLE(OILPAN)
     visitor->trace(m_documentResources);
     visitor->trace(m_preloads);
     visitor->trace(m_resourceTimingInfoMap);
 #endif
 }
 
 } // namespace blink