Phase III Step 1: Make ImageResourceContent manage its own ResourceStatus

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 <memory>
 
 #include "core/loader/resource/ImageResource.h"
 #include "core/loader/resource/ImageResourceInfo.h"
@@ skipping 20 matching lines @@
  public:
   NullImageResourceInfo() {}
 
   DEFINE_INLINE_VIRTUAL_TRACE() { ImageResourceInfo::trace(visitor); }
 
  private:
   const KURL& url() const override { return m_url; }
   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 shouldShowPlaceholder() const override { return false; }
   bool isCacheValidator() 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; }
 
@@ skipping 232 matching lines @@
   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;
 }
 
+// |status| is the status of corresponding ImageResource.
+void ImageResourceContent::updateStatus(ResourceStatus status,
+                                        NotifyFinishOption notifyFinishOption) {
+  switch (notifyFinishOption) {
+    case ShouldNotifyFinish:
+      // When |ShouldNotifyFinish|, we set |m_status| to a loaded

[kouhei (in TOK), 2017/03/15 11:18:53]

Can we have switch(status) here to ensure all status is covered? (w/
NOTREACHED() where needed)

[hiroshige, 2017/05/04 20:21:57]

Done.

+      // ResourceStatus.
+      if (status == ResourceStatus::LoadError ||
+          status == ResourceStatus::DecodeError) {
+        // In case of error, Resource's status is set to an error status before
+        // updateImage() and thus we use the error status also as
+        // ImageResourceContent's status.
+        m_status = status;
+      } else {
+        // In case of successful load, Resource's status might not be set to
+        // Cached here. Therefore we set |m_status| to Cached, regardless of
+        // |status|.
+        m_status = ResourceStatus::Cached;
+      }
+      break;
+    case DoNotNotifyFinish:
+      if (m_status == ResourceStatus::NotStarted)

[kouhei (in TOK), 2017/03/15 11:18:53]

Ditto

[yhirano, 2017/03/17 13:47:25]

Can you tell me why this is needed? Setting the status to Pending is basically
done by notifyStartLoad(), right?

[yhirano, 2017/03/17 13:47:25]

Is |status| arbitrary here? Can we put some DCHECKs?

[hiroshige, 2017/05/04 20:21:57]

Right. I removed the transition in DoNotNotifyFinish.

[hiroshige, 2017/05/04 20:21:57]

|status| shouldn't be kNotStarted.

Also, |status| should be kPending, except for one callsite of UpdateImage() in
ReloadIfLoFiOrPlaceholderImage().
We might consider adding DCHECK_EQ(status, kPending) if it is not called from
ReloadIfLoFiOrPlaceholderImage(), but this complicates the concept (and code) of
state transition a little bit, and I don't have a clear reason to forbid
updating
the image data after the state changed to e.g. kCached.

Therefore, I only added a CHECK() for kNotStarted (in UpdateImage()) in the
latest Patch Set.

[hiroshige, 2017/05/04 20:21:57]

Removed.

+        m_status = ResourceStatus::Pending;
+      break;
+  }
+}
+
 ImageResourceContent::UpdateImageResult ImageResourceContent::updateImage(
     PassRefPtr<SharedBuffer> data,
+    ResourceStatus status,
     UpdateImageOption updateImageOption,
     bool allDataReceived) {
   TRACE_EVENT0("blink", "ImageResourceContent::updateImage");
 
 #if DCHECK_IS_ON()
   DCHECK(!m_isUpdateImageBeingCalled);
   AutoReset<bool> scope(&m_isUpdateImageBeingCalled, true);
 #endif
 
   // Clears the existing image, if instructed by |updateImageOption|.
@@ skipping 20 matching lines @@
       if (data) {
         if (!m_image)
           m_image = createImage();
         DCHECK(m_image);
         m_sizeAvailable = m_image->setData(std::move(data), allDataReceived);
       }
 
       // Go ahead and tell our observers to try to draw if we have either
       // received all the data or the size is known. Each chunk from the network
       // causes observers to repaint, which will force that chunk to decode.
-      if (m_sizeAvailable == Image::SizeUnavailable && !allDataReceived)
+      if (m_sizeAvailable == Image::SizeUnavailable && !allDataReceived) {
+        updateStatus(status, DoNotNotifyFinish);
         return UpdateImageResult::NoDecodeError;
+      }
 
       if (m_info->shouldShowPlaceholder() && allDataReceived) {
         if (m_image && !m_image->isNull()) {
           IntSize dimensions = m_image->size();
           clearImage();
           m_image = PlaceholderImage::create(this, dimensions);
         }
       }
 
       if (!m_image || m_image->isNull()) {
         clearImage();
+        updateStatus(status, DoNotNotifyFinish);
         return UpdateImageResult::ShouldDecodeError;
       }
       break;
   }
 
   // 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);
+
+  NotifyFinishOption notifyFinishOption =
+      allDataReceived ? ShouldNotifyFinish : DoNotNotifyFinish;
+  updateStatus(status, notifyFinishOption);
+  notifyObservers(notifyFinishOption);
   return UpdateImageResult::NoDecodeError;
 }
 
+void ImageResourceContent::notifyStartLoad() {

[yhirano, 2017/03/17 13:47:25]

DCHECK_EQ(m_status, NotStarted)?

[hiroshige, 2017/05/04 20:21:57]

Not necessarily NotStarted, e.g. in the case of revalidation.

+  m_status = ResourceStatus::Pending;

[kouhei (in TOK), 2017/03/15 11:18:53]

Can we always update status via updateStatus?

[hiroshige, 2017/05/04 20:21:57]

I removed UpdateStatus(kDoNotNotifyFinish) case and made NotifyStartLoad() here
the
single point of the state transision to kPending.

+}
+
 void ImageResourceContent::decodedSizeChangedTo(const blink::Image* image,
                                                 size_t newSize) {
   if (!image || image != m_image)
     return;
 
   m_info->setDecodedSize(newSize);
 }
 
 bool ImageResourceContent::shouldPauseAnimation(const blink::Image* image) {
   if (!image || image != m_image)
@@ skipping 72 matching lines @@
 bool ImageResourceContent::errorOccurred() const {
   return getStatus() == ResourceStatus::LoadError ||
          getStatus() == ResourceStatus::DecodeError;
 }
 
 bool ImageResourceContent::loadFailedOrCanceled() const {
   return getStatus() == ResourceStatus::LoadError;
 }
 
 ResourceStatus ImageResourceContent::getStatus() const {
-  return m_info->getStatus();
+  return m_status;
 }
 
 const KURL& ImageResourceContent::url() const {
   return m_info->url();
 }
 
 bool ImageResourceContent::hasCacheControlNoStoreHeader() const {
   return m_info->hasCacheControlNoStoreHeader();
 }
 
 float ImageResourceContent::devicePixelRatioHeaderValue() const {
   return m_info->devicePixelRatioHeaderValue();
 }
 
 bool ImageResourceContent::hasDevicePixelRatioHeaderValue() const {
   return m_info->hasDevicePixelRatioHeaderValue();
 }
 
 const ResourceResponse& ImageResourceContent::response() const {
   return m_info->response();
 }
 
 const ResourceError& ImageResourceContent::resourceError() const {
   return m_info->resourceError();
 }
 
 }  // namespace blink