Phase II Step 3: Reload LoFi/placeholder images via new ImageResource

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 516325610dddc8c5c7192fc3900ccba6e0c11f40..6b81a92ce2939518d11eaaa5ec2c592ff0f0455b 100644
--- a/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp
@@ -782,6 +782,9 @@ ResourceFetcher::determineRevalidationPolicy(Resource::Type type,
   if (!existingResource)
     return Load;
 
+  if (fetchRequest.enforceNewResource())

[Nate Chapin, 2016/12/28 00:14:57]

Adding yet another clause to this function makes me sad. Could ImageResource
override canReuse() and get the same effect?

[hiroshige, 2017/01/25 02:01:49]

ACK, I'll handle this.

[hiroshige, 2017/03/13 21:27:04]

Removed.

[hiroshige, 2017/03/13 21:29:16]

In Patch Set 31, I merged FetchRequest::enforceNewResource() into
placeholderImageRequestType() and put the condition to ImageResource::canReuse()
to avoid a new clause in determineRevalidationPolicy().

However in Patch Set 32, as commented above, I use startLoad() directly and
bypass requestResource() when reloading.
Therefore, this condition is not needed in Patch Set 32 and thus it is simply
removed.

+    return Reload;
+
   // Checks if the resource has an explicit policy about integrity metadata.
   //
   // This is necessary because ScriptResource and CSSStyleSheetResource objects
@@ -1117,8 +1120,6 @@ ResourceTimingInfo* ResourceFetcher::getNavigationTimingInfo() {
 
 void ResourceFetcher::handleLoadCompletion(Resource* resource) {

[Nate Chapin, 2016/12/28 00:14:57]

This helper seems like it's not useful if it's only the
context().didLoadResource() call.

[hiroshige, 2016/12/28 01:04:21]

+tyoshino@ (who introduced this method), is it fine with you to revert
handleLoadCompletion() back to didLoadResource()?

[hiroshige, 2017/01/25 02:01:49]

I reverted this change (while I was removing reload-initiating code from
ImageResourceContent) and thus we no longer have to handle this issue.

   context().didLoadResource(resource);
-
-  resource->reloadIfLoFiOrPlaceholderImage(this, Resource::kReloadIfNeeded);
 }
 
 void ResourceFetcher::handleLoaderFinish(Resource* resource,
@@ -1174,7 +1175,7 @@ void ResourceFetcher::handleLoaderFinish(Resource* resource,
                                      encodedDataLength);
 
   if (type == DidFinishLoading)
-    resource->finish(finishTime);
+    resource->finish(finishTime, this);
 
   handleLoadCompletion(resource);
 }
@@ -1194,7 +1195,7 @@ void ResourceFetcher::handleLoaderError(Resource* resource,
                             resource->response().encodedDataLength(),
                             isInternalRequest);
 
-  resource->error(error);
+  resource->error(error, this);
 
   handleLoadCompletion(resource);
 }
@@ -1309,7 +1310,7 @@ void ResourceFetcher::updateAllImageResourcePriorities() {
 }
 
 void ResourceFetcher::reloadLoFiImages() {
-  for (const auto& documentResource : m_documentResources) {
+  for (auto& documentResource : m_documentResources) {
     Resource* resource = documentResource.value.get();
     if (resource)
       resource->reloadIfLoFiOrPlaceholderImage(this, Resource::kReloadAlways);