High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations.

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

Index: third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp
index 9ea87b7d94178db75d3f5bbe19579caeeb37f9b3..93e2ab841939ce403bd044a1fb3cc22587da9373 100644
--- a/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.cpp
@@ -28,7 +28,6 @@
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/SharedBuffer.h"
 #include "platform/graphics/DecodingImageGenerator.h"
-#include "platform/graphics/FrameData.h"
 #include "platform/graphics/ImageDecodingStore.h"
 #include "platform/graphics/ImageFrameGenerator.h"
 #include "platform/image-decoders/SegmentReader.h"
@@ -37,6 +36,25 @@
 
 namespace blink {
 
+struct DeferredFrameData {
+    DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
+    WTF_MAKE_NONCOPYABLE(DeferredFrameData);
+public:
+    DeferredFrameData()
+        : m_orientation(DefaultImageOrientation)
+        , m_duration(0)
+        , m_isComplete(false)
+        , m_frameBytes(0)
+        , m_uniqueID(DecodingImageGenerator::NeedNewImageUniqueID)
+    {}
+
+    ImageOrientation m_orientation;
+    float m_duration;
+    bool m_isComplete;
+    size_t m_frameBytes;
+    uint32_t m_uniqueID;
+};
+
 bool DeferredImageDecoder::s_enabled = true;
 
 PassOwnPtr<DeferredImageDecoder> DeferredImageDecoder::create(const SharedBuffer& data, ImageDecoder::AlphaOption alphaOption, ImageDecoder::GammaAndColorProfileOption colorOptions)
@@ -90,16 +108,15 @@ PassRefPtr<SkImage> DeferredImageDecoder::createFrameAtIndex(size_t index)
     prepareLazyDecodedFrames();
 
     if (index < m_frameData.size()) {
-        FrameData* frameData = &m_frameData[index];
-        // ImageFrameGenerator has the latest known alpha state. There will be a
-        // performance boost if this frame is opaque.
-        ASSERT(m_frameGenerator);
-        frameData->m_hasAlpha = m_frameGenerator->hasAlpha(index);
+        DeferredFrameData* frameData = &m_frameData[index];
         if (m_actualDecoder)
             frameData->m_frameBytes = m_actualDecoder->frameBytesAtIndex(index);
         else
             frameData->m_frameBytes = m_size.area() * sizeof(ImageFrame::PixelData);
-        return createFrameImageAtIndex(index, !frameData->m_hasAlpha);
+        // ImageFrameGenerator has the latest known alpha state. There will be a
+        // performance boost if this frame is opaque.
+        ASSERT(m_frameGenerator);
+        return createFrameImageAtIndex(index, !m_frameGenerator->hasAlpha(index));
     }
 
     if (!m_actualDecoder || m_actualDecoder->failed())
@@ -258,7 +275,6 @@ void DeferredImageDecoder::prepareLazyDecodedFrames()
         return;
 
     for (size_t i = previousSize; i < m_frameData.size(); ++i) {
-        m_frameData[i].m_haveMetadata = true;
         m_frameData[i].m_duration = m_actualDecoder->frameDurationAtIndex(i);
         m_frameData[i].m_orientation = m_actualDecoder->orientation();
         m_frameData[i].m_isComplete = m_actualDecoder->frameIsCompleteAtIndex(i);
@@ -283,7 +299,7 @@ inline SkImageInfo imageInfoFrom(const SkISize& decodedSize, bool knownToBeOpaqu
     return SkImageInfo::MakeN32(decodedSize.width(), decodedSize.height(), knownToBeOpaque ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
 }
 
-PassRefPtr<SkImage> DeferredImageDecoder::createFrameImageAtIndex(size_t index, bool knownToBeOpaque) const
+PassRefPtr<SkImage> DeferredImageDecoder::createFrameImageAtIndex(size_t index, bool knownToBeOpaque)
 {
     const SkISize& decodedSize = m_frameGenerator->getFullSize();
     ASSERT(decodedSize.width() > 0);
@@ -291,11 +307,12 @@ PassRefPtr<SkImage> DeferredImageDecoder::createFrameImageAtIndex(size_t index,
 
     RefPtr<SkROBuffer> roBuffer = adoptRef(m_rwBuffer->newRBufferSnapshot());
     RefPtr<SegmentReader> segmentReader = SegmentReader::createFromSkROBuffer(roBuffer.release());
-    DecodingImageGenerator* generator = new DecodingImageGenerator(m_frameGenerator, imageInfoFrom(decodedSize, knownToBeOpaque), segmentReader.release(), m_allDataReceived, index);
+    DecodingImageGenerator* generator = new DecodingImageGenerator(m_frameGenerator, imageInfoFrom(decodedSize, knownToBeOpaque), segmentReader.release(), m_allDataReceived, index, m_frameData[index].m_uniqueID);
     RefPtr<SkImage> image = adoptRef(SkImage::NewFromGenerator(generator)); // SkImage takes ownership of the generator.
     if (!image)
         return nullptr;
 
+    m_frameData[index].m_uniqueID = (m_allDataReceived || (m_actualDecoder && m_actualDecoder->frameIsCompleteAtIndex(index))) ? image->uniqueID() : DecodingImageGenerator::NeedNewImageUniqueID;

[scroggo_chromium, 2016/05/03 13:08:52]

Thanks, yes, I forgot to look at the GIF decoder override :-/

So yes, looking at m_actualDecoder->frameIsCompleteAtIndex() makes sense here.
Now that I understand it better, I think you actually *do* want to use
m_isComplete (which caches m_actualDecoder->frameIsCompleteAtIndex()). If
looking at m_allDataReceived helps (and you indicated that it does), then use
it, too, although I would expect that if all data had been received we would
have also parsed the frame. So that might be worth looking into.

[Peter Kasting, 2016/05/04 03:01:07]

I also would think that if m_allDataReceived, then either the frame should be
complete at this index, or else it's wrong to access uniqueID().  So I think
that condition check ought not to be needed?

Also, shouldn't m_uniqueID already be kNeedNewImageUniqueID unless the frame was
already marked complete?  So it seems like this could be rewritten more like:

    if (m_actualDecoder && m_actualDecoder->frameIsCompleteAtIndex(index))
        m_frameData[index].m_uniqueID = image->uniqueID();

[aleksandar.stojiljkovic, 2016/05/04 20:56:29]

It is important to have this check:
if (m_allDataReceived || m_frameData[index].m_isComplete)
m_isComplete has diffefrent meaning for gif animations or e.g. one frame JPEG.
reusing uniqueid is important there with jpegs too - when m_allDataReceived is
received but m_isComplete is false. verified this claim with debugger.

[scroggo_chromium, 2016/05/04 21:15:54]

Ah, of course! I find it very confusing that frameIsCompleteAtIndex() returns a
different conceptual meaning depending on whether we're talking about a
multi-frame image or a single frame image. Maybe we really want a method called
frameHasAllData (that's a terrible name, but I think it almost captures my idea)
which would return m_allDataReceived in single frame images and whether the
frame has been parsed in GIF (and something analogous in WEBP). That should be a
separate change, if we go that route.

(We really don't care here whether the image is "Complete" - what we want to
know is - might it change later? Even if the stream is truncated, we want to
keep reusing the same ID once we've processed the entire stream.)

     generator->setCanYUVDecode(m_canYUVDecode);
 
     return image.release();
@@ -308,3 +325,10 @@ bool DeferredImageDecoder::hotSpot(IntPoint& hotSpot) const
 }
 
 } // namespace blink
+
+namespace WTF {
+template<> struct VectorTraits<blink::DeferredFrameData> : public SimpleClassVectorTraits<blink::DeferredFrameData> {
+    STATIC_ONLY(VectorTraits);
+    static const bool canInitializeWithMemset = false; // Not all DeferredFrameData members initialize to 0.
+};
+}