ScalableImageDecoder: don't forcefully decode image data when querying frame complete...
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Apr 2019 06:16:04 +0000 (06:16 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Apr 2019 06:16:04 +0000 (06:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191354
<rdar://problem/46123406>

Reviewed by Michael Catanzaro.

ScalableImageDecoder::frameIsCompleteAtIndex() should only check the
index validity and, if the index is valid, check for completeness of the
corresponding frame. ScalableImageDecoder::frameDurationAtIndex() should
also only retrieve duration for already-complete frames, or expand the
default 0-second value according to the flashing-protection rule when
the target frame is not yet complete.

Both methods avoid calling ScalableImageDecoder::frameBufferAtIndex()
as that method goes on and decodes image data to determine specific
information. The ImageSource class that's querying this information
doesn't anticipate this, and doesn't handle the increased memory
consumption of the decoded data, leaving MemoryCache in the blind about
the image resource's actual amount of consumed memory. ImageSource can
instead gracefully handle any incomplete frame by marking the decoding
status for this frame as only partial.

* platform/image-decoders/ScalableImageDecoder.cpp:
(WebCore::ScalableImageDecoder::frameIsCompleteAtIndex const):
(WebCore::ScalableImageDecoder::frameHasAlphaAtIndex const):
(WebCore::ScalableImageDecoder::frameDurationAtIndex const):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/image-decoders/ScalableImageDecoder.cpp

index e8e512b..87363a3 100644 (file)
@@ -1,3 +1,32 @@
+2019-04-16  Zan Dobersek  <zdobersek@igalia.com>
+
+        ScalableImageDecoder: don't forcefully decode image data when querying frame completeness, duration
+        https://bugs.webkit.org/show_bug.cgi?id=191354
+        <rdar://problem/46123406>
+
+        Reviewed by Michael Catanzaro.
+
+        ScalableImageDecoder::frameIsCompleteAtIndex() should only check the
+        index validity and, if the index is valid, check for completeness of the
+        corresponding frame. ScalableImageDecoder::frameDurationAtIndex() should
+        also only retrieve duration for already-complete frames, or expand the
+        default 0-second value according to the flashing-protection rule when
+        the target frame is not yet complete.
+
+        Both methods avoid calling ScalableImageDecoder::frameBufferAtIndex()
+        as that method goes on and decodes image data to determine specific
+        information. The ImageSource class that's querying this information
+        doesn't anticipate this, and doesn't handle the increased memory
+        consumption of the decoded data, leaving MemoryCache in the blind about
+        the image resource's actual amount of consumed memory. ImageSource can
+        instead gracefully handle any incomplete frame by marking the decoding
+        status for this frame as only partial.
+
+        * platform/image-decoders/ScalableImageDecoder.cpp:
+        (WebCore::ScalableImageDecoder::frameIsCompleteAtIndex const):
+        (WebCore::ScalableImageDecoder::frameHasAlphaAtIndex const):
+        (WebCore::ScalableImageDecoder::frameDurationAtIndex const):
+
 2019-04-16  Ross Kirsling  <ross.kirsling@sony.com>
 
         Unreviewed non-unified build fix after r244307.
index 00810e5..2af2cf2 100644 (file)
@@ -196,11 +196,11 @@ template <MatchType type> int getScaledValue(const Vector<int>& scaledValues, in
 bool ScalableImageDecoder::frameIsCompleteAtIndex(size_t index) const
 {
     LockHolder lockHolder(m_mutex);
-    // FIXME(176089): asking whether enough data has been appended for a decode
-    // operation to succeed should not require decoding the entire frame.
-    // This function should be implementable in a way that allows const.
-    auto* buffer = const_cast<ScalableImageDecoder*>(this)->frameBufferAtIndex(index);
-    return buffer && buffer->isComplete();
+    if (index >= m_frameBufferCache.size())
+        return false;
+
+    auto& frame = m_frameBufferCache[index];
+    return frame.isComplete();
 }
 
 bool ScalableImageDecoder::frameHasAlphaAtIndex(size_t index) const
@@ -208,9 +208,11 @@ bool ScalableImageDecoder::frameHasAlphaAtIndex(size_t index) const
     LockHolder lockHolder(m_mutex);
     if (m_frameBufferCache.size() <= index)
         return true;
-    if (m_frameBufferCache[index].isComplete())
-        return m_frameBufferCache[index].hasAlpha();
-    return true;
+
+    auto& frame = m_frameBufferCache[index];
+    if (!frame.isComplete())
+        return true;
+    return frame.hasAlpha();
 }
 
 unsigned ScalableImageDecoder::frameBytesAtIndex(size_t index, SubsamplingLevel) const
@@ -225,20 +227,24 @@ unsigned ScalableImageDecoder::frameBytesAtIndex(size_t index, SubsamplingLevel)
 Seconds ScalableImageDecoder::frameDurationAtIndex(size_t index) const
 {
     LockHolder lockHolder(m_mutex);
-    // FIXME(176089): asking for the duration of a sub-image should not require decoding
-    // the entire frame. This function should be implementable in a way that
-    // allows const.
-    auto* buffer = const_cast<ScalableImageDecoder*>(this)->frameBufferAtIndex(index);
-    if (!buffer || buffer->isInvalid())
+    if (index >= m_frameBufferCache.size())
         return 0_s;
-    
+
+    // Returning 0_s in case of an incomplete frame can break display of animated image formats.
+    // We pick up the decoded duration if it's available, otherwise the default 0_s value is
+    // adjusted below.
+    Seconds duration = 0_s;
+    auto& frame = m_frameBufferCache[index];
+    if (frame.isComplete())
+        duration = frame.duration();
+
     // Many annoying ads specify a 0 duration to make an image flash as quickly as possible.
     // We follow Firefox's behavior and use a duration of 100 ms for any frames that specify
     // a duration of <= 10 ms. See <rdar://problem/7689300> and <http://webkit.org/b/36082>
     // for more information.
-    if (buffer->duration() < 11_ms)
+    if (duration < 11_ms)
         return 100_ms;
-    return buffer->duration();
+    return duration;
 }
 
 NativeImagePtr ScalableImageDecoder::createFrameImageAtIndex(size_t index, SubsamplingLevel, const DecodingOptions&)