Bug 1629490. The composited frame is now invalid for animated images that have finished. r=aosmond

This is actually a one line change. The rest is just removing unneeded parameters.

The composited frame used to be valid for animations that had finished because we saved a separate composited frame, and since the animation had finished it was the final frame, so it had to be valid to display.

Now we don't have a separate composited frame so we only have a valid frame to display if we've decoded up to the current frame. For non-finished animations RequestRefresh will clear mCompositedFrameInvalid, but for finished animations we have to clear it, the easiest place is when we know we are fully decoded.

This problem caused img.decode to never fulfill or reject because we never sent out any frame update notifications because mCompositedFrameInvalid was always false because mAnimationFinished was always true. So we didn't get the invalidation that flipping mCompositedFrameInvalid to false when a full decode finished would cause.

Differential Revision: https://phabricator.services.mozilla.com/D70838

image/FrameAnimator.cpp

@@ -24,19 +24,18 @@ namespace image {
 ///////////////////////////////////////////////////////////////////////////////
 
 const gfx::IntRect AnimationState::UpdateState(
-    bool aAnimationFinished, RasterImage* aImage, const gfx::IntSize& aSize,
+    RasterImage* aImage, const gfx::IntSize& aSize,
     bool aAllowInvalidation /* = true */) {
   LookupResult result = SurfaceCache::Lookup(
       ImageKey(aImage),
       RasterSurfaceKey(aSize, DefaultSurfaceFlags(), PlaybackType::eAnimated),
       /* aMarkUsed = */ false);
 
-  return UpdateStateInternal(result, aAnimationFinished, aSize,
-                             aAllowInvalidation);
+  return UpdateStateInternal(result, aSize, aAllowInvalidation);
 }
 
 const gfx::IntRect AnimationState::UpdateStateInternal(
-    LookupResult& aResult, bool aAnimationFinished, const gfx::IntSize& aSize,
+    LookupResult& aResult, const gfx::IntSize& aSize,
     bool aAllowInvalidation /* = true */) {
   // Update mDiscarded and mIsCurrentlyDecoded.
   if (aResult.Type() == MatchType::NOT_FOUND) {
@@ -69,16 +68,11 @@ const gfx::IntRect AnimationState::UpdateStateInternal(
 
   if (aAllowInvalidation) {
     // 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.
+    if (mIsCurrentlyDecoded) {
+      // It is safe to clear mCompositedFrameInvalid safe to do for images that
+      // are fully decoded but 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.
       if (mCompositedFrameInvalid) {
         // Invalidate if we are marking the composited frame valid.
         ret.SizeTo(aSize);
@@ -345,8 +339,7 @@ void FrameAnimator::ResetAnimation(AnimationState& aState) {
 }
 
 RefreshResult FrameAnimator::RequestRefresh(AnimationState& aState,
-                                            const TimeStamp& aTime,
-                                            bool aAnimationFinished) {
+                                            const TimeStamp& aTime) {
   // By default, an empty RefreshResult.
   RefreshResult ret;
 
@@ -364,8 +357,7 @@ RefreshResult FrameAnimator::RequestRefresh(AnimationState& aState,
       RasterSurfaceKey(mSize, DefaultSurfaceFlags(), PlaybackType::eAnimated),
       /* aMarkUsed = */ true);
 
-  ret.mDirtyRect =
-      aState.UpdateStateInternal(result, aAnimationFinished, mSize);
+  ret.mDirtyRect = aState.UpdateStateInternal(result, mSize);
   if (aState.IsDiscarded() || !result) {
     aState.MaybeAdvanceAnimationFrameTime(aTime);
     if (!ret.mDirtyRect.IsEmpty()) {

image/FrameAnimator.h

@@ -44,13 +44,11 @@ class AnimationState {
    * mCompositedFrameInvalid, and mIsCurrentlyDecoded. If aAllowInvalidation
    * is true then returns a rect to invalidate.
    */
-  const gfx::IntRect UpdateState(bool aAnimationFinished, RasterImage* aImage,
-                                 const gfx::IntSize& aSize,
+  const gfx::IntRect UpdateState(RasterImage* aImage, const gfx::IntSize& aSize,
                                  bool aAllowInvalidation = true);
 
  private:
   const gfx::IntRect UpdateStateInternal(LookupResult& aResult,
-                                         bool aAnimationFinished,
                                          const gfx::IntSize& aSize,
                                          bool aAllowInvalidation = true);
 
@@ -298,8 +296,7 @@ class FrameAnimator {
    * 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,
-                               bool aAnimationFinished);
+  RefreshResult RequestRefresh(AnimationState& aState, const TimeStamp& aTime);
 
   /**
    * Get the full frame for the current frame of the animation (it may or may

image/RasterImage.cpp

@@ -163,8 +163,7 @@ RasterImage::RequestRefresh(const TimeStamp& aTime) {
   RefreshResult res;
   if (mAnimationState) {
     MOZ_ASSERT(mFrameAnimator);
-    res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime,
-                                         mAnimationFinished);
+    res = mFrameAnimator->RequestRefresh(*mAnimationState, aTime);
   }
 
   if (res.mFrameAdvanced) {
@@ -489,8 +488,7 @@ void RasterImage::OnSurfaceDiscardedInternal(bool aAnimatedFramesDiscarded) {
     ReleaseImageContainer();
 
     auto size = ToUnoriented(mSize);
-    IntRect rect = mAnimationState->UpdateState(mAnimationFinished, this,
-                                                size.ToUnknownSize());
+    IntRect rect = mAnimationState->UpdateState(this, size.ToUnknownSize());
 
     auto dirtyRect = UnorientedIntRect::FromUnknownRect(rect);
     NotifyProgress(NoProgress, dirtyRect);
@@ -1041,8 +1039,7 @@ void RasterImage::Discard() {
     ReleaseImageContainer();
 
     auto size = ToUnoriented(mSize);
-    IntRect rect = mAnimationState->UpdateState(mAnimationFinished, this,
-                                                size.ToUnknownSize());
+    IntRect rect = mAnimationState->UpdateState(this, size.ToUnknownSize());
 
     auto dirtyRect = UnorientedIntRect::FromUnknownRect(rect);
     NotifyProgress(NoProgress, dirtyRect);
@@ -1259,8 +1256,7 @@ bool RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
 #ifdef DEBUG
     IntRect rect =
 #endif
-        mAnimationState->UpdateState(mAnimationFinished, this,
-                                     ToUnoriented(mSize).ToUnknownSize(),
+        mAnimationState->UpdateState(this, ToUnoriented(mSize).ToUnknownSize(),
                                      false);
     MOZ_ASSERT(rect.IsEmpty());
   }
@@ -1709,8 +1705,7 @@ void RasterImage::NotifyDecodeComplete(
     mAnimationState->NotifyDecodeComplete();
 
     auto size = ToUnoriented(mSize);
-    IntRect rect = mAnimationState->UpdateState(mAnimationFinished, this,
-                                                size.ToUnknownSize());
+    IntRect rect = mAnimationState->UpdateState(this, size.ToUnknownSize());
 
     if (!rect.IsEmpty()) {
       auto dirtyRect = UnorientedIntRect::FromUnknownRect(rect);