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

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

Index: third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
index a5eae6c5bf861db7dc1fd4028a4489e01506f60b..2373a11a21861dab9b7f323a35d062b6f262a7cc 100644
--- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
+++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -513,6 +513,11 @@ ResourceFetcher::PrepareRequestResult ResourceFetcher::prepareRequest(
   return Continue;
 }
 
+void ResourceFetcher::addToDocumentResources(Resource* resource) {
+  m_documentResources.set(
+      MemoryCache::removeFragmentIdentifierIfNeeded(resource->url()), resource);
+}
+
 Resource* ResourceFetcher::requestResource(
     FetchRequest& request,
     const ResourceFactory& factory,
@@ -612,8 +617,7 @@ Resource* ResourceFetcher::requestResource(
       identifier, resource, request,
       policy == Use ? ResourceLoadingFromCache : ResourceLoadingFromNetwork,
       isStaticData);
-  m_documentResources.set(
-      MemoryCache::removeFragmentIdentifierIfNeeded(request.url()), resource);
+  addToDocumentResources(resource);
 
   // Returns with an existing resource if the resource does not need to start
   // loading immediately. If revalidation policy was determined as |Revalidate|,
@@ -1332,7 +1336,7 @@ void ResourceFetcher::updateAllImageResourcePriorities() {
 }
 
 void ResourceFetcher::reloadLoFiImages() {
-  for (const auto& documentResource : m_documentResources) {
+  for (auto& documentResource : m_documentResources) {

[yhirano, 2017/03/17 13:09:35]

What is the intention of this change?

     Resource* resource = documentResource.value.get();
     if (resource)
       resource->reloadIfLoFiOrPlaceholderImage(this, Resource::kReloadAlways);

[kouhei (in TOK), 2017/03/15 10:25:28]

or should we replace m_documentResources using the return value?
Are we sure that we don't update m_documentResources set while iterating? or is
it safe?

[hiroshige, 2017/03/15 19:03:31]

In fact I am sure that we update |m_documentResources| upon reloading.
(Previously this is not needed because reloading uses the same Resource object,
but now we replace old references to ImageResource from
MemoryCache/m_documentResources with new one)

I think updates to m_documentResources should be done within reloading
(especially when we possibly call ResourceFetcher::requestResource() for
reloading possibly in the future, which updates m_documentResources), so I'll
update the code here to be safe with updating while iterating.