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..30594906a5beeffd35718dfc9534ff5ddbdb5fd0 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -123,10 +123,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);
@@ -189,15 +186,13 @@ bool ImageResource::isSafeToUnlock() const
 
 void ImageResource::destroyDecodedDataForFailedRevalidation()
 {
-    m_image = nullptr;
-    setDecodedSize(0);
+    m_image->destroyDecodedData();
 }
 
 void ImageResource::destroyDecodedDataIfPossible()
 {
-    if (!hasClientsOrObservers() && !isLoading() && (!m_image || (m_image->hasOneRef() && m_image->isBitmapImage()))) {
-        m_image = nullptr;
-        setDecodedSize(0);
+    if (!hasClientsOrObservers() && !isLoading() && m_image && (m_image->hasOneRef() && m_image->isBitmapImage())) {

[scroggo_chromium, 2016/06/20 19:00:43]

Are the parentheses around

  m_image->hasOneRef() && m_image->isBitmapImage()

still necessary? It looks like you just have &&'s, so the order should not
matter?

[hajimehoshi, 2016/06/22 09:42:29]

Done.

+        m_image->destroyDecodedData();
     } else if (m_image && !errorOccurred()) {
         m_image->destroyDecodedData();

[scroggo_chromium, 2016/06/20 19:00:43]

This else statement appears to do the exact same thing as the if case. Should
this be one giant if?

[hajimehoshi, 2016/06/22 09:42:29]

Done.

     }
@@ -212,6 +207,16 @@ void ImageResource::allClientsAndObserversRemoved()
     Resource::allClientsAndObserversRemoved();
 }
 
+PassRefPtr<SharedBuffer> ImageResource::resourceBuffer() const
+{
+    RefPtr<SharedBuffer> data = Resource::resourceBuffer();
+    if (data)
+        return data;
+    if (m_image)
+        return m_image->data();
+    return nullptr;
+}
+
 void ImageResource::appendData(const char* data, size_t length)
 {
     if (m_multipartParser) {
@@ -355,8 +360,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)

[hajimehoshi, 2016/06/22 09:42:29]

Found that there is a case |m_data| is null (ImageResource::finish now clears
|m_data| as of this CL). I've changed the condition.

+    if (m_image) {
+        DCHECK(m_data);
         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
@@ -375,13 +382,15 @@ void ImageResource::updateImage(bool allDataReceived)
         // (decoding delayed until painting) that seems hard.
         notifyObservers();
     }
+
+    if (allDataReceived)
+        m_data.clear();
 }
 
 void ImageResource::updateImageAndClearBuffer()
 {
     clearImage();
     updateImage(true);
-    m_data.clear();
 }
 
 void ImageResource::finish(double loadFinishTime)
@@ -500,10 +509,9 @@ void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher)
         return;
     m_resourceRequest.setCachePolicy(WebCachePolicy::BypassingCache);
     m_resourceRequest.setLoFiState(WebURLRequest::LoFiOff);
-    if (isLoading())
+    if (isLoading()) {

[scroggo_chromium, 2016/06/20 19:00:43]

nit: My understanding of the style guide is that this brace should not be here.

[hajimehoshi, 2016/06/22 09:42:29]

Done.

         m_loader->cancel();
-    else
-        updateImageAndClearBuffer();
+    }
     setStatus(NotStarted);
     fetcher->startLoad(this);
 }