Bug 1501923 - Fix crash where we reset an animation just before crossing the discard threshold. r=tnikkel

If an animated frame buffer was reset just before the necessary frame to
cross the discard threshold, followed by said frame being inserted by
the decoder, it would insert a null pointer into the display queue for
the first frame. This is because it assumed that we have always advanced
past the first frame -- which was true, but the reset placed us back at
the beginning.

This would initially manifest to the user as the animation stopping,
since it could not advance past the first frame. Once a memory report
was requested, it would crash because we assume every frame in the
display queue is valid.

This patch removes the assumption about what frame we have advanced to.

Differential Revision: https://phabricator.services.mozilla.com/D13407

image/AnimationFrameBuffer.cpp

@@ -140,14 +140,16 @@ AnimationFrameDiscardingQueue::AnimationFrameDiscardingQueue(
     AnimationFrameRetainedBuffer&& aQueue)
     : AnimationFrameBuffer(aQueue),
       mInsertIndex(aQueue.mFrames.Length()),
-      mFirstFrame(std::move(aQueue.mFrames[0])) {
+      mFirstFrame(aQueue.mFrames[0]) {
   MOZ_ASSERT(!mSizeKnown);
   MOZ_ASSERT(!mRedecodeError);
   MOZ_ASSERT(mInsertIndex > 0);
-  MOZ_ASSERT(mGetIndex > 0);
   mMayDiscard = true;
 
-  for (size_t i = aQueue.mGetIndex; i < mInsertIndex; ++i) {
+  // We avoided moving aQueue.mFrames[0] for mFirstFrame above because it is
+  // possible the animation was reset back to the beginning, and then we crossed
+  // the threshold without advancing further. That would mean mGetIndex is 0.
+  for (size_t i = mGetIndex; i < mInsertIndex; ++i) {
     MOZ_ASSERT(aQueue.mFrames[i]);
     mDisplay.push_back(std::move(aQueue.mFrames[i]));
   }

image/test/gtest/TestAnimationFrameBuffer.cpp

@@ -635,6 +635,31 @@ TEST_F(ImageAnimationFrameBuffer, DiscardingReset)
   TestDiscardingQueueReset(buffer, firstFrame, kThreshold, kBatch, kStartFrame);
 }
 
+TEST_F(ImageAnimationFrameBuffer, ResetBeforeDiscardingThreshold)
+{
+  const size_t kThreshold = 3;
+  const size_t kBatch = 1;
+  const size_t kStartFrame = 0;
+
+  // Get the starting buffer to just before the point where we need to switch
+  // to a discarding buffer, reset the animation so advancing points at the
+  // first frame, and insert the last frame to cross the threshold.
+  AnimationFrameRetainedBuffer retained(kThreshold, kBatch, kStartFrame);
+  VerifyInsert(retained, AnimationFrameBuffer::InsertStatus::CONTINUE);
+  VerifyInsertAndAdvance(retained, 1, AnimationFrameBuffer::InsertStatus::YIELD);
+  bool restartDecoder = retained.Reset();
+  EXPECT_FALSE(restartDecoder);
+  VerifyInsert(retained, AnimationFrameBuffer::InsertStatus::DISCARD_YIELD);
+
+  const imgFrame* firstFrame = retained.Frames()[0].get();
+  EXPECT_TRUE(firstFrame != nullptr);
+  AnimationFrameDiscardingQueue buffer(std::move(retained));
+  const imgFrame* displayFirstFrame = buffer.Get(0, true);
+  const imgFrame* advanceFirstFrame = buffer.Get(0, false);
+  EXPECT_EQ(firstFrame, displayFirstFrame);
+  EXPECT_EQ(firstFrame, advanceFirstFrame);
+}
+
 TEST_F(ImageAnimationFrameBuffer, RecyclingReset)
 {
   const size_t kThreshold = 8;