Bug 1363092. Don't update the state of an animated image that requires an invalidation when creating a new decoder because we may not be able to send invalidations. r=aosmond

image/FrameAnimator.cpp

@@ -30,7 +30,8 @@ namespace image {
 const gfx::IntRect
 AnimationState::UpdateState(bool aAnimationFinished,
                             RasterImage *aImage,
-                            const gfx::IntSize& aSize)
+                            const gfx::IntSize& aSize,
+                            bool aAllowInvalidation /* = true */)
 {
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(aImage),
@@ -44,7 +45,8 @@ AnimationState::UpdateState(bool aAnimationFinished,
 const gfx::IntRect
 AnimationState::UpdateStateInternal(LookupResult& aResult,
                                     bool aAnimationFinished,
-                                    const gfx::IntSize& aSize)
+                                    const gfx::IntSize& aSize,
+                                    bool aAllowInvalidation /* = true */)
 {
   // Update mDiscarded and mIsCurrentlyDecoded.
   if (aResult.Type() == MatchType::NOT_FOUND) {
@@ -78,31 +80,33 @@ AnimationState::UpdateStateInternal(LookupResult& aResult,
 
   gfx::IntRect ret;
 
-  // Update the value of mCompositedFrameInvalid.
+  if (aAllowInvalidation) {
-  if (mIsCurrentlyDecoded || aAnimationFinished) {
+    // Update the value of mCompositedFrameInvalid.
-    // Animated images that have finished their animation (ie because it is a
+    if (mIsCurrentlyDecoded || aAnimationFinished) {
-    // finite length animation) don't have RequestRefresh called on them, and so
+      // Animated images that have finished their animation (ie because it is a
-    // mCompositedFrameInvalid would never get cleared. We clear it here (and
+      // finite length animation) don't have RequestRefresh called on them, and so
-    // also in RasterImage::Decode when we create a decoder for an image that
+      // mCompositedFrameInvalid would never get cleared. We clear it here (and
-    // has finished animated so it can display sooner than waiting until the
+      // also in RasterImage::Decode when we create a decoder for an image that
-    // decode completes). We also do it if we are fully decoded. This is safe
+      // has finished animated so it can display sooner than waiting until the
-    // to do for images that aren't finished animating because before we paint
+      // decode completes). We also do it if we are fully decoded. This is safe
-    // the refresh driver will call into us to advance to the correct frame,
+      // to do for images that aren't finished animating because before we paint
-    // and that will succeed because we have all the frames.
+      // the refresh driver will call into us to advance to the correct frame,
-    if (mCompositedFrameInvalid) {
+      // and that will succeed because we have all the frames.
-      // Invalidate if we are marking the composited frame valid.
+      if (mCompositedFrameInvalid) {
-      ret.SizeTo(aSize);
+        // Invalidate if we are marking the composited frame valid.
-    }
+        ret.SizeTo(aSize);
-    mCompositedFrameInvalid = false;
+      }
-  } else if (aResult.Type() == MatchType::NOT_FOUND ||
+      mCompositedFrameInvalid = false;
-             aResult.Type() == MatchType::PENDING) {
+    } else if (aResult.Type() == MatchType::NOT_FOUND ||
-    if (mHasBeenDecoded) {
+               aResult.Type() == MatchType::PENDING) {
-      MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+      if (mHasBeenDecoded) {
-      mCompositedFrameInvalid = true;
+        MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+        mCompositedFrameInvalid = true;
+      }
     }
+    // Otherwise don't change the value of mCompositedFrameInvalid, it will be
+    // updated by RequestRefresh.
   }
-  // Otherwise don't change the value of mCompositedFrameInvalid, it will be
-  // updated by RequestRefresh.
 
   return ret;
 }

image/FrameAnimator.h

@@ -42,16 +42,18 @@ public:
   /**
    * 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. Returns a rect to
+   * mCompositedFrameInvalid, and mIsCurrentlyDecoded. If aAllowInvalidation
-   * invalidate.
+   * is true then returns a rect to invalidate.
    */
   const gfx::IntRect UpdateState(bool aAnimationFinished,
                             RasterImage *aImage,
-                            const gfx::IntSize& aSize);
+                            const gfx::IntSize& aSize,
+                            bool aAllowInvalidation = true);
 private:
   const gfx::IntRect UpdateStateInternal(LookupResult& aResult,
                                     bool aAnimationFinished,
-                                    const gfx::IntSize& aSize);
+                                    const gfx::IntSize& aSize,
+                                    bool aAllowInvalidation = true);
 
 public:
   /**

image/RasterImage.cpp

@@ -1261,20 +1261,13 @@ RasterImage::Decode(const IntSize& aSize,
     task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this),
                                                   mSourceBuffer, mSize,
                                                   decoderFlags, surfaceFlags);
-    // We may not be able to send an invalidation right here because of async
+    // We pass false for aAllowInvalidation because we may be asked to use
-    // notifications but that shouldn't be a problem because we shouldn't be
+    // async notifications. Any potential invalidation here will be sent when
-    // getting a non-empty rect back from UpdateState. This is because UpdateState
+    // RequestRefresh is called, or NotifyDecodeComplete.
-    // will only return a non-empty rect if we are currently decoded, or the
-    // animation is finished. We can't be decoded because we are creating a decoder
-    // here. If the animation is finished then the composited frame would have
-    // been valid when the animation finished, and it's not possible to mark
-    // the composited frame as invalid when the animation is finished. So
-    // the composited frame can't change from invalid to valid in this UpdateState
-    // call, and hence no rect can be returned.
 #ifdef DEBUG
     gfx::IntRect rect =
 #endif
-      mAnimationState->UpdateState(mAnimationFinished, this, mSize);
+      mAnimationState->UpdateState(mAnimationFinished, this, mSize, false);
     MOZ_ASSERT(rect.IsEmpty());
   } else {
     task = DecoderFactory::CreateDecoder(mDecoderType, WrapNotNull(this),