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
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 01 Jun 2017 02:19:55 -0500
changeset 361789 9d8e1c9e19e009ec64bbae332a78b87c5acd7150
parent 361788 4e8564ce92d8056d6282a499cac1933f4a90a769
child 361790 4507e6e48d484390f59047e6d9ecfba6662457c7
push id31943
push userryanvm@gmail.com
push dateThu, 01 Jun 2017 15:54:45 +0000
treeherdermozilla-central@62005e6aecdf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1363092
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
image/FrameAnimator.h
image/RasterImage.cpp
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -25,31 +25,33 @@ namespace image {
 
 ///////////////////////////////////////////////////////////////////////////////
 // AnimationState implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
 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),
                          RasterSurfaceKey(aSize,
                                           DefaultSurfaceFlags(),
                                           PlaybackType::eAnimated));
 
   return UpdateStateInternal(result, aAnimationFinished, aSize);
 }
 
 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) {
     // 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.
@@ -73,41 +75,43 @@ AnimationState::UpdateStateInternal(Look
       } else {
         mIsCurrentlyDecoded = false;
       }
     }
   }
 
   gfx::IntRect ret;
 
-  // 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 (mCompositedFrameInvalid) {
-      // Invalidate if we are marking the composited frame valid.
-      ret.SizeTo(aSize);
+  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 (mCompositedFrameInvalid) {
+        // Invalidate if we are marking the composited frame valid.
+        ret.SizeTo(aSize);
+      }
+      mCompositedFrameInvalid = false;
+    } else if (aResult.Type() == MatchType::NOT_FOUND ||
+               aResult.Type() == MatchType::PENDING) {
+      if (mHasBeenDecoded) {
+        MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+        mCompositedFrameInvalid = true;
+      }
     }
-    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.
   }
-  // Otherwise don't change the value of mCompositedFrameInvalid, it will be
-  // updated by RequestRefresh.
 
   return ret;
 }
 
 void
 AnimationState::NotifyDecodeComplete()
 {
   mHasBeenDecoded = true;
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -37,26 +37,28 @@ public:
     , mIsCurrentlyDecoded(false)
     , mCompositedFrameInvalid(false)
     , mDiscarded(false)
   { }
 
   /**
    * 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
-   * invalidate.
+   * 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::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:
   /**
    * Call when a decode of this image has been completed.
    */
   void NotifyDecodeComplete();
 
   /**
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1256,30 +1256,23 @@ RasterImage::Decode(const IntSize& aSize
   }
 
   // Create a decoder.
   RefPtr<IDecodingTask> task;
   if (mAnimationState && aPlaybackType == PlaybackType::eAnimated) {
     task = DecoderFactory::CreateAnimationDecoder(mDecoderType, WrapNotNull(this),
                                                   mSourceBuffer, mSize,
                                                   decoderFlags, surfaceFlags);
-    // We may not be able to send an invalidation right here because of async
-    // notifications but that shouldn't be a problem because we shouldn't be
-    // getting a non-empty rect back from UpdateState. This is because UpdateState
-    // 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.
+    // We pass false for aAllowInvalidation because we may be asked to use
+    // async notifications. Any potential invalidation here will be sent when
+    // RequestRefresh is called, or NotifyDecodeComplete.
 #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),
                                          mSourceBuffer, mSize, aSize,
                                          decoderFlags, surfaceFlags);
   }
 
   // Make sure DecoderFactory was able to create a decoder successfully.