High CPU and increased memory usage fix for high-res (GIF, WEBP...) animations.

third_party/WebKit/Source/platform/graphics/BitmapImage.cpp

Index: third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp b/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
index 78511f31d29142eb28e38b71f05c9731c1ab2495..53165e91444e0dabffe6c1d35bcf92d0b9734c36 100644
--- a/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
+++ b/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
@@ -57,6 +57,7 @@ PassRefPtr<BitmapImage> BitmapImage::createWithOrientationForTesting(const SkBit
 BitmapImage::BitmapImage(ImageObserver* observer)
     : Image(observer)
     , m_currentFrame(0)
+    , m_cachedFrameIndex(0)
     , m_repetitionCount(cAnimationNone)
     , m_repetitionCountStatus(Unknown)
     , m_repetitionsComplete(0)
@@ -76,6 +77,8 @@ BitmapImage::BitmapImage(const SkBitmap& bitmap, ImageObserver* observer)
     : Image(observer)
     , m_size(bitmap.width(), bitmap.height())
     , m_currentFrame(0)
+    , m_cachedFrame(adoptRef(SkImage::NewFromBitmap(bitmap)))
+    , m_cachedFrameIndex(0)
     , m_repetitionCount(cAnimationNone)
     , m_repetitionCountStatus(Unknown)
     , m_repetitionsComplete(0)
@@ -93,7 +96,6 @@ BitmapImage::BitmapImage(const SkBitmap& bitmap, ImageObserver* observer)
 
     m_frames.grow(1);
     m_frames[0].m_hasAlpha = !bitmap.isOpaque();
-    m_frames[0].m_frame = adoptRef(SkImage::NewFromBitmap(bitmap));
     m_frames[0].m_haveMetadata = true;
 }
 
@@ -107,31 +109,13 @@ bool BitmapImage::currentFrameHasSingleSecurityOrigin() const
     return true;
 }
 
-void BitmapImage::destroyDecodedData(bool destroyAll)
+void BitmapImage::destroyDecodedData()
 {
-    for (size_t i = 0; i < m_frames.size(); ++i) {
-        // The underlying frame isn't actually changing (we're just trying to
-        // save the memory for the framebuffer data), so we don't need to clear
-        // the metadata.
-        m_frames[i].clear(false);
-    }
-
-    m_source.clearCacheExceptFrame(destroyAll ? kNotFound : m_currentFrame);
-    notifyMemoryChanged();
-}
-
-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;
-    size_t allFrameBytes = 0;
+    m_cachedFrame.clear();
     for (size_t i = 0; i < m_frames.size(); ++i)
-        allFrameBytes += m_frames[i].m_frameBytes;
-
-    if (allFrameBytes > cLargeAnimationCutoff) {
-        destroyDecodedData(false);
-    }
+        m_frames[i].clear(true);
+    m_source.clearCacheExceptFrame(kNotFound);
+    notifyMemoryChanged();
 }
 
 void BitmapImage::notifyMemoryChanged()
@@ -149,7 +133,7 @@ size_t BitmapImage::totalFrameBytes()
     return totalBytes;
 }
 
-void BitmapImage::cacheFrame(size_t index)
+PassRefPtr<SkImage> BitmapImage::decodeAndCacheFrame(size_t index)
 {
     size_t numFrames = frameCount();
     if (m_frames.size() < numFrames)
@@ -157,7 +141,9 @@ void BitmapImage::cacheFrame(size_t index)
 
     // We are caching frame snapshots.  This is OK even for partially decoded frames,
     // as they are cleared by dataChanged() when new data arrives.
-    m_frames[index].m_frame = m_source.createFrameAtIndex(index);
+    RefPtr<SkImage> image = m_source.createFrameAtIndex(index);
+    m_cachedFrame = image;
+    m_cachedFrameIndex = index;
 
     m_frames[index].m_orientation = m_source.orientationAtIndex(index);
     m_frames[index].m_haveMetadata = true;
@@ -172,6 +158,7 @@ void BitmapImage::cacheFrame(size_t index)
         m_hasUniformFrameSize = false;
 
     notifyMemoryChanged();
+    return image.release();
 }
 
 void BitmapImage::updateSize() const
@@ -226,8 +213,11 @@ bool BitmapImage::dataChanged(bool allDataReceived)
         // NOTE: Don't call frameIsCompleteAtIndex() here, that will try to
         // decode any uncached (i.e. never-decoded or
         // cleared-on-a-previous-pass) frames!
-        if (m_frames[i].m_haveMetadata && !m_frames[i].m_isComplete)
+        if (m_frames[i].m_haveMetadata && !m_frames[i].m_isComplete) {
             m_frames[i].clear(true);
+            if (i == m_cachedFrameIndex)
+                m_cachedFrame.clear();

[scroggo_chromium, 2016/05/04 21:15:54]

Should we set m_cachedFrameIndex to an invalid value?

[aleksandar.stojiljkovic, 2016/05/04 21:28:07]

No need, once a non null value is assigned to it a valid index will be assigned
to m_cachedFrameIndex.

+        }
     }
 
     // Feed all the data we've seen so far to the image decoder.
@@ -291,7 +281,7 @@ void BitmapImage::draw(SkCanvas* canvas, const SkPaint& paint, const FloatRect&
         WebCoreClampingModeToSkiaRectConstraint(clampMode));
     canvas->restoreToCount(initialSaveCount);
 
-    if (currentFrameIsLazyDecoded())
+    if (image->isLazyGenerated())
         PlatformInstrumentation::didDrawLazyPixelRef(image->uniqueID());
 
     if (ImageObserver* observer = getImageObserver())
@@ -333,23 +323,15 @@ bool BitmapImage::isSizeAvailable()
     return m_sizeAvailable;
 }
 
-bool BitmapImage::ensureFrameIsCached(size_t index)
-{
-    if (index >= frameCount())
-        return false;
-
-    if (index >= m_frames.size() || !m_frames[index].m_frame)
-        cacheFrame(index);
-
-    return true;
-}
-
 PassRefPtr<SkImage> BitmapImage::frameAtIndex(size_t index)
 {
-    if (!ensureFrameIsCached(index))
+    if (index >= frameCount())
         return nullptr;
 
-    return m_frames[index].m_frame;
+    if (index == m_cachedFrameIndex && m_cachedFrame)
+        return m_cachedFrame;

[Peter Kasting, 2016/05/07 01:58:29]

Does this implicitly ref m_cachedFrame and then pass back the new ref?  (I want
to make sure we don't pass back the only ref, and I'm not very familiar with the
Blink smart pointer types.)

[aleksandar.stojiljkovic, 2016/05/07 19:50:53]

Yes, see PasRefPtr.h.
 template <typename U> PassRefPtr(const RefPtr<U>&,
EnsurePtrConvertibleArgDecl(U, T));
It is cheaper than to create RefPtr and call .release but basically the same.
Anyway, this all should go away soon.

+
+    return decodeAndCacheFrame(index);
 }
 
 bool BitmapImage::frameIsCompleteAtIndex(size_t index)
@@ -565,9 +547,7 @@ void BitmapImage::resetAnimation()
     m_repetitionsComplete = 0;
     m_desiredFrameStartTime = 0;
     m_animationFinished = false;
-
-    // For extremely large animations, when the animation is reset, we just throw everything away.
-    destroyDecodedDataIfNecessary();
+    m_cachedFrame.clear();
 }
 
 bool BitmapImage::maybeAnimated()
@@ -625,12 +605,12 @@ bool BitmapImage::internalAdvanceAnimation(bool skippingFrames)
         } else
             m_currentFrame = 0;
     }
-    destroyDecodedDataIfNecessary();
 
     // We need to draw this frame if we advanced to it while not skipping, or if
     // while trying to skip frames we hit the last frame and thus had to stop.
     if (skippingFrames != advancedAnimation)
         getImageObserver()->animationAdvanced(this);
+
     return advancedAnimation;
 }