Issue #2073 - m-c 1343341: Infrastructure necessary to allow discarding of animated images (squashed)

Includes squashed changes of:
 - m-c 1317907: Refactor FrameAnimator::GetCompositedFrame to be a bit simpler
 - m-c 1351434: bugfix
 - m-c 686905: Enable the pref image.mem.animated.discardable to allow discarding of animated images

12 files changed, 363 insertions, 96 deletions

diff --git a/image/DynamicImage.cpp b/image/DynamicImage.cpp
index dfdc3e5d81..bef401a6cb 100644
--- a/image/DynamicImage.cpp
+++ b/image/DynamicImage.cpp
@@ -80,7 +80,7 @@ DynamicImage::OnImageDataComplete(nsIRequest* aRequest,
 }
 
 void
-DynamicImage::OnSurfaceDiscarded()
+DynamicImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey)
 { }
 
 void
diff --git a/image/DynamicImage.h b/image/DynamicImage.h
index 751bed82a1..a39a29b8e3 100644
--- a/image/DynamicImage.h
+++ b/image/DynamicImage.h
@@ -53,7 +53,7 @@ public:
                                        nsresult aStatus,
                                        bool aLastPart) override;
 
-  virtual void OnSurfaceDiscarded() override;
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override;
 
   virtual void SetInnerWindowID(uint64_t aInnerWindowId) override;
   virtual uint64_t InnerWindowID() const override;
diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp
index 8c29b5636a..066da94f40 100644
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -11,6 +11,7 @@
 #include "LookupResult.h"
 #include "MainThreadUtils.h"
 #include "RasterImage.h"
+#include "gfxPrefs.h"
 
 #include "pixman.h"
 
@@ -25,9 +26,80 @@ namespace image {
 ///////////////////////////////////////////////////////////////////////////////
 
 void
-AnimationState::SetDoneDecoding(bool aDone)
+AnimationState::UpdateState(bool aAnimationFinished,
+                            RasterImage *aImage,
+                            const gfx::IntSize& aSize)
 {
-  mDoneDecoding = aDone;
+  LookupResult result =
+    SurfaceCache::Lookup(ImageKey(aImage),
+                         RasterSurfaceKey(aSize,
+                                          DefaultSurfaceFlags(),
+                                          PlaybackType::eAnimated));
+
+  UpdateStateInternal(result, aAnimationFinished);
+}
+
+void
+AnimationState::UpdateStateInternal(LookupResult& aResult,
+                                    bool aAnimationFinished)
+{
+  // Update mDiscarded and mIsCurrentlyDecoded.
+  if (aResult.Type() == MatchType::NOT_FOUND) {
+    // no frames, we've either been discarded, or never been decoded before.
+    mDiscarded = mHasBeenDecoded;
+    mIsCurrentlyDecoded = false;
+  } else if (aResult.Type() == MatchType::PENDING) {
+    // no frames yet, but a decoder is or will be working on it.
+    mDiscarded = false;
+    mIsCurrentlyDecoded = false;
+  } else {
+    MOZ_ASSERT(aResult.Type() == MatchType::EXACT);
+    mDiscarded = false;
+
+    // If mHasBeenDecoded is true then we know the true total frame count and
+    // we can use it to determine if we have all the frames now so we know if
+    // we are currently fully decoded.
+    // If mHasBeenDecoded is false then we'll get another UpdateState call
+    // when the decode finishes.
+    if (mHasBeenDecoded) {
+      Maybe<uint32_t> frameCount = FrameCount();
+      MOZ_ASSERT(frameCount.isSome());
+      aResult.Surface().Seek(*frameCount - 1);
+      if (aResult.Surface() && aResult.Surface()->IsFinished()) {
+        mIsCurrentlyDecoded = true;
+      } else {
+        mIsCurrentlyDecoded = false;
+      }
+    }
+  }
+
+  // Update the value of mCompositedFrameInvalid.
+  if (mIsCurrentlyDecoded || aAnimationFinished) {
+    // Animated images that have finished their animation (ie because it is a
+    // finite length animation) don't have RequestRefresh called on them, and so
+    // mCompositedFrameInvalid would never get cleared. We clear it here (and
+    // also in RasterImage::Decode when we create a decoder for an image that
+    // has finished animated so it can display sooner than waiting until the
+    // decode completes). We also do it if we are fully decoded. This is safe
+    // to do for images that aren't finished animating because before we paint
+    // the refresh driver will call into us to advance to the correct frame,
+    // and that will succeed because we have all the frames.
+    mCompositedFrameInvalid = false;
+  } else if (aResult.Type() == MatchType::NOT_FOUND ||
+             aResult.Type() == MatchType::PENDING) {
+    if (mHasBeenDecoded) {
+      MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+      mCompositedFrameInvalid = true;
+    }
+  }
+  // Otherwise don't change the value of mCompositedFrameInvalid, it will be
+  // updated by RequestRefresh.
+}
+
+void
+AnimationState::NotifyDecodeComplete()
+{
+  mHasBeenDecoded = true;
 }
 
 void
@@ -52,7 +124,7 @@ AnimationState::UpdateKnownFrameCount(uint32_t aFrameCount)
     return;
   }
 
