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

third_party/WebKit/Source/core/loader/resource/ImageResourceContent.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/loader/resource/ImageResourceContent.h"
 
 #include "core/loader/resource/ImageResource.h"
 #include "core/loader/resource/ImageResourceInfo.h"
 #include "core/loader/resource/ImageResourceObserver.h"
 #include "core/svg/graphics/SVGImage.h"
@@ skipping 16 matching lines @@
       public ImageResourceInfo {
   USING_GARBAGE_COLLECTED_MIXIN(NullImageResourceInfo);
 
  public:
   NullImageResourceInfo() {}
 
   DEFINE_INLINE_VIRTUAL_TRACE() { ImageResourceInfo::trace(visitor); }
 
  private:
   const KURL& url() const override { return m_url; }
-  bool isSchedulingReload() const override { return false; }
   bool hasDevicePixelRatioHeaderValue() const override { return false; }
   float devicePixelRatioHeaderValue() const override { return 1.0; }
   const ResourceResponse& response() const override { return m_response; }
   ResourceStatus getStatus() const override { return ResourceStatus::Cached; }
   bool isPlaceholder() const override { return false; }
   bool isCacheValidator() const override { return false; }
-  bool schedulingReloadOrShouldReloadBrokenPlaceholder() const override {
-    return false;
-  }
   bool isAccessAllowed(
       SecurityOrigin*,
       DoesCurrentFrameHaveSingleSecurityOrigin) const override {
     return true;
   }
   bool hasCacheControlNoStoreHeader() const override { return false; }
   const ResourceError& resourceError() const override { return m_error; }
+  const ImageResource* resourceForTest() const override { return nullptr; }
 
   void decodeError(bool allDataReceived) override {}
+  bool reloadIfLoFiOrPlaceholderIfNeeded(
+      ResourceFetcher* fetcherForReload) override {
+    return false;
+  }
   void setDecodedSize(size_t) override {}
   void willAddClientOrObserver() override {}
   void didRemoveClientOrObserver() override {}
   void emulateLoadStartedForInspector(
       ResourceFetcher*,
       const KURL&,
       const AtomicString& initiatorName) override {}
 
   const KURL m_url;
   const ResourceResponse m_response;
@@ skipping 42 matching lines @@
 
   m_observers.add(observer);
 
   if (m_info->isCacheValidator())
     return;
 
   if (m_image && !m_image->isNull()) {
     observer->imageChanged(this);
   }
 
-  if (isLoaded() && m_observers.contains(observer) &&
-      !m_info->schedulingReloadOrShouldReloadBrokenPlaceholder()) {
+  if (isLoaded() && m_observers.contains(observer)) {
     markObserverFinished(observer);
     observer->imageNotifyFinished(this);
   }
 }
 
 void ImageResourceContent::removeObserver(ImageResourceObserver* observer) {
   DCHECK(observer);
 
   auto it = m_observers.find(observer);
   if (it != m_observers.end()) {
@@ skipping 132 matching lines @@
     NotifyFinishOption notifyingFinishOption,
     const IntRect* changeRect) {
   for (auto* observer : m_finishedObservers.asVector()) {
     if (m_finishedObservers.contains(observer))
       observer->imageChanged(this, changeRect);
   }
   for (auto* observer : m_observers.asVector()) {
     if (m_observers.contains(observer)) {
       observer->imageChanged(this, changeRect);
       if (notifyingFinishOption == ShouldNotifyFinish &&
-          m_observers.contains(observer) &&
-          !m_info->schedulingReloadOrShouldReloadBrokenPlaceholder()) {
+          m_observers.contains(observer)) {
         markObserverFinished(observer);
         observer->imageNotifyFinished(this);
       }
     }
   }
 }
 
 PassRefPtr<Image> ImageResourceContent::createImage() {
   if (m_info->response().mimeType() == "image/svg+xml")
     return SVGImage::create(this);
   return BitmapImage::create(this);
 }
 
 void ImageResourceContent::clearImage() {
   if (!m_image)
     return;
   int64_t length = m_image->data() ? m_image->data()->size() : 0;
   v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(-length);
 
   // If our Image has an observer, it's always us so we need to clear the back
   // pointer before dropping our reference.
   m_image->clearImageObserver();
   m_image.clear();
   m_sizeAvailable = Image::SizeUnavailable;
 }
 
 void ImageResourceContent::updateImage(PassRefPtr<SharedBuffer> data,
                                        UpdateImageOption updateImageOption,
-                                       bool allDataReceived) {
+                                       bool allDataReceived,
+                                       ResourceFetcher* fetcherForReload) {
   TRACE_EVENT0("blink", "ImageResourceContent::updateImage");
 
   // Clears the existing image, if instructed by |updateImageOption|.
   switch (updateImageOption) {
     case ClearAndUpdateImage:
     case ClearImageOnly:
       clearImage();
       break;
     case UpdateImage:
       break;
@@ skipping 39 matching lines @@
           clearImage();
         }
       }
       if (!m_image || m_image->isNull()) {
         clearImage();
         m_info->decodeError(allDataReceived);
       }
       break;
   }
 
+  if (m_info->reloadIfLoFiOrPlaceholderIfNeeded(fetcherForReload))

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

Is it critical that this happen before notifyObservers? Or in
ImageResourceContent, for that matter? It looks like that wasn't the way it used
to be?

I don't like having to plumb the ResourceFetcher all around to enable it to be
called here if there's a good alternative.

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

This is to keep the current behavior of
"ImageResourceObserver::imageNotifyFinished() is not called when Placeholder is
reloaded due to DecodeError".

See the design doc
https://docs.google.com/document/d/1O-fB83mrE0B_V8gzXNqHgmRLCvstTB4MMi3RnVLr8...
(Phase II, Step 3, "Cleanup reloading around DecodeError") for more details
about timing dependencies around invoking reloading.

In the latest Patch Set, I moved reloading from here (and thus I removed the
plumbing between ImageResource and ImageResourceContent -- I agree this was not
clean) to ImageResource::decodeError().
Still we have the plumbing ResourceLoader::reloadIfLoFiOrPlaceholderImage() just
to get a reference to ResourceFetcher though.

Any better thoughts for handling this timing dependency are welcome.

+    return;
+
   // Notifies the observers.
   // It would be nice to only redraw the decoded band of the image, but with the
   // current design (decoding delayed until painting) that seems hard.
   notifyObservers(allDataReceived ? ShouldNotifyFinish : DoNotNotifyFinish);
 }
 
 void ImageResourceContent::decodedSizeChangedTo(const blink::Image* image,
                                                 size_t newSize) {
   if (!image || image != m_image)
     return;
@@ skipping 104 matching lines @@
   return m_info->hasDevicePixelRatioHeaderValue();
 }
 
 const ResourceResponse& ImageResourceContent::response() const {
   return m_info->response();
 }
 
 const ResourceError& ImageResourceContent::resourceError() const {
   return m_info->resourceError();
 }
+const ImageResource* ImageResourceContent::resourceForTest() const {
+  return m_info->resourceForTest();
+}
 
 }  // namespace blink