Remove duplication of encoded image data

third_party/WebKit/Source/platform/graphics/BitmapImage.cpp

 /*
  * Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2008 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 100 matching lines @@
 
 void BitmapImage::destroyDecodedData()
 {
     m_cachedFrame.clear();
     for (size_t i = 0; i < m_frames.size(); ++i)
         m_frames[i].clear(true);
     m_source.clearCacheExceptFrame(kNotFound);
     notifyMemoryChanged();
 }
 
+PassRefPtr<SharedBuffer> BitmapImage::data()
+{
+    return m_source.data();
+}
+
 void BitmapImage::notifyMemoryChanged()
 {
     if (getImageObserver())
         getImageObserver()->decodedSizeChangedTo(this, totalFrameBytes());
 }
 
 size_t BitmapImage::totalFrameBytes()
 {
     const size_t numFrames = frameCount();
     size_t totalBytes = 0;
@@ skipping 46 matching lines @@
 {
     updateSize();
     return m_sizeRespectingOrientation;
 }
 
 bool BitmapImage::getHotSpot(IntPoint& hotSpot) const
 {
     return m_source.getHotSpot(hotSpot);
 }
 
+bool BitmapImage::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
+{
+    m_temporaryEncodedImageData = data;

[f(malita), 2016/07/08 15:34:40]

Do we need m_temporaryEncodedImageData at all?  Looks like we only set it
temporarily here, and dataChanged always clears it.

Why not set the source data upfront, before calling dataChanged() - i.e. would
this work:

bool BitmapImage::setData(PassRefPtr<SharedBuffer> data, bool allDataReceived)
{
    if (!data || data->isEmpty())
        return true;

    m_source.setData(*data, allDataReceived);

    return dataChanged(allDataReceived);
}

[hajimehoshi, 2016/07/11 06:15:07]

Done.

[f(malita), 2016/07/12 17:10:59]

Thanks, LGTM!

+    if (!m_temporaryEncodedImageData.get())
+        return true;
+
+    int length = m_temporaryEncodedImageData->size();
+    if (!length)
+        return true;
+
+    return dataChanged(allDataReceived);
+}
+
 bool BitmapImage::dataChanged(bool allDataReceived)
 {
     TRACE_EVENT0("blink", "BitmapImage::dataChanged");
 
     // Clear all partially-decoded frames. For most image formats, there is only
     // one frame, but at least GIF and ICO can have more. With GIFs, the frames
     // come in order and we ask to decode them in order, waiting to request a
     // subsequent frame until the prior one is complete. Given that we clear
     // incomplete frames here, this means there is at most one incomplete frame
     // (even if we use destroyDecodedData() -- since it doesn't reset the
@@ skipping 14 matching lines @@
         // cleared-on-a-previous-pass) frames!
         if (m_frames[i].m_haveMetadata && !m_frames[i].m_isComplete) {
             m_frames[i].clear(true);
             if (i == m_cachedFrameIndex)
                 m_cachedFrame.clear();
         }
     }
 
     // Feed all the data we've seen so far to the image decoder.
     m_allDataReceived = allDataReceived;
-    ASSERT(data());
-    m_source.setData(*data(), allDataReceived);
+    // |m_temporaryEncodedImageData| can be null only when testing.
+    if (m_temporaryEncodedImageData) {
+        m_source.setData(*m_temporaryEncodedImageData, allDataReceived);
+        m_temporaryEncodedImageData.clear();
+    }
 
     m_haveFrameCount = false;
     return isSizeAvailable();
 }
 
 bool BitmapImage::hasColorProfile() const
 {
     return m_source.hasColorProfile();
 }
 
@@ skipping 357 matching lines @@
 
     // We need to draw this frame if we advanced to it while not skipping, or if
     // while trying to skip frames we hit the last frame and thus had to stop.
     if (skippingFrames != advancedAnimation)
         getImageObserver()->animationAdvanced(this);
 
     return advancedAnimation;
 }
 
 } // namespace blink