-  MOZ_ASSERT(!mDoneDecoding, "Adding new frames after decoding is finished?");
+  MOZ_ASSERT(!mHasBeenDecoded, "Adding new frames after decoding is finished?");
   MOZ_ASSERT(aFrameCount <= mFrameCount + 1, "Skipped a frame?");
 
   mFrameCount = aFrameCount;
@@ -61,7 +133,7 @@ AnimationState::UpdateKnownFrameCount(uint32_t aFrameCount)
 Maybe<uint32_t>
 AnimationState::FrameCount() const
 {
-  return mDoneDecoding ? Some(mFrameCount) : Nothing();
+  return mHasBeenDecoded ? Some(mFrameCount) : Nothing();
 }
 
 void
@@ -98,7 +170,7 @@ AnimationState::LoopLength() const
     return FrameTimeout::Forever();
   }
 
-  MOZ_ASSERT(mDoneDecoding, "We know the loop length but decoding isn't done?");
+  MOZ_ASSERT(mHasBeenDecoded, "We know the loop length but decoding isn't done?");
 
   // If we're not looping, a single loop time has no meaning.
   if (mAnimationMode != imgIContainer::kNormalAnimMode) {
@@ -113,32 +185,41 @@ AnimationState::LoopLength() const
 // FrameAnimator implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
-TimeStamp
-FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState) const
+Maybe<TimeStamp>
+FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState,
+                                         DrawableSurface& aFrames) const
 {
   TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime;
-  FrameTimeout timeout = GetTimeoutForFrame(aState.mCurrentAnimationFrameIndex);
+  Maybe<FrameTimeout> timeout =
+    GetTimeoutForFrame(aState, aFrames, aState.mCurrentAnimationFrameIndex);
+
+  if (timeout.isNothing()) {
+    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    return Nothing();
+  }
 
-  if (timeout == FrameTimeout::Forever()) {
+  if (*timeout == FrameTimeout::Forever()) {
     // We need to return a sentinel value in this case, because our logic
     // doesn't work correctly if we have an infinitely long timeout. We use one
     // year in the future as the sentinel because it works with the loop in
     // RequestRefresh() below.
     // XXX(seth): It'd be preferable to make our logic work correctly with
     // infinitely long timeouts.
-    return TimeStamp::NowLoRes() +
-           TimeDuration::FromMilliseconds(31536000.0);
+    return Some(TimeStamp::NowLoRes() +
+                TimeDuration::FromMilliseconds(31536000.0));
   }
 
   TimeDuration durationOfTimeout =
-    TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds()));
+    TimeDuration::FromMilliseconds(double(timeout->AsMilliseconds()));
   TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
 
-  return currentFrameEndTime;
+  return Some(currentFrameEndTime);
 }
 
 RefreshResult
-FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
+FrameAnimator::AdvanceFrame(AnimationState& aState,
+                            DrawableSurface& aFrames,
+                            TimeStamp aTime)
 {
   NS_ASSERTION(aTime <= TimeStamp::Now(),
                "Given time appears to be in the future");
@@ -198,7 +279,7 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
   // the appropriate notification on the main thread. Make sure we stay in sync
   // with AnimationState.
   MOZ_ASSERT(nextFrameIndex < aState.KnownFrameCount());
-  RawAccessFrameRef nextFrame = GetRawFrame(nextFrameIndex);
+  RawAccessFrameRef nextFrame = GetRawFrame(aFrames, nextFrameIndex);
 
   // We should always check to see if we have the next frame even if we have
   // previously finished decoding. If we needed to redecode (e.g. due to a draw
@@ -210,7 +291,11 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
     return ret;
   }
 
