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

third_party/WebKit/Source/platform/loader/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
@@ skipping 495 matching lines @@
            ? FetchContext::V8ActivityLoggingPolicy::SuppressLogging
            : FetchContext::V8ActivityLoggingPolicy::Log));
   if (!request.url().isValid())
     return Abort;
 
   resourceRequest.setAllowStoredCredentials(
       request.options().allowCredentials == AllowStoredCredentials);
   return Continue;
 }
 
+void ResourceFetcher::addToDocumentResources(Resource* resource) {
+  m_documentResources.set(
+      MemoryCache::removeFragmentIdentifierIfNeeded(resource->url()), resource);
+}
+
 Resource* ResourceFetcher::requestResource(
     FetchRequest& request,
     const ResourceFactory& factory,
     const SubstituteData& substituteData) {
   unsigned long identifier = createUniqueIdentifier();
   ResourceRequest& resourceRequest = request.mutableResourceRequest();
   network_instrumentation::ScopedResourceLoadTracker scopedResourceLoadTracker(
       identifier, resourceRequest);
   SCOPED_BLINK_UMA_HISTOGRAM_TIMER("Blink.Fetch.RequestResourceTime");
   // TODO(dproy): Remove this. http://crbug.com/659666
@@ skipping 79 matching lines @@
     // TODO(yoav): I'd expect the stated scenario to not go here, as its policy
     // would be Use.
   }
 
   // If only the fragment identifiers differ, it is the same resource.
   DCHECK(equalIgnoringFragmentIdentifier(resource->url(), request.url()));
   requestLoadStarted(
       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|,
   // the resource was already initialized for the revalidation here, but won't
   // start loading.
   if (!resourceNeedsLoad(resource, request, policy))
     return resource;
 
   if (!startLoad(resource))
     return nullptr;
@@ skipping 698 matching lines @@
                                 resourcePriority.intraPriorityValue);
     network_instrumentation::resourcePrioritySet(resource->identifier(),
                                                  resourceLoadPriority);
     context().dispatchDidChangeResourcePriority(
         resource->identifier(), resourceLoadPriority,
         resourcePriority.intraPriorityValue);
   }
 }
 
 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. 

   }
 }
 
 void ResourceFetcher::logPreloadStats(ClearPreloadsPolicy policy) {
   if (!m_preloads)
     return;
   unsigned scripts = 0;
   unsigned scriptMisses = 0;
   unsigned stylesheets = 0;
   unsigned stylesheetMisses = 0;
@@ skipping 197 matching lines @@
   visitor->trace(m_context);
   visitor->trace(m_archive);
   visitor->trace(m_loaders);
   visitor->trace(m_nonBlockingLoaders);
   visitor->trace(m_documentResources);
   visitor->trace(m_preloads);
   visitor->trace(m_resourceTimingInfoMap);
 }
 
 }  // namespace blink