Remove duplication of encoded image data

third_party/WebKit/Source/core/fetch/ImageResource.cpp

Index: third_party/WebKit/Source/core/fetch/ImageResource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.cpp b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
index 6ebd8315d925a5caf55530bf8627cac0c344b0cc..5f71f11bc721a499751e7f0c6443285ac3d2be6d 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -123,11 +123,6 @@ void ImageResource::addObserver(ImageResourceObserver* observer)
     if (isCacheValidator())
         return;
 
-    if (m_data && !m_image && !errorOccurred()) {
-        createImage();
-        m_image->setData(m_data, true);
-    }
-

[hajimehoshi, 2016/06/10 13:30:28]

I think it is safe to remove since this was introduced with purgable buffer, and
now an Image holds the encode image data just temporarily and can generate
encoded image data from SkRWBuffer on the fly.

[hiroshige, 2016/06/15 13:27:01]

So can we add
|CHECK(!m_data || m_image)|
here?

[hajimehoshi, 2016/06/17 09:25:15]

Done.

     if (m_image && !m_image->isNull()) {
         observer->imageChanged(this);
     }
@@ -391,7 +386,7 @@ void ImageResource::finish(double loadFinishTime)
         if (m_data)
             updateImageAndClearBuffer();
     } else {
-        updateImage(true);
+        updateImageAndClearBuffer();

[f(malita), 2016/06/10 14:26:43]

My understanding of the current impl is that image resources (resources in
general) are cached in discardable memory for reuse.

What are the caching/data sharing implications of this change for

a) multiple image instances with the same source

b) same-source images with non-overlapping lifespans - say across page reload or
navigation.

If I understand this CL correctly, we're now moving the data to the image
decoder at the expense of the resource cache.

[hajimehoshi, 2016/06/13 09:10:47]

I think this doesn't happen with this CL.
ImageResource has an blink::Image as a RefPtr and the Image should be owned by
another object in this case.
Did you mean the resource cache is ResourceFetcher::cachedResource
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/fetch/Res...
?

I think this CL doesn't affect the cache resource management. So, there is no
performance regression.

     }
     Resource::finish(loadFinishTime);
 }
@@ -500,10 +495,9 @@ void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher)
         return;
     m_resourceRequest.setCachePolicy(WebCachePolicy::BypassingCache);
     m_resourceRequest.setLoFiState(WebURLRequest::LoFiOff);
-    if (isLoading())
+    if (isLoading()) {
         m_loader->cancel();
-    else
-        updateImageAndClearBuffer();
+    }
     setStatus(NotStarted);
     fetcher->startLoad(this);
 }