Bug 1502275 - Skip recreating the decoder after redecode errors if an animated image is reset. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 06 Nov 2018 10:15:07 -0500
changeset 446319 56814f076c2be5b23f8e8fa6f9e8ece64914a201
parent 446318 07332eea29764a5ffe0404ed2d040a843ad13b05
child 446320 219b5cd5af707408a79c0a5b036212f35519a0c9
push id109855
push useraosmond@gmail.com
push dateWed, 14 Nov 2018 16:15:59 +0000
treeherdermozilla-inbound@56814f076c2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1502275
milestone65.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 1502275 - Skip recreating the decoder after redecode errors if an animated image is reset. r=tnikkel Redecode errors break the state machine of FrameAnimator, since the decoder and the animation state are now out of sync. Going forward this tight coupling should be eliminated, as the decoder will produce full frames and the animator can just take the current frame without worrying about its relative position. For the moment, we should just not reset an animation if it hit a redecode error (likely due to OOM) just like how we already stop advancing the animation before the reset. Differential Revision: https://phabricator.services.mozilla.com/D11046
image/AnimationSurfaceProvider.cpp
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -71,17 +71,17 @@ AnimationSurfaceProvider::DropImageRefer
                                     mImage.forget());
 }
 
 void
 AnimationSurfaceProvider::Reset()
 {
   // We want to go back to the beginning.
   bool mayDiscard;
-  bool restartDecoder;
+  bool restartDecoder = false;
 
   {
     MutexAutoLock lock(mFramesMutex);
 
     // If we have not crossed the threshold, we know we haven't discarded any
     // frames, and thus we know it is safe move our display index back to the
     // very beginning. It would be cleaner to let the frame buffer make this
     // decision inside the AnimationFrameBuffer::Reset method, but if we have
@@ -95,22 +95,33 @@ AnimationSurfaceProvider::Reset()
 
   if (mayDiscard) {
     // We are over the threshold and have started discarding old frames. In
     // that case we need to seize the decoding mutex. Thankfully we know that
     // we are in the process of decoding at most the batch size frames, so
     // this should not take too long to acquire.
     MutexAutoLock lock(mDecodingMutex);
 
-    // Recreate the decoder so we can regenerate the frames again.
-    mDecoder = DecoderFactory::CloneAnimationDecoder(mDecoder);
-    MOZ_ASSERT(mDecoder);
+    // We may have hit an error while redecoding. Because FrameAnimator is
+    // tightly coupled to our own state, that means we would need to go through
+    // some heroics to resume animating in those cases. The typical reason for
+    // a redecode to fail is out of memory, and recycling should prevent most of
+    // those errors. When image.animated.generate-full-frames has shipped
+    // enabled on a release or two, we can simply remove the old FrameAnimator
+    // blending code and simplify this quite a bit -- just always pop the next
+    // full frame and timeout off the stack.
+    if (mDecoder) {
+      mDecoder = DecoderFactory::CloneAnimationDecoder(mDecoder);
+      MOZ_ASSERT(mDecoder);
 
-    MutexAutoLock lock2(mFramesMutex);
-    restartDecoder = mFrames->Reset();
+      MutexAutoLock lock2(mFramesMutex);
+      restartDecoder = mFrames->Reset();
+    } else {
+      MOZ_ASSERT(mFrames->HasRedecodeError());
+    }
   }
 
   if (restartDecoder) {
     DecodePool::Singleton()->AsyncRun(this);
   }
 }
 
 void