Changeset 222836 in webkit

Timestamp:

Oct 4, 2017 2:09:00 AM (7 years ago)

Author:

magomez@igalia.com

Message:

[GTK][WPE] Fix playback of GIFs
​https://bugs.webkit.org/show_bug.cgi?id=176089

Reviewed by Carlos Garcia Campos.

Allow GIFImageReader to decode again already decoded frames. Thanks to this, we don't
need to delete the GIFImageReader when GIFImageDecoder::clearFrameBufferCache() is
called, and the we don't need the lock to avoid crashes in that situation.

Covered by existent tests.

• platform/image-decoders/gif/GIFImageDecoder.cpp:

(WebCore::GIFImageDecoder::clearFrameBufferCache):
(WebCore::GIFImageDecoder::decode):

• platform/image-decoders/gif/GIFImageDecoder.h:
• platform/image-decoders/gif/GIFImageReader.cpp:

(GIFImageReader::decode):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/image-decoders/gif/GIFImageDecoder.cpp (modified) (3 diffs)
• platform/image-decoders/gif/GIFImageDecoder.h (modified) (1 diff)
• platform/image-decoders/gif/GIFImageReader.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-04  Miguel Gomez  <magomez@igalia.com>
+
+        [GTK][WPE] Fix playback of GIFs
+        https://bugs.webkit.org/show_bug.cgi?id=176089
+
+        Reviewed by Carlos Garcia Campos.
+
+        Allow GIFImageReader to decode again already decoded frames. Thanks to this, we don't
+        need to delete the GIFImageReader when GIFImageDecoder::clearFrameBufferCache() is
+        called, and the we don't need the lock to avoid crashes in that situation.
+
+        Covered by existent tests.
+
+        * platform/image-decoders/gif/GIFImageDecoder.cpp:
+        (WebCore::GIFImageDecoder::clearFrameBufferCache):
+        (WebCore::GIFImageDecoder::decode):
+        * platform/image-decoders/gif/GIFImageDecoder.h:
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::decode):
+
 2017-10-04  Gustavo Noronha Silva  <gustavo.noronha@collabora.co.uk>
 

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp

@@ -126 +126 @@
         return; // Nothing to do.
 
-    // Lock the decodelock here, as we are going to destroy the GIFImageReader and doing so while
-    // there's an ongoing decode will cause a crash.
-    LockHolder locker(m_decodeLock);
-
     // The "-1" here is tricky.  It does not mean that |clearBeforeFrame| is the
     // last frame we wish to preserve, but rather that we never want to clear
@@ -171 +167 @@
             j->clear();
     }
-
-    // When some frames are cleared, the reader is out of sync, since the caller might ask for any frame not
-    // necessarily in the order expected by the reader. See https://bugs.webkit.org/show_bug.cgi?id=159089.
-    m_reader = nullptr;
 }
 
@@ -303 +295 @@
         return;
 
-    LockHolder locker(m_decodeLock);
     if (!m_reader) {
         m_reader = std::make_unique<GIFImageReader>(this);

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.h

@@ -81 +81 @@
     mutable RepetitionCount m_repetitionCount { RepetitionCountOnce };
     std::unique_ptr<GIFImageReader> m_reader;
-    Lock m_decodeLock;
 };
 

trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp

@@ -364 +364 @@
         return true;
 
+    // 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.
+    // See bug https://bugs.webkit.org/show_bug.cgi?id=176089.
+    m_currentDecodingFrame = std::min(m_currentDecodingFrame, static_cast<size_t>(haltAtFrame));
+
     while (m_currentDecodingFrame < std::min(m_frames.size(), static_cast<size_t>(haltAtFrame))) {
         bool frameDecoded = false;