Remove multi-frame image decoder when the image is completely decoded

Count decoded frames in ImageFrameGenerator, and remove remove multi-frame image decoders when the image is completely decoded

This saves memory on pages with a lot of multi-frame images. data and discussion please see https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/KZ8EDtEZ9-c

BUG=429614

Review URL: https://codereview.chromium.org/688423004

git-svn-id: svn://svn.chromium.org/blink/trunk@185311 bbb929c8-8fbe-4397-9dbb-9b2b20218538

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

@@ -74,6 +74,7 @@ public:
         ImageDecodingStore::instance()->setCacheLimitInBytes(1024 * 1024);
         DeferredImageDecoder::setEnabled(true);
         m_data = SharedBuffer::create(whitePNG, sizeof(whitePNG));
+        m_frameCount = 1;
         OwnPtr<MockImageDecoder> decoder = MockImageDecoder::create(this);
         m_actualDecoder = decoder.get();
         m_actualDecoder->setSize(1, 1);
@@ -81,7 +82,6 @@ public:
         m_canvas.reset(SkCanvas::NewRasterN32(100, 100));
         ASSERT_TRUE(m_canvas.get());
         m_frameBufferRequestCount = 0;
-        m_frameCount = 1;
         m_repetitionCount = cAnimationNone;
         m_status = ImageFrame::FrameComplete;
         m_frameDuration = 0;

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

@@ -90,6 +90,7 @@ ImageFrameGenerator::ImageFrameGenerator(const SkISize& fullSize, PassRefPtr<Sha
     , m_isMultiFrame(isMultiFrame)
     , m_decodeFailedAndEmpty(false)
     , m_decodeCount(0)
+    , m_frameCount(0)
 {
     setData(data.get(), allDataReceived);
 }
@@ -204,6 +205,9 @@ SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_
 
     if (!decoder)
         return SkBitmap();
+    if (index >= m_frameComplete.size())
+        m_frameComplete.resize(index + 1);
+    m_frameComplete[index] = complete;
 
     // If we are not resuming decoding that means the decoder is freshly
     // created and we have ownership. If we are resuming decoding then
@@ -223,13 +227,26 @@ SkBitmap ImageFrameGenerator::tryToResumeDecode(const SkISize& scaledSize, size_
     }
 
     // If the image generated is complete then there is no need to keep
-    // the decoder. The exception is multi-frame decoder which can generate
-    // multiple complete frames.
-    const bool removeDecoder = complete && !m_isMultiFrame;
+    // the decoder. For multi-frame images, if all frames in the image are
+    // decoded, we remove the decoder.
+    bool removeDecoder;
+
+    if (m_isMultiFrame) {
+        size_t decodedFrameCount = 0;
+        for (Vector<bool>::iterator it = m_frameComplete.begin(); it != m_frameComplete.end(); ++it) {
+            if (*it)
+                decodedFrameCount++;
+        }
+        removeDecoder = m_frameCount && (decodedFrameCount == m_frameCount);
+    } else {
+        removeDecoder = complete;
+    }
 
     if (resumeDecoding) {
-        if (removeDecoder)
+        if (removeDecoder) {
             ImageDecodingStore::instance()->removeDecoder(this, decoder);
+            m_frameComplete.clear();
+        }
         else
             ImageDecodingStore::instance()->unlockDecoder(this, decoder);
     } else if (!removeDecoder) {
@@ -282,6 +299,13 @@ bool ImageFrameGenerator::decode(size_t index, ImageDecoder** decoder, SkBitmap*
     (*decoder)->setData(data, allDataReceived);
 
     ImageFrame* frame = (*decoder)->frameBufferAtIndex(index);
+    // For multi-frame image decoders, we need to know how many frames are
+    // in that image in order to release the decoder when all frames are
+    // decoded. frameCount() is reliable only if all data is received and set in
+    // decoder, particularly with GIF.
+    if (allDataReceived)
+        m_frameCount = (*decoder)->frameCount();
+
     (*decoder)->setData(0, false); // Unref SharedBuffer from ImageDecoder.
     (*decoder)->clearCacheExceptFrame(index);
     (*decoder)->setMemoryAllocator(0);

third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.h

@@ -109,6 +109,8 @@ private:
     bool m_decodeFailedAndEmpty;
     Vector<bool> m_hasAlpha;
     int m_decodeCount;
+    Vector<bool> m_frameComplete;
+    size_t m_frameCount;
     OwnPtr<ExternalMemoryAllocator> m_externalAllocator;
 
     OwnPtr<ImageDecoderFactory> m_imageDecoderFactory;

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

@@ -59,6 +59,7 @@ public:
         m_decodersDestroyed = 0;
         m_frameBufferRequestCount = 0;
         m_status = ImageFrame::FrameEmpty;
+        m_frameCount = 1;
     }
 
     virtual void TearDown() override
@@ -83,8 +84,8 @@ public:
         return currentStatus;
     }
 
-    virtual size_t frameCount() override { return 1; }
-    virtual int repetitionCount() const override { return cAnimationNone; }
+    virtual size_t frameCount() override { return m_frameCount; }
+    virtual int repetitionCount() const override { return m_frameCount == 1 ? cAnimationNone:cAnimationLoopOnce; }
     virtual float frameDuration() const override { return 0; }
 
 protected:
@@ -101,6 +102,15 @@ protected:
 
     void setFrameStatus(ImageFrame::Status status)  { m_status = m_nextFrameStatus = status; }
     void setNextFrameStatus(ImageFrame::Status status)  { m_nextFrameStatus = status; }
+    void setFrameCount(size_t count)
+    {
+        m_frameCount = count;
+        if (count > 1) {
+            m_generator.clear();
+            m_generator = ImageFrameGenerator::create(fullSize(), m_data, true, true);
+            useMockImageDecoderFactory();
+        }
+    }
 
     RefPtr<SharedBuffer> m_data;
     RefPtr<ImageFrameGenerator> m_generator;
@@ -108,6 +118,7 @@ protected:
     int m_frameBufferRequestCount;
     ImageFrame::Status m_status;
     ImageFrame::Status m_nextFrameStatus;
+    size_t m_frameCount;
 };
 
 TEST_F(ImageFrameGeneratorTest, incompleteDecode)
