Changeset 238275 in webkit

Timestamp:

Nov 16, 2018 2:57:17 AM (5 years ago)

Author:

zandobersek@gmail.com

Message:

ScalableImageDecoder: don't forcefully decode image data when querying frame completeness, duration
​https://bugs.webkit.org/show_bug.cgi?id=191354

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.

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/image-decoders/ScalableImageDecoder.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-11-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
+
+        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.
+
+        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):
+
 2018-11-16  Antoine Quint  <graouts@apple.com>
 

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

@@ -174 +174 @@
 {
     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();
 }
 
@@ -186 +186 @@
     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();
 }
 
@@ -203 +205 @@
 {
     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;
-    
+
+    auto& frame = m_frameBufferCache[index];
+    if (!frame.isComplete())
+        return 0_s;
+
     // 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)
+    auto duration = frame.duration();
+    if (duration < 11_ms)
         return 100_ms;
-    return buffer->duration();
+    return duration;
 }