Bug 1444537 - Part 1. Ensure discarding of animated image frames is clear. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 27 Mar 2018 10:56:48 -0400
changeset 410190 4d66a4a931e4dccedce652d3d93799c685dd3eb2
parent 410189 8ab5b2af5a4c1083d3a21a0fa0685f9571407900
child 410191 1c5d4e9652092cb2cbec3c42656486a09b84ec9f
push id101408
push useraosmond@gmail.com
push dateTue, 27 Mar 2018 14:57:15 +0000
treeherdermozilla-inbound@1c5d4e965209 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1444537
milestone61.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 1444537 - Part 1. Ensure discarding of animated image frames is clear. r=tnikkel We should only attempt to discard animation image frames after passing the frame threshold on the very first pass on the animation. Redecodes are already in the correct state, as it will discard frames as it advances the animation. This patch makes it clear what it should be doing when, but there should be no functional change.
image/AnimationFrameBuffer.cpp
--- a/image/AnimationFrameBuffer.cpp
+++ b/image/AnimationFrameBuffer.cpp
@@ -72,39 +72,44 @@ AnimationFrameBuffer::Insert(RawAccessFr
     // (probably an empty frame) with the new frame.
     MOZ_ASSERT(MayDiscard());
     MOZ_ASSERT(mInsertIndex < mFrames.Length());
 
     if (mInsertIndex > 0) {
       MOZ_ASSERT(!mFrames[mInsertIndex]);
       mFrames[mInsertIndex] = Move(aFrame);
     }
-  } else {
-    if (mInsertIndex == mFrames.Length()) {
-      // We are still on the first pass of the animation decoding, so this is
-      // the first time we have seen this frame.
-      mFrames.AppendElement(Move(aFrame));
-    } else if (mInsertIndex > 0) {
-      // We were forced to restart an animation before we decoded the last
-      // frame. Thus we might need to insert, even on a "first pass."
-      MOZ_ASSERT(mInsertIndex < mFrames.Length());
-      MOZ_ASSERT(!mFrames[mInsertIndex]);
-      mFrames[mInsertIndex] = Move(aFrame);
-    }
+  } else if (mInsertIndex == mFrames.Length()) {
+    // We are still on the first pass of the animation decoding, so this is
+    // the first time we have seen this frame.
+    mFrames.AppendElement(Move(aFrame));
 
     if (mInsertIndex == mThreshold) {
-      // We just tripped over the threshold, and on the first pass of the
-      // decoding; this is our chance to do any clearing of already displayed
-      // frames. After this, we only need to release as we advance.
+      // We just tripped over the threshold for the first time. This is our
+      // chance to do any clearing of already displayed frames. After this,
+      // we only need to release as we advance or force a restart.
       MOZ_ASSERT(MayDiscard());
       MOZ_ASSERT(mGetIndex < mInsertIndex);
       for (size_t i = 1; i < mGetIndex; ++i) {
         RawAccessFrameRef discard = Move(mFrames[i]);
       }
     }
+  } else if (mInsertIndex > 0) {
+    // We were forced to restart an animation before we decoded the last
+    // frame. If we were discarding frames, then we tossed what we had
+    // except for the first frame.
+    MOZ_ASSERT(mInsertIndex < mFrames.Length());
+    MOZ_ASSERT(!mFrames[mInsertIndex]);
+    MOZ_ASSERT(MayDiscard());
+    mFrames[mInsertIndex] = Move(aFrame);
+  } else { // mInsertIndex == 0
+    // We were forced to restart an animation before we decoded the last
+    // frame. We don't need the redecoded first frame because we always keep
+    // the original.
+    MOZ_ASSERT(MayDiscard());
   }
 
   MOZ_ASSERT(mFrames[mInsertIndex]);
   ++mInsertIndex;
 
   // Ensure we only request more decoded frames if we actually need them. If we
   // need to advance to a certain point in the animation on behalf of the owner,
   // then do so. This ensures we keep decoding. If the batch size is really