[GTK][WPE] GIFImageDecoder never clears decoded frames even when told to do so

https://bugs.webkit.org/show_bug.cgi?id=177864

Reviewed by Carlos Garcia Campos.

Fix GIFImageDecoder::clearFrameBufferCache() so it really deletes decoded buffers, and modify
GIFImageDecoder to be able to decode frames that are not requested in the expected order.

Covered by existent tests.

* platform/image-decoders/gif/GIFImageDecoder.cpp:
(WebCore::GIFImageDecoder::findFirstRequiredFrameToDecode):
(WebCore::GIFImageDecoder::frameBufferAtIndex):
(WebCore::GIFImageDecoder::clearFrameBufferCache):
* platform/image-decoders/gif/GIFImageDecoder.h:
* platform/image-decoders/gif/GIFImageReader.cpp:
(GIFImageReader::decode):
* platform/image-decoders/gif/GIFImageReader.h:
(GIFImageReader::frameContext const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222910 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6938223..4ae046c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2017-10-05  Miguel Gomez  <magomez@igalia.com>
+
+        [GTK][WPE] GIFImageDecoder never clears decoded frames even when told to do so
+        https://bugs.webkit.org/show_bug.cgi?id=177864
+
+        Reviewed by Carlos Garcia Campos.
+
+        Fix GIFImageDecoder::clearFrameBufferCache() so it really deletes decoded buffers, and modify
+        GIFImageDecoder to be able to decode frames that are not requested in the expected order.
+
+        Covered by existent tests.
+
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::findFirstRequiredFrameToDecode):
+        (WebCore::GIFImageDecoder::frameBufferAtIndex):
+        (WebCore::GIFImageDecoder::clearFrameBufferCache):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::decode):
+        * platform/image-decoders/gif/GIFImageReader.h:
+        (GIFImageReader::frameContext const):
+
 2017-10-05  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [WinCairo] Fix build after Bug 167956

diff --git a/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp b/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
index 82e85dc..99e957e 100644 (file)
--- a/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -101,14 +101,55 @@ RepetitionCount GIFImageDecoder::repetitionCount() const
     return m_repetitionCount;
 }
 
+size_t GIFImageDecoder::findFirstRequiredFrameToDecode(size_t frameIndex)
+{
+    // The first frame doesn't depend on any other.
+    if (!frameIndex)
+        return 0;
+
+    for (size_t i = frameIndex; i > 0; --i) {
+        ImageFrame& frame = m_frameBufferCache[i - 1];
+
+        // Frames with disposal method RestoreToPrevious are useless, skip them.
+        if (frame.disposalMethod() == ImageFrame::DisposalMethod::RestoreToPrevious)
+            continue;
+
+        // At this point the disposal method can be Unspecified, DoNotDispose or RestoreToBackground.
+        // In every case, if the frame is complete we can start decoding the next one.
+        if (frame.isComplete())
+            return i;
+
+        // If the disposal method of this frame is RestoreToBackground and it fills the whole area,
+        // the next frame's backing store is initialized to transparent, so we start decoding with it.
+        if (frame.disposalMethod() == ImageFrame::DisposalMethod::RestoreToBackground) {
+            // We cannot use frame.backingStore()->frameRect() here, because it has been cleared
+            // when the frame was removed from the cache. We need to get the values from the
+            // reader context.
+            const auto* frameContext = m_reader->frameContext(i - 1);
+            ASSERT(frameContext);
+            IntRect frameRect(frameContext->xOffset, frameContext->yOffset, frameContext->width, frameContext->height);
+            // We would need to scale frameRect and check whether it fills the whole scaledSize(). But
+            // can check whether the original frameRect fills size() instead. If the frame fills the
+            // whole area then it can be decoded without dependencies.
+            if (frameRect.contains({ { }, size() }))
+                return i;
+        }
+    }
+
+    return 0;
+}
+
 ImageFrame* GIFImageDecoder::frameBufferAtIndex(size_t index)
 {
     if (index >= frameCount())
         return 0;
 
     ImageFrame& frame = m_frameBufferCache[index];
-    if (!frame.isComplete())
-        decode(index + 1, GIFFullQuery, isAllDataReceived());
+    if (!frame.isComplete()) {
+        for (auto i = findFirstRequiredFrameToDecode(index); i <= index; i++)
+            decode(i + 1, GIFFullQuery, isAllDataReceived());
+    }
+
     return &frame;
 }
 
@@ -163,7 +204,7 @@ void GIFImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
     // Now |i| holds the last frame we need to preserve; clear prior frames.
     for (Vector<ImageFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) {
         ASSERT(!j->isPartial());
-        if (j->isInvalid())
+        if (!j->isInvalid())
             j->clear();
     }
 }

diff --git a/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h b/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h
index e1eb044..68715ae 100644 (file)
--- a/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h
+++ b/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h
@@ -65,6 +65,7 @@ public:
 private:
     GIFImageDecoder(AlphaOption, GammaAndColorProfileOption);
     void tryDecodeSize(bool allDataReceived) final { decode(0, GIFSizeQuery, allDataReceived); }
+    size_t findFirstRequiredFrameToDecode(size_t);
 
     // If the query is GIFFullQuery, decodes the image up to (but not
     // including) |haltAtFrame|. Otherwise, decodes as much as is needed to

diff --git a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
index 7d0b010..c1db8a6 100644 (file)
--- a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
+++ b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp
@@ -365,9 +365,9 @@ bool GIFImageReader::decode(GIFImageDecoder::GIFQuery query, unsigned haltAtFram
 
     // Already decoded frames can be deleted from the cache and then they require to be decoded again, so
     // the haltAtFrame value we receive here may be lower than m_currentDecodingFrame. In this case
-    // we position m_currentDecodingFrame to haltAtFrame and decode from there.
+    // we position m_currentDecodingFrame to haltAtFrame - 1 and decode from there.
     // See bug https://bugs.webkit.org/show_bug.cgi?id=176089.
-    m_currentDecodingFrame = std::min(m_currentDecodingFrame, static_cast<size_t>(haltAtFrame));
+    m_currentDecodingFrame = std::min(m_currentDecodingFrame, static_cast<size_t>(haltAtFrame) - 1);
 
     while (m_currentDecodingFrame < std::min(m_frames.size(), static_cast<size_t>(haltAtFrame))) {
         bool frameDecoded = false;

diff --git a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h
index 40b1d69..e08da2b 100644 (file)
--- a/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h
+++ b/Source/WebCore/platform/image-decoders/gif/GIFImageReader.h
@@ -284,6 +284,11 @@ public:
         return m_currentDecodingFrame < m_frames.size() ? m_frames[m_currentDecodingFrame].get() : 0;
     }
 
+    const GIFFrameContext* frameContext(size_t frame) const
+    {
+        return frame < m_frames.size() ? m_frames[frame].get() : nullptr;
+    }
+
 private:
     bool parse(size_t dataPosition, size_t len, bool parseSizeOnly);
     void setRemainingBytes(size_t);