Bug 1444537 - Part 1: Ensure discarding of animated image frames is clear. r=tnikkel, a=jcristau
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 27 Mar 2018 10:56:48 -0400
changeset 460433 9c6d282fa4b4c91ddfd7054e95cff07eecaa893c
parent 460432 8f10c0a7a87fec424520e133e0038bf338324b02
child 460434 a9b81a287bd326bbb6df92092d59b9bcb66db73d
push id8942
push userryanvm@gmail.com
push dateThu, 29 Mar 2018 14:09:55 +0000
treeherdermozilla-beta@75cefda36f28 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, jcristau
bugs1444537
milestone60.0
Bug 1444537 - Part 1: Ensure discarding of animated image frames is clear. r=tnikkel, a=jcristau 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