Don't cache GIF frames if it would be too large.

third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
index 68e9334457a0978512762e0a267028649ede44d6..75cd4bfdb6b292f445c1057704be53e3a73b88ec 100644
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -34,6 +34,7 @@ namespace blink {
 
 GIFImageDecoder::GIFImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes)
     : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes)
+    , m_purgeAggressively(false)
     , m_repetitionCount(cAnimationLoopOnce)
 {
 }
@@ -301,6 +302,44 @@ void GIFImageDecoder::decode(size_t index)
     if (failed())
         return;
 
+    if (!m_purgeAggressively) {

[scroggo_chromium, 2016/06/28 15:55:28]

How do you feel about making this a private method? e.g.

void determineWhetherToPurgeAggressively() {
    if (m_purgeAggressively)
        return;

    const uint64_t frameArea = ...
    bool wouldOverflow = ...
    if (wouldOverflow) {
        m_purgeAggressively = true;
        return;
    }

    const uint64_t frameMemoryUsage = ...
    wouldOverflow = ...
    if (wouldOverflow) {
        m_purgeAggressively = true;
        return;
    }

    const uint64_t totalMemoryUsage = ...
    if (totalMemoryUsage > m_maxDecodedBytes)
        m_purgeAggressively = true;
}

That isolates the code around deciding whether to purge aggressively. It also
reduces your nested if/elses, and I think it makes the code a little easier to
follow.

(And maybe there's a shorter name that is clear about what it does...)

[cblume, 2016/06/28 16:53:33]

I like it. Done.

+        // We don't want to cache so much that we cause a memory issue.
+        //
+        // If we used a LRU cache we would fill it and then on next animation loop
+        // we would need to decode all the frames again -- the LRU would give no
+        // benefit and would consume more memory.
+        // So instead, simply purge unused frames if caching all of the frames of
+        // the image would use more memory than the image decoder is allowed
+        // (m_maxDecodedBytes).

[scroggo_chromium, 2016/06/28 15:55:28]

or overflow.

[cblume, 2016/06/28 16:53:33]

Done.

+        //
+        // As we decode we will learn the total number of frames, and thus total
+        // possible image memory used.
+
+        const uint64_t frameArea = decodedSize().area();
+        // We are about to multiply by 4, which may require an extra bit of storage
+        bool wouldOverflow = frameArea > (UINT64_C(1) << 62);
+        if (!wouldOverflow) {

[scroggo_chromium, 2016/06/28 15:55:28]

I think you want

    if (wouldOverflow)

? (No "!")

[cblume, 2016/06/28 16:53:33]

Done.

+            m_purgeAggressively = true;
+        } else {
+            const uint64_t frameMemoryUsage = frameArea * 4; // 4 bytes per pixel
+            // We are about to multiply by a size_t, which does not have a fixed
+            // size.
+            // To simplify things, let's make sure our per-frame memory usage and
+            // index can be stored in 32 bits and store the multiplicand in a 64-bit
+            // number.
+            wouldOverflow = (frameMemoryUsage > (UINT32_C(1) << 31))
+                || (index > (UINT32_C(1) << 31));
+            if (wouldOverflow) {
+                m_purgeAggressively = true;
+            } else {
+                const uint64_t totalMemoryUsage = frameMemoryUsage * index;
+                if (wouldOverflow || totalMemoryUsage > m_maxDecodedBytes) {

[scroggo_chromium, 2016/06/28 15:55:28]

No need to check wouldOverflow here. It is guaranteed to be false, since this is
the else statement to if (wouldOverflow)

[cblume, 2016/06/28 16:53:33]

Done.

+                    m_purgeAggressively = true;
+                }
+            }
+        }
+    }
+
     Vector<size_t> framesToDecode;
     size_t frameToDecode = index;
     do {
@@ -314,6 +353,9 @@ void GIFImageDecoder::decode(size_t index)
             return;
         }
 
+        if (m_purgeAggressively)
+            clearCacheExceptFrame(*i);
+
         // We need more data to continue decoding.
         if (m_frameBufferCache[*i].getStatus() != ImageFrame::FrameComplete)
             break;