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

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

Index: third_party/WebKit/Source/platform/graphics/BitmapImage.h
diff --git a/third_party/WebKit/Source/platform/graphics/BitmapImage.h b/third_party/WebKit/Source/platform/graphics/BitmapImage.h
index 2a650a893f4364f6de016512b4c67a9031db798d..1b88a2b604516f1ca0027cd707fc4b74bc784da6 100644
--- a/third_party/WebKit/Source/platform/graphics/BitmapImage.h
+++ b/third_party/WebKit/Source/platform/graphics/BitmapImage.h
@@ -44,8 +44,9 @@ namespace blink {
 template <typename T> class Timer;
 
 class PLATFORM_EXPORT BitmapImage final : public Image {
-    friend class GeneratedImage;
+    friend class BitmapImageTest;
     friend class CrossfadeGeneratedImage;
+    friend class GeneratedImage;
     friend class GradientGeneratedImage;
     friend class GraphicsContext;
 public:
@@ -65,15 +66,6 @@ public:
     bool getHotSpot(IntPoint&) const override;
     String filenameExtension() const override;
     bool dataChanged(bool allDataReceived) override;
-
-    bool isAllDataReceived() const { return m_allDataReceived; }
-    bool hasColorProfile() const;
-
-    // It may look unusual that there's no start animation call as public API.
-    // This because we start and stop animating lazily. Animation starts when
-    // the image is rendered, and automatically pauses once all observers no
-    // longer want to render the image.
-    void stopAnimation() override;
     void resetAnimation() override;
     bool maybeAnimated() override;
 
@@ -88,6 +80,13 @@ public:
     bool currentFrameIsComplete() override;
     bool currentFrameIsLazyDecoded() override;
 
+    // Called to wipe out the entire frame buffer cache and tell the image
+    // source to destroy everything; this is used when e.g. we want to free
+    // some room in the image cache.
+    void destroyDecodedData() override;
+
+    bool isAllDataReceived() const { return m_allDataReceived; }

[Peter Kasting, 2016/05/09 22:31:25]

Nit: When you move the declarations of functions, you need to reorder their
definitions in the .cpp file to match in the same CL.  If you don't want to do
that in this CL it's fine to do separately, but in that case don't move the
declarations in the .h file around either.

[aleksandar.stojiljkovic, 2016/05/11 10:33:14]

Difficult as there is already missmatch in order of methods in cph and header
file. I merged 2 private sections, kept the order in cpp file and reordered
change methods in header file. Full reordering is good to do after refactoring -
the plan is to remove FrameData usage in BitmapImage.

+    bool hasColorProfile() const;
     ImageOrientation currentFrameOrientation();
 
     // Construct a BitmapImage with the given orientation.
@@ -96,11 +95,6 @@ public:
     void advanceAnimationForTesting() override { internalAdvanceAnimation(false); }
 
 private:
-    friend class BitmapImageTest;
-
-    void updateSize() const;
-
-private:
     enum RepetitionCountStatus {
       Unknown,    // We haven't checked the source's repetition count.
       Uncertain,  // We have a repetition count, but it might be wrong (some GIFs have a count after the image data, and will report "loop once" until all data has been decoded).
@@ -112,6 +106,7 @@ private:
 
     void draw(SkCanvas*, const SkPaint&, const FloatRect& dstRect, const FloatRect& srcRect, RespectImageOrientationEnum, ImageClampingMode) override;
 
+    void updateSize() const;
     size_t currentFrame() const { return m_currentFrame; }
     size_t frameCount();
 
@@ -122,28 +117,12 @@ private:
     bool frameHasAlphaAtIndex(size_t);
     ImageOrientation frameOrientationAtIndex(size_t);
 
-    // Decodes and caches a frame. Never accessed except internally.
-    void cacheFrame(size_t index);
-
-    // Called before accessing m_frames[index]. Returns false on index out of bounds.
-    bool ensureFrameIsCached(size_t index);
+    PassRefPtr<SkImage> decodeAndCacheFrame(size_t index);
 
     // Returns the total number of bytes allocated for all framebuffers, i.e.
     // the sum of m_source.frameBytesAtIndex(...) for all frames.
     size_t totalFrameBytes();
 
-    // Called to invalidate cached data. When |destroyAll| is true, we wipe out
-    // the entire frame buffer cache and tell the image source to destroy
-    // everything; this is used when e.g. we want to free some room in the image
-    // cache. If |destroyAll| is false, we delete frames except the current
-    // frame; this is used while animating large images to keep memory footprint
-    // low; the decoder should preserve the current frame and may preserve some
-    // other frames to avoid redecoding the whole image on every frame.
-    void destroyDecodedData(bool destroyAll) override;
-
-    // If the image is large enough, calls destroyDecodedData().
-    void destroyDecodedDataIfNecessary();
-
     // Notifies observers that the memory footprint has changed.
     void notifyMemoryChanged();
 
@@ -156,6 +135,11 @@ private:
     void startAnimation(CatchUpAnimation = CatchUp) override;
     void advanceAnimation(Timer<BitmapImage>*);
 
+    // There's no start animation call because we start and stop animating

[Peter Kasting, 2016/05/09 22:31:25]

Eh?  startAnimation() is just a couple lines above this!

I would remove this comment entirely, along with the blank line above it; or
else maybe move the last sentence up to after "// Animation." above.

[aleksandar.stojiljkovic, 2016/05/11 10:33:14]

Done.

+    // lazily.  Animation starts when the image is rendered, and automatically
+    // pauses once no observer wants to render the image.
+    void stopAnimation();
+
     // Function that does the real work of advancing the animation.  When
     // skippingFrames is true, we're in the middle of a loop trying to skip over
     // a bunch of animation frames, so we should not do things like decode each
@@ -170,6 +154,9 @@ private:
     size_t m_currentFrame; // The index of the current frame of animation.
     Vector<FrameData, 1> m_frames; // An array of the cached frames of the animation. We have to ref frames to pin them in the cache.
 
+    RefPtr<SkImage> m_cachedFrame; // A cached copy of the most recently-accessed frame.
+    size_t m_cachedFrameIndex; // Index of the frame that is cached.
+
     OwnPtr<Timer<BitmapImage>> m_frameTimer;
     int m_repetitionCount; // How many total animation loops we should do.  This will be cAnimationNone if this image type is incapable of animation.
     RepetitionCountStatus m_repetitionCountStatus;