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 c10b9a9682ccdc81789ec0b463f4832a8b7b1a9c..326c1da02e70d2d1e1bf8ecb274081eaf92b6649 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -124,10 +124,7 @@ void ImageResource::addObserver(ImageResourceObserver* observer)
     if (isCacheValidator())
         return;
 
-    if (m_data && !m_image && !errorOccurred()) {
-        createImage();
-        m_image->setData(m_data, true);
-    }
+    DCHECK(!m_data || m_image);
 
     if (m_image && !m_image->isNull()) {
         observer->imageChanged(this);
@@ -196,12 +193,10 @@ void ImageResource::destroyDecodedDataForFailedRevalidation()
 
 void ImageResource::destroyDecodedDataIfPossible()
 {
-    if (!hasClientsOrObservers() && !isLoading() && (!m_image || (m_image->hasOneRef() && m_image->isBitmapImage()))) {
-        clearImage();
-        setDecodedSize(0);
-    } else if (m_image && !errorOccurred()) {
+    if (!m_image)
+        return;
+    if ((!hasClientsOrObservers() && !isLoading() && m_image->hasOneRef() && m_image->isBitmapImage()) || !errorOccurred())
         m_image->destroyDecodedData();
-    }
 }
 
 void ImageResource::doResetAnimation()
@@ -226,6 +221,16 @@ void ImageResource::allClientsAndObserversRemoved()
     Resource::allClientsAndObserversRemoved();
 }
 
+PassRefPtr<SharedBuffer> ImageResource::resourceBuffer() const
+{
+    RefPtr<SharedBuffer> data = Resource::resourceBuffer();
+    if (data)
+        return data.release();

[haraken, 2016/07/13 02:18:30]

if (m_data)
  return m_data.get();

[hajimehoshi, 2016/07/13 07:36:22]

Done.

+    if (m_image)
+        return m_image->data();
+    return nullptr;
+}
+
 void ImageResource::appendData(const char* data, size_t length)
 {
     if (m_multipartParser) {
@@ -370,8 +375,10 @@ void ImageResource::updateImage(bool allDataReceived)
     // Have the image update its data from its internal buffer.
     // It will not do anything now, but will delay decoding until
     // queried for info (like size or specific image frames).
-    if (m_image)
+    if (m_data) {
+        DCHECK(m_image);
         sizeAvailable = m_image->setData(m_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
@@ -390,13 +397,15 @@ void ImageResource::updateImage(bool allDataReceived)
         // (decoding delayed until painting) that seems hard.
         notifyObservers();
     }
+
+    if (allDataReceived)
+        m_data.clear();

[haraken, 2016/07/13 02:18:30]

Add a comment what this is doing. It would be a good idea to add a link to your
document that explains the lifetime of m_data and m_image.

[hajimehoshi, 2016/07/13 07:36:22]

Done.

 }
 
 void ImageResource::updateImageAndClearBuffer()
 {
     clearImage();
     updateImage(true);
-    m_data.clear();

[haraken, 2016/07/13 02:18:29]

I don't fully understand why you need to move this code into
ImageResource::updateImage.

[hajimehoshi, 2016/07/13 07:36:22]

m_data.clear() is called in updateImage(true) instead.

[haraken, 2016/07/13 07:38:34]

My question is why you moved m_data.clear() into updateImage(true).

It's strange that "updateImageAndClearBuffer" does not clear the buffer.

[hajimehoshi, 2016/07/13 07:59:59]

Ah, right. The reason why I moved m_data.clear() to updateImage() is because now
m_data is always cleared when m_image is updated (and all data is received).

On second thought, I reverted this and moved m_data.clear() in updateImage() to
finish() to minimize diff. Done.

 }
 
 void ImageResource::finish(double loadFinishTime)
@@ -518,7 +527,7 @@ void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher)
     if (isLoading())
         m_loader->cancel();
     else
-        updateImageAndClearBuffer();

[haraken, 2016/07/13 02:18:29]

Ditto. Why do you need this change?

[hajimehoshi, 2016/07/13 07:36:22]

As the document says, updateImageAndClearBuffer() ensures that m_image exists
with the current m_data and clears m_data (new data is set to m_data when
finishing the loading).

Found that this is no longer needed (m_image must exist and m_data doesn't).
Removed.

+        updateImage(true);
     setStatus(NotStarted);
     fetcher->startLoad(this);
 }