GIF Animations "clear from cache related glitch" fix

third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp

 /*
  * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
  *  License as published by the Free Software Foundation; either
  *  version 2 of the License, or (at your option) any later version.
  *
  *  This library is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 162 matching lines @@
     return ImageSize(frameSizeAtIndex(index)).area * sizeof(ImageFrame::PixelData);
 }
 
 bool ImageDecoder::deferredImageDecodingEnabled()
 {
     return DeferredImageDecoder::enabled();
 }
 
 size_t ImageDecoder::clearCacheExceptFrame(size_t clearExceptFrame)
 {
     // Don't clear if there are no frames or only one frame.

[aleksandar.stojiljkovic, 2016/01/12 17:34:59]

Copied this code including the comment from here.
Agree about comment. Code looks like a patch disabling cache clearing for single
frame images from DeferredImageDecoder:
void BitmapImage::destroyDecodedDataIfNecessary()
{
    // Animated images >5MB are considered large enough that we'll only hang on
    // to one frame at a time.
    static const size_t cLargeAnimationCutoff = 5242880;

    ....
    destroyDecodedData(false);

destroyDecodedData(true) calls clearCacheExceptFrame(kNotFound) that would
remove all frames but is not used any more (there is only call to
destroyDecodedData(false)). In other words, there are calls to
clearCacheExceptFrame(kNotFound) only in webkit tests.
I'll remove the comment from my change in this patch.

     if (m_frameBufferCache.size() <= 1)
         return 0;
 
     size_t frameBytesCleared = 0;
     for (size_t i = 0; i < m_frameBufferCache.size(); ++i) {
         if (i != clearExceptFrame) {
             frameBytesCleared += frameBytesAtIndex(i);
             clearFrameBuffer(i);
         }
     }
     return frameBytesCleared;
 }
 
+size_t ImageDecoder::clearCacheExceptFrames(const SizeTHashSet& clearExceptFrames)
+{
+    // Don't clear if there are no frames or only one frame.

[tomhudson, 2016/01/12 16:15:59]

This comment is backwards. It's nearly obvious from the following two lines of
code that you aren't clearing if there are 0 or 1 frames, but it's completely
inobvious *WHY*.

[aleksandar.stojiljkovic, 2016/01/12 17:34:59]

Done.

+    if (m_frameBufferCache.size() <= 1)
+        return 0;
+
+    size_t frameBytesCleared = 0;
+    for (size_t i = 0; i < m_frameBufferCache.size(); ++i) {
+        if (m_frameBufferCache[i].status() != ImageFrame::FrameEmpty && !clearExceptFrames.contains(i)) {
+            frameBytesCleared += frameBytesAtIndex(i);
+            clearFrameBuffer(i);
+        }
+    }
+    return frameBytesCleared;
+}
+
 void ImageDecoder::clearFrameBuffer(size_t frameIndex)
 {
     m_frameBufferCache[frameIndex].clearPixelData();
 }
 
 size_t ImageDecoder::findRequiredPreviousFrame(size_t frameIndex, bool frameRectIsOpaque)
 {
     ASSERT(frameIndex <= m_frameBufferCache.size());
     if (!frameIndex) {
         // The first frame doesn't rely on any previous data.
@@ skipping 58 matching lines @@
     return m_planes[i];
 }
 
 size_t ImagePlanes::rowBytes(int i) const
 {
     ASSERT((i >= 0) && i < 3);
     return m_rowBytes[i];
 }
 
 } // namespace blink