-  if (GetTimeoutForFrame(nextFrameIndex) == FrameTimeout::Forever()) {
+  Maybe<FrameTimeout> nextFrameTimeout = GetTimeoutForFrame(aState, aFrames, nextFrameIndex);
+  // GetTimeoutForFrame can only return none if frame doesn't exist,
+  // but we just got it above.
+  MOZ_ASSERT(nextFrameTimeout.isSome());
+  if (*nextFrameTimeout == FrameTimeout::Forever()) {
     ret.mAnimationFinished = true;
   }
 
@@ -220,11 +305,13 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
 
     // Change frame
-    if (!DoBlend(&ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) {
+    if (!DoBlend(aFrames, &ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) {
       // something went wrong, move on to next
       NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed");
       nextFrame->SetCompositingFailed(true);
-      aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState);
+      Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState, aFrames);
+      MOZ_ASSERT(currentFrameEndTime.isSome());
+      aState.mCurrentAnimationFrameTime = *currentFrameEndTime;
       aState.mCurrentAnimationFrameIndex = nextFrameIndex;
 
       return ret;
@@ -233,7 +320,9 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
     nextFrame->SetCompositingFailed(false);
   }
 
-  aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState);
+  Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState, aFrames);
+  MOZ_ASSERT(currentFrameEndTime.isSome());
+  aState.mCurrentAnimationFrameTime = *currentFrameEndTime;
 
   // If we can get closer to the current time by a multiple of the image's loop
   // time, we should. We can only do this if we're done decoding; otherwise, we
@@ -262,41 +351,88 @@ FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
 }
 
 RefreshResult
-FrameAnimator::RequestRefresh(AnimationState& aState, const TimeStamp& aTime)
+FrameAnimator::RequestRefresh(AnimationState& aState,
+                              const TimeStamp& aTime,
+                              bool aAnimationFinished)
 {
-  // only advance the frame if the current time is greater than or
-  // equal to the current frame's end time.
-  TimeStamp currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
-
   // By default, an empty RefreshResult.
   RefreshResult ret;
 
-  while (currentFrameEndTime <= aTime) {
-    TimeStamp oldFrameEndTime = currentFrameEndTime;
+  if (aState.IsDiscarded()) {
+    return ret;
+  }
+
+  // Get the animation frames once now, and pass them down to callees because
+  // the surface could be discarded at anytime on a different thread. This is
+  // must easier to reason about then trying to write code that is safe to
+  // having the surface disappear at anytime.
+  LookupResult result =
+    SurfaceCache::Lookup(ImageKey(mImage),
+                         RasterSurfaceKey(mSize,
+                                          DefaultSurfaceFlags(),
+                                          PlaybackType::eAnimated));
+
+  aState.UpdateStateInternal(result, aAnimationFinished);
+  if (aState.IsDiscarded() || !result) {
+    return ret;
+  }
+
+  // only advance the frame if the current time is greater than or
+  // equal to the current frame's end time.
+  Maybe<TimeStamp> currentFrameEndTime =
+    GetCurrentImgFrameEndTime(aState, result.Surface());
+  if (currentFrameEndTime.isNothing()) {
+    MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.mCompositedFrameInvalid);
+    // Nothing we can do but wait for our previous current frame to be decoded
+    // again so we can determine what to do next.
+    return ret;
+  }
+
+  while (*currentFrameEndTime <= aTime) {
+    TimeStamp oldFrameEndTime = *currentFrameEndTime;
 
-    RefreshResult frameRes = AdvanceFrame(aState, aTime);
+    RefreshResult frameRes = AdvanceFrame(aState, result.Surface(), aTime);
 
     // Accumulate our result for returning to callers.
     ret.Accumulate(frameRes);
 
-    currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+    currentFrameEndTime = GetCurrentImgFrameEndTime(aState, result.Surface());
+    // AdvanceFrame can't advance to a frame that doesn't exist yet.
+    MOZ_ASSERT(currentFrameEndTime.isSome());
 
     // If we didn't advance a frame, and our frame end time didn't change,
     // then we need to break out of this loop & wait for the frame(s)
     // to finish downloading.
-    if (!frameRes.mFrameAdvanced && (currentFrameEndTime == oldFrameEndTime)) {
+    if (!frameRes.mFrameAdvanced && (*currentFrameEndTime == oldFrameEndTime)) {
       break;
     }
   }
 
+  // Advanced to the correct frame, the composited frame is now valid to be drawn.
+  if (*currentFrameEndTime > aTime) {
+    aState.mCompositedFrameInvalid = false;
+  }
+
+  MOZ_ASSERT(!aState.mIsCurrentlyDecoded || !aState.mCompositedFrameInvalid);
+
   return ret;
 }
 
 LookupResult
-FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
+FrameAnimator::GetCompositedFrame(AnimationState& aState)
 {
+  if (aState.mCompositedFrameInvalid) {
+    MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+    MOZ_ASSERT(aState.GetHasBeenDecoded());
+    MOZ_ASSERT(!aState.GetIsCurrentlyDecoded());
+    return LookupResult(MatchType::NOT_FOUND);
+  }
+
   // If we have a composited version of this frame, return that.
-  if (mLastCompositedFrameIndex == int32_t(aFrameNum)) {
+  if (mLastCompositedFrameIndex >= 0 &&
+      (uint32_t(mLastCompositedFrameIndex) == aState.mCurrentAnimationFrameIndex)) {
     return LookupResult(DrawableSurface(mCompositingFrame->DrawableRef()),
                         MatchType::EXACT);
   }
@@ -314,7 +450,7 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
 
   // Seek to the appropriate frame. If seeking fails, it means that we couldn't
   // get the frame we're looking for; treat this as if the lookup failed.
-  if (NS_FAILED(result.Surface().Seek(aFrameNum))) {
+  if (NS_FAILED(result.Surface().Seek(aState.mCurrentAnimationFrameIndex))) {
     return LookupResult(MatchType::NOT_FOUND);
   }
 
@@ -324,17 +460,19 @@ FrameAnimator::GetCompositedFrame(uint32_t aFrameNum)
   return result;
 }
 
-FrameTimeout
-FrameAnimator::GetTimeoutForFrame(uint32_t aFrameNum) const
+Maybe<FrameTimeout>
+FrameAnimator::GetTimeoutForFrame(AnimationState& aState,
+                                  DrawableSurface& aFrames,
+                                  uint32_t aFrameNum) const
 {
-  RawAccessFrameRef frame = GetRawFrame(aFrameNum);
+  RawAccessFrameRef frame = GetRawFrame(aFrames, aFrameNum);
   if (frame) {
     AnimationData data = frame->GetAnimationData();
-    return data.mTimeout;
+    return Some(data.mTimeout);
   }
 
-  NS_WARNING("No frame; called GetTimeoutForFrame too early?");
-  return FrameTimeout::FromRawMilliseconds(100);
+  MOZ_ASSERT(aState.mHasBeenDecoded && !aState.mIsCurrentlyDecoded);
+  return Nothing();
 }
 
 static void
@@ -382,37 +520,29 @@ FrameAnimator::CollectSizeOfCompositingSurfaces(
 }
 
 RawAccessFrameRef
-FrameAnimator::GetRawFrame(uint32_t aFrameNum) const
+FrameAnimator::GetRawFrame(DrawableSurface& aFrames, uint32_t aFrameNum) const
 {
-  LookupResult result =
-    SurfaceCache::Lookup(ImageKey(mImage),
-                         RasterSurfaceKey(mSize,
-                                          DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
-  if (!result) {
-    return RawAccessFrameRef();
-  }
-
   // Seek to the frame we want. If seeking fails, it means we couldn't get the
   // frame we're looking for, so we bail here to avoid returning the wrong frame
   // to the caller.
-  if (NS_FAILED(result.Surface().Seek(aFrameNum))) {
+  if (NS_FAILED(aFrames.Seek(aFrameNum))) {
     return RawAccessFrameRef();  // Not available yet.
   }
 
-  return result.Surface()->RawAccessRef();
+  return aFrames->RawAccessRef();
 }
 
 //******************************************************************************
 // DoBlend gets called when the timer for animation get fired and we have to
 // update the composited frame of the animation.
 bool
-FrameAnimator::DoBlend(IntRect* aDirtyRect,
+FrameAnimator::DoBlend(DrawableSurface& aFrames,
+                       IntRect* aDirtyRect,
                        uint32_t aPrevFrameIndex,
                        uint32_t aNextFrameIndex)
 {
-  RawAccessFrameRef prevFrame = GetRawFrame(aPrevFrameIndex);
-  RawAccessFrameRef nextFrame = GetRawFrame(aNextFrameIndex);
+  RawAccessFrameRef prevFrame = GetRawFrame(aFrames, aPrevFrameIndex);
+  RawAccessFrameRef nextFrame = GetRawFrame(aFrames, aNextFrameIndex);
 
   MOZ_ASSERT(prevFrame && nextFrame, "Should have frames here");
 
diff --git a/image/FrameAnimator.h b/image/FrameAnimator.h
index da0cb4bf5a..44b5a52e7c 100644
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -15,11 +15,13 @@
 #include "nsCOMPtr.h"
 #include "nsRect.h"
 #include "SurfaceCache.h"
+#include "gfxPrefs.h"
 
 namespace mozilla {
 namespace image {
 
 class RasterImage;
+class DrawableSurface;
 
 class AnimationState
 {
@@ -31,14 +33,62 @@ public:
     , mLoopCount(-1)
     , mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0))
     , mAnimationMode(aAnimationMode)
-    , mDoneDecoding(false)
+    , mHasBeenDecoded(false)
+    , mIsCurrentlyDecoded(false)
+    , mCompositedFrameInvalid(false)
+    , mDiscarded(false)
   { }
 
   /**
-   * Call when this image is finished decoding so we know that there aren't any
-   * more frames coming.
+   * Call this whenever a decode completes, a decode starts, or the image is
+   * discarded. It will update the internal state. Specifically mDiscarded,
+   * mCompositedFrameInvalid, and mIsCurrentlyDecoded.
    */
-  void SetDoneDecoding(bool aDone);
+  void UpdateState(bool aAnimationFinished,
+                   RasterImage *aImage,
+                   const gfx::IntSize& aSize);
+private:
+  void UpdateStateInternal(LookupResult& aResult,
+                           bool aAnimationFinished);
+
+public:
+  /**
+   * Call when a decode of this image has been completed.
+   */
+  void NotifyDecodeComplete();
+
+  /**
+   * Returns true if this image has been fully decoded before.
+   */
+  bool GetHasBeenDecoded() { return mHasBeenDecoded; }
+
+  /**
+   * Returns true if this image has been discarded and a decoded has not yet
+   * been created to redecode it.
+   */
+  bool IsDiscarded() { return mDiscarded; }
+
+  /**
+   * Sets the composited frame as valid or invalid.
+   */
+  void SetCompositedFrameInvalid(bool aInvalid) {
+    MOZ_ASSERT(!aInvalid || gfxPrefs::ImageMemAnimatedDiscardable());
+    mCompositedFrameInvalid = aInvalid;
+  }
+
+  /**
+   * Returns whether the composited frame is valid to draw to the screen.
+   */
+  bool GetCompositedFrameInvalid() {
+    return mCompositedFrameInvalid;
+  }
+
+  /**
+   * Returns whether the image is currently full decoded..
+   */
+  bool GetIsCurrentlyDecoded() {
+    return mIsCurrentlyDecoded;
+  }
 
   /**
    * Call when you need to re-start animating. Ensures we start from the first
@@ -140,8 +190,44 @@ private:
   //! The animation mode of this image. Constants defined in imgIContainer.
   uint16_t mAnimationMode;
 
-  //! Whether this image is done being decoded.
-  bool mDoneDecoding;
+  /**
+   * The following four bools (mHasBeenDecoded, mIsCurrentlyDecoded,
+   * mCompositedFrameInvalid, mDiscarded) track the state of the image with
+   * regards to decoding. They all start out false, including mDiscarded,
+   * because we want to treat being discarded differently from "not yet decoded
+   * for the first time".
+   *
+   * (When we are decoding the image for the first time we want to show the
+   * image at the speed of data coming in from the network or the speed
+   * specified in the image file, whichever is slower. But when redecoding we
+   * want to show nothing until the frame for the current time has been
+   * decoded. The prevents the user from seeing the image "fast forward"
+   * to the expected spot.)
+   *
+   * When the image is decoded for the first time mHasBeenDecoded and
+   * mIsCurrentlyDecoded get set to true. When the image is discarded
+   * mIsCurrentlyDecoded gets set to false, and mCompositedFrameInvalid
+   * & mDiscarded get set to true. When we create a decoder to redecode the
+   * image mDiscarded gets set to false. mCompositedFrameInvalid gets set to
+   * false when we are able to advance to the frame that should be showing
+   * for the current time. mIsCurrentlyDecoded gets set to true when the
+   * redecode finishes.
+   */
+
+  //! Whether this image has been decoded at least once.
+  bool mHasBeenDecoded;
+
+  //! Whether this image is currently fully decoded.
+  bool mIsCurrentlyDecoded;
+
+  //! Whether the composited frame is valid to draw to the screen, note that
+  //! the composited frame can exist and be filled with image data but not
+  //! valid to draw to the screen.
+  bool mCompositedFrameInvalid;
+
+  //! Whether this image is currently discarded. Only set to true after the
+  //! image has been decoded at least once.
+  bool mDiscarded;
 };
 
 /**
@@ -197,14 +283,16 @@ public:
    * Returns the result of that blending, including whether the current frame
    * changed and what the resulting dirty rectangle is.
    */
-  RefreshResult RequestRefresh(AnimationState& aState, const TimeStamp& aTime);
+  RefreshResult RequestRefresh(AnimationState& aState,
+                               const TimeStamp& aTime,
+                               bool aAnimationFinished);
 
   /**
-   * If we have a composited frame for @aFrameNum, returns it. Otherwise,
-   * returns an empty LookupResult. It is an error to call this method with
-   * aFrameNum == 0, because the first frame is never composited.
+   * Get the full frame for the current frame of the animation (it may or may
+   * not have required compositing). It may not be available because it hasn't
+   * been decoded yet, in which case we return an empty LookupResult.
    */
-  LookupResult GetCompositedFrame(uint32_t aFrameNum);
+  LookupResult GetCompositedFrame(AnimationState& aState);
 
   /**
    * Collect an accounting of the memory occupied by the compositing surfaces we
@@ -227,24 +315,32 @@ private: // methods
    * @returns a RefreshResult that shows whether the frame was successfully
    *          advanced, and its resulting dirty rect.
    */
-  RefreshResult AdvanceFrame(AnimationState& aState, TimeStamp aTime);
+  RefreshResult AdvanceFrame(AnimationState& aState,
+                             DrawableSurface& aFrames,
+                             TimeStamp aTime);
 
   /**
    * Get the @aIndex-th frame in the frame index, ignoring results of blending.
    */
-  RawAccessFrameRef GetRawFrame(uint32_t aFrameNum) const;
+  RawAccessFrameRef GetRawFrame(DrawableSurface& aFrames,
+                                uint32_t aFrameNum) const;
 
-  /// @return the given frame's timeout.
-  FrameTimeout GetTimeoutForFrame(uint32_t aFrameNum) const;
+  /// @return the given frame's timeout if it is available
+  Maybe<FrameTimeout> GetTimeoutForFrame(AnimationState& aState,
+                                         DrawableSurface& aFrames,
+                                         uint32_t aFrameNum) const;
 
   /**
    * Get the time the frame we're currently displaying is supposed to end.
    *
-   * In the error case, returns an "infinity" timestamp.
+   * In the error case (like if the requested frame is not currently
+   * decoded), returns None().
    */
-  TimeStamp GetCurrentImgFrameEndTime(AnimationState& aState) const;
+  Maybe<TimeStamp> GetCurrentImgFrameEndTime(AnimationState& aState,
+                                             DrawableSurface& aFrames) const;
 
-  bool DoBlend(gfx::IntRect* aDirtyRect,
+  bool DoBlend(DrawableSurface& aFrames,
+               gfx::IntRect* aDirtyRect,
                uint32_t aPrevFrameIndex,
                uint32_t aNextFrameIndex);
 
diff --git a/image/Image.h b/image/Image.h
index 98c5e8ca54..4aa9b55afd 100644
--- a/image/Image.h
+++ b/image/Image.h
@@ -211,7 +211,7 @@ public:
   /**
    * Called when the SurfaceCache discards a surface belonging to this image.
    */
-  virtual void OnSurfaceDiscarded() = 0;
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) = 0;
 
   virtual void SetInnerWindowID(uint64_t aInnerWindowId) = 0;
   virtual uint64_t InnerWindowID() const = 0;
@@ -249,7 +249,7 @@ public:
   }
 #endif
 
-  virtual void OnSurfaceDiscarded() override { }
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override { }
 
   virtual void SetInnerWindowID(uint64_t aInnerWindowId) override
   {
diff --git a/image/ImageWrapper.cpp b/image/ImageWrapper.cpp
index 7d2fbfa363..c593521c9e 100644
--- a/image/ImageWrapper.cpp
+++ b/image/ImageWrapper.cpp
@@ -88,9 +88,9 @@ ImageWrapper::OnImageDataComplete(nsIRequest* aRequest,
 }
 
 void
-ImageWrapper::OnSurfaceDiscarded()
+ImageWrapper::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey)
 {
-  return mInnerImage->OnSurfaceDiscarded();
+  return mInnerImage->OnSurfaceDiscarded(aSurfaceKey);
 }
 
 void
diff --git a/image/ImageWrapper.h b/image/ImageWrapper.h
index f60a1c09c3..94cf0948b5 100644
--- a/image/ImageWrapper.h
+++ b/image/ImageWrapper.h
@@ -45,7 +45,7 @@ public:
                                        nsresult aStatus,
                                        bool aLastPart) override;
 
-  virtual void OnSurfaceDiscarded() override;
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override;
 
   virtual void SetInnerWindowID(uint64_t aInnerWindowId) override;
   virtual uint64_t InnerWindowID() const override;
diff --git a/image/RasterImage.cpp b/image/RasterImage.cpp
index 4fd3797bb0..f7dfd0bb1a 100644
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -174,7 +174,7 @@ RasterImage::RequestRefresh(const TimeStamp& aTime)
   RefreshResult res;
   if (mAnimationState) {
     MOZ_ASSERT(mFrameAnimator);
-    res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime);
+    res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime, mAnimationFinished);
   }
 
   if (res.mFrameAdvanced) {
@@ -275,8 +275,7 @@ RasterImage::LookupFrameInternal(const IntSize& aSize,
     MOZ_ASSERT(mFrameAnimator);
     MOZ_ASSERT(ToSurfaceFlags(aFlags) == DefaultSurfaceFlags(),
                "Can't composite frames with non-default surface flags");
-    const size_t index = mAnimationState->GetCurrentAnimationFrameIndex();
-    return mFrameAnimator->GetCompositedFrame(index);
+    return mFrameAnimator->GetCompositedFrame(*mAnimationState);
   }
 
   SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
@@ -332,6 +331,7 @@ RasterImage::LookupFrame(const IntSize& aSize,
     // one. (Or we're sync decoding and the existing decoder hasn't even started
     // yet.) Trigger decoding so it'll be available next time.
     MOZ_ASSERT(aPlaybackType != PlaybackType::eAnimated ||
+               gfxPrefs::ImageMemAnimatedDiscardable() ||
                !mAnimationState || mAnimationState->KnownFrameCount() < 1,
                "Animated frames should be locked");
 
@@ -399,7 +399,7 @@ RasterImage::WillDrawOpaqueNow()
     return false;
   }
 
-  if (mAnimationState) {
+  if (mAnimationState && !gfxPrefs::ImageMemAnimatedDiscardable()) {
     // We never discard frames of animated images.
     return true;
   }
@@ -426,11 +426,33 @@ RasterImage::WillDrawOpaqueNow()
 }
 
 void
-RasterImage::OnSurfaceDiscarded()
+RasterImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey)
 {
   MOZ_ASSERT(mProgressTracker);
 
-  NS_DispatchToMainThread(NewRunnableMethod(mProgressTracker, &ProgressTracker::OnDiscard));
+  bool animatedFramesDiscarded =
+    mAnimationState && aSurfaceKey.Playback() == PlaybackType::eAnimated;
+
+  RefPtr<RasterImage> image = this;
+  NS_DispatchToMainThread(NS_NewRunnableFunction(
+                            [=]() -> void {
+    image->OnSurfaceDiscardedInternal(animatedFramesDiscarded);
+  }));
+}
+
+void
+RasterImage::OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (aAnimatedFramesDiscarded && mAnimationState) {
+    MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+  }
+
+  if (mProgressTracker) {
+    mProgressTracker->OnDiscard();
+  }
 }
 
 //******************************************************************************
@@ -706,9 +728,11 @@ RasterImage::SetMetadata(const ImageMetadata& aMetadata,
     mAnimationState.emplace(mAnimationMode);
     mFrameAnimator = MakeUnique<FrameAnimator>(this, mSize);
 
-    // We don't support discarding animated images (See bug 414259).
-    // Lock the image and throw away the key.
-    LockImage();
+    if (!gfxPrefs::ImageMemAnimatedDiscardable()) {
+      // We don't support discarding animated images (See bug 414259).
+      // Lock the image and throw away the key.
+      LockImage();
+    }
 
     if (!aFromMetadataDecode) {
       // The metadata decode reported that this image isn't animated, but we
@@ -1015,11 +1039,16 @@ RasterImage::Discard()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(CanDiscard(), "Asked to discard but can't");
-  MOZ_ASSERT(!mAnimationState, "Asked to discard for animated image");
+  MOZ_ASSERT(!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable(),
+    "Asked to discard for animated image");
 
   // Delete all the decoded frames.
   SurfaceCache::RemoveImage(ImageKey(this));
 
+  if (mAnimationState) {
+    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+  }
+
   // Notify that we discarded.
   if (mProgressTracker) {
     mProgressTracker->OnDiscard();
@@ -1028,8 +1057,8 @@ RasterImage::Discard()
 
 bool
 RasterImage::CanDiscard() {
-  return mHasSourceData &&       // ...have the source data...
-         !mAnimationState;       // Can never discard animated images
+  return mHasSourceData &&                                                 // ...have the source data...
+         (!mAnimationState || gfxPrefs::ImageMemAnimatedDiscardable());    // Can discard animated images if the pref is set
 }
 
 NS_IMETHODIMP
@@ -1158,6 +1187,13 @@ RasterImage::Decode(const IntSize& aSize,
     task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this),
                                                   mSourceBuffer, mSize,
                                                   decoderFlags, surfaceFlags);
+    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+    // If the animation is finished we can draw right away because we just draw
+    // the final frame all the time from now on. See comment in
+    // AnimationState::UpdateState.
+    if (mAnimationFinished) {
+      mAnimationState->SetCompositedFrameInvalid(false);
+    }
   } else {
     task = DecoderFactory::CreateDecoder(mDecoderType, WrapNotNull(this),
                                          mSourceBuffer, mSize, aSize,
@@ -1597,7 +1633,8 @@ RasterImage::NotifyDecodeComplete(const DecoderFinalStatus& aStatus,
       mHasBeenDecoded && mAnimationState) {
     // We've finished a full decode of all animation frames and our AnimationState
     // has been notified about them all, so let it know not to expect anymore.
-    mAnimationState->SetDoneDecoding(true);
+    mAnimationState->NotifyDecodeComplete();
+    mAnimationState->UpdateState(mAnimationFinished, this, mSize);
   }
 
   // Only act on errors if we have no usable frames from the decoder.
diff --git a/image/RasterImage.h b/image/RasterImage.h
index 860983b22e..42af0a4887 100644
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -163,7 +163,7 @@ public:
   virtual nsresult StopAnimation() override;
 
   // Methods inherited from Image
-  virtual void OnSurfaceDiscarded() override;
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override;
 
   virtual size_t SizeOfSourceWithComputedFallback(MallocSizeOf aMallocSizeOf)
     const override;
@@ -321,7 +321,9 @@ private:
   // never unlock so that animated images always have their lock count >= 1. In
   // that case we use our animation consumers count as a proxy for lock count.
   bool IsUnlocked() {
-    return (mLockCount == 0 || (mAnimationState && mAnimationConsumers == 0));
+    return (mLockCount == 0 ||
+            (!gfxPrefs::ImageMemAnimatedDiscardable() &&
+             (mAnimationState && mAnimationConsumers == 0)));
   }
 
 
@@ -378,6 +380,8 @@ private:
    */
   void RecoverFromInvalidFrames(const nsIntSize& aSize, uint32_t aFlags);
 
+  void OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded);
+
 private: // data
   nsIntSize                  mSize;
   Orientation                mOrientation;
diff --git a/image/SurfaceCache.cpp b/image/SurfaceCache.cpp
index 66fdfcca04..f3068fd4c1 100644
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -503,7 +503,7 @@ public:
 
     // If the surface was not a placeholder, tell its image that we discarded it.
     if (!aSurface->IsPlaceholder()) {
-      static_cast<Image*>(imageKey)->OnSurfaceDiscarded();
+      static_cast<Image*>(imageKey)->OnSurfaceDiscarded(aSurface->GetSurfaceKey());
     }
 
     StopTracking(aSurface, aAutoLock);
diff --git a/image/VectorImage.cpp b/image/VectorImage.cpp
index 59b31be47e..fb56c4b662 100644
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -1113,7 +1113,7 @@ VectorImage::RequestDiscard()
 }
 
 void
-VectorImage::OnSurfaceDiscarded()
+VectorImage::OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey)
 {
   MOZ_ASSERT(mProgressTracker);
 
diff --git a/image/VectorImage.h b/image/VectorImage.h
index bd4d393ed4..471ac7df1f 100644
--- a/image/VectorImage.h
+++ b/image/VectorImage.h
@@ -49,7 +49,7 @@ public:
                                        nsresult aResult,
                                        bool aLastPart) override;
 
-  virtual void OnSurfaceDiscarded() override;
+  virtual void OnSurfaceDiscarded(const SurfaceKey& aSurfaceKey) override;
 
   /**
    * Callback for SVGRootRenderingObserver.