Bug 1388733 - Ensure animations resume when the image surfaces are discarded while still decoding. r=tnikkel

When an animated image has been discarded, we avoided marking the
composited frame invalid unless it had been previously decoded. Most of
the time this was fine, but if the animated image was still decoding for
the first time, then we still had a composited frame lingering that we
did not mark as invalid. As a result, when we called
RasterImage::LookupFrame (and indirectly
FrameAnimator::GetCompositedFrame), it would always return the
composited frame. This meant that RasterImage::Decode would never be
called to trigger a redecode. At the same time,
FrameAnimator::RequestRefresh would not cause us to advance the frame
because the state was still discarded.

With this patch we separate out the concepts of "has ever requested to
be decoded" and "has ever completed decoding." The former is now used to
control whether or not a composited frame is marked as invalid after we
discover we currently have no surface for the animation -- this solves
the animation remaining frozen as we now request the redecode as
expected. The latter remains used to determine if we actually know the
total number of frames.

image/FrameAnimator.cpp

@@ -57,9 +57,11 @@ AnimationState::UpdateStateInternal(LookupResult& aResult,
     // no frames yet, but a decoder is or will be working on it.
     mDiscarded = false;
     mIsCurrentlyDecoded = false;
+    mHasRequestedDecode = true;
   } else {
     MOZ_ASSERT(aResult.Type() == MatchType::EXACT);
     mDiscarded = false;
+    mHasRequestedDecode = true;
 
     // 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
@@ -99,7 +101,7 @@ AnimationState::UpdateStateInternal(LookupResult& aResult,
       mCompositedFrameInvalid = false;
     } else if (aResult.Type() == MatchType::NOT_FOUND ||
                aResult.Type() == MatchType::PENDING) {
-      if (mHasBeenDecoded) {
+      if (mHasRequestedDecode) {
         MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
         mCompositedFrameInvalid = true;
       }
@@ -209,7 +211,7 @@ FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState,
     GetTimeoutForFrame(aState, aFrames, aState.mCurrentAnimationFrameIndex);
 
   if (timeout.isNothing()) {
-    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode() && !aState.GetIsCurrentlyDecoded());
     return Nothing();
   }
 
@@ -416,7 +418,7 @@ FrameAnimator::RequestRefresh(AnimationState& aState,
     GetCurrentImgFrameEndTime(aState, result.Surface());
   if (currentFrameEndTime.isNothing()) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
-    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode() && !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.
@@ -465,7 +467,7 @@ FrameAnimator::GetCompositedFrame(AnimationState& aState)
 
   if (aState.mCompositedFrameInvalid) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
-    MOZ_ASSERT(aState.GetHasBeenDecoded());
+    MOZ_ASSERT(aState.GetHasRequestedDecode());
     MOZ_ASSERT(!aState.GetIsCurrentlyDecoded());
     if (result.Type() == MatchType::NOT_FOUND) {
       return result;
@@ -512,7 +514,7 @@ FrameAnimator::GetTimeoutForFrame(AnimationState& aState,
     return Some(data.mTimeout);
   }
 
-  MOZ_ASSERT(aState.mHasBeenDecoded && !aState.mIsCurrentlyDecoded);
+  MOZ_ASSERT(aState.mHasRequestedDecode && !aState.mIsCurrentlyDecoded);
   return Nothing();
 }
 

image/FrameAnimator.h

@@ -34,6 +34,7 @@ public:
     , mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0))
     , mAnimationMode(aAnimationMode)
     , mHasBeenDecoded(false)
+    , mHasRequestedDecode(false)
     , mIsCurrentlyDecoded(false)
     , mCompositedFrameInvalid(false)
     , mDiscarded(false)
@@ -66,6 +67,11 @@ public:
    */
   bool GetHasBeenDecoded() { return mHasBeenDecoded; }
 
+  /**
+   * Returns true if this image has ever requested a decode before.
+   */
+  bool GetHasRequestedDecode() { return mHasRequestedDecode; }
+
   /**
    * Returns true if this image has been discarded and a decoded has not yet
    * been created to redecode it.
@@ -221,6 +227,9 @@ private:
   //! Whether this image has been decoded at least once.
   bool mHasBeenDecoded;
 
+  //! Whether this image has ever requested a decode.
+  bool mHasRequestedDecode;
+
   //! Whether this image is currently fully decoded.
   bool mIsCurrentlyDecoded;