@@ -179,8 +190,8 @@ TEST_F(ImageFrameGeneratorTest, frameHasAlpha)
     setFrameStatus(ImageFrame::FramePartial);
 
     char buffer[100 * 100 * 4];
-    m_generator->decodeAndScale(imageInfo(), 1, buffer, 100 * 4);
-    EXPECT_TRUE(m_generator->hasAlpha(1));
+    m_generator->decodeAndScale(imageInfo(), 0, buffer, 100 * 4);
+    EXPECT_TRUE(m_generator->hasAlpha(0));
     EXPECT_EQ(1, m_frameBufferRequestCount);
 
     ImageDecoder* tempDecoder = 0;
@@ -190,9 +201,33 @@ TEST_F(ImageFrameGeneratorTest, frameHasAlpha)
     ImageDecodingStore::instance()->unlockDecoder(m_generator.get(), tempDecoder);
 
     setFrameStatus(ImageFrame::FrameComplete);
+    m_generator->decodeAndScale(imageInfo(), 0, buffer, 100 * 4);
+    EXPECT_EQ(2, m_frameBufferRequestCount);
+    EXPECT_FALSE(m_generator->hasAlpha(0));
+}
+
+TEST_F(ImageFrameGeneratorTest, removeMultiFrameDecoder)
+{
+    setFrameCount(3);
+    setFrameStatus(ImageFrame::FrameComplete);
+
+    char buffer[100 * 100 * 4];
+    m_generator->decodeAndScale(imageInfo(), 0, buffer, 100 * 4);
+    EXPECT_EQ(1, m_frameBufferRequestCount);
+    EXPECT_EQ(0, m_decodersDestroyed);
+
+    setFrameStatus(ImageFrame::FrameComplete);
+
     m_generator->decodeAndScale(imageInfo(), 1, buffer, 100 * 4);
     EXPECT_EQ(2, m_frameBufferRequestCount);
-    EXPECT_FALSE(m_generator->hasAlpha(1));
+    EXPECT_EQ(0, m_decodersDestroyed);
+
+    setFrameStatus(ImageFrame::FrameComplete);
+
+    // Multi frame decoder should be removed.
+    m_generator->decodeAndScale(imageInfo(), 2, buffer, 100 * 4);
+    EXPECT_EQ(3, m_frameBufferRequestCount);
+    EXPECT_EQ(1, m_decodersDestroyed);
 }
 
 } // namespace blink

third_party/WebKit/Source/platform/graphics/test/MockImageDecoder.h

@@ -69,8 +69,9 @@ public:
     virtual bool setSize(unsigned width, unsigned height) override
     {
         ImageDecoder::setSize(width, height);
-        m_frameBufferCache.resize(1);
-        m_frameBufferCache[0].setSize(width, height);
+        m_frameBufferCache.resize(m_client->frameCount());
+        for (size_t index = 0; index < m_client->frameCount(); ++index)
+            m_frameBufferCache[index].setSize(width, height);
         return true;
     }
 
@@ -89,12 +90,12 @@ public:
         return m_client->repetitionCount();
     }
 
-    virtual ImageFrame* frameBufferAtIndex(size_t) override
+    virtual ImageFrame* frameBufferAtIndex(size_t index) override
     {
         m_client->frameBufferRequested();
 
-        m_frameBufferCache[0].setStatus(m_client->status());
-        return &m_frameBufferCache[0];
+        m_frameBufferCache[index].setStatus(m_client->status());
+        return &m_frameBufferCache[index];
     }
 
     virtual bool frameIsCompleteAtIndex(size_t) const override
@@ -107,7 +108,13 @@ public:
         return m_client->frameDuration();
     }
 
-    void setFrameHasAlpha(bool hasAlpha) { m_frameBufferCache[0].setHasAlpha(hasAlpha); }
+    void setFrameHasAlpha(bool hasAlpha)
+    {
+        for (size_t index = 0; index < m_client->frameCount(); ++index)
+            m_frameBufferCache[index].setHasAlpha(hasAlpha);
+    }
+
+    virtual size_t clearCacheExceptFrame(size_t) override { return 0; }
 
 private:
     MockImageDecoderClient* m_client;