Bug 899861 - Animated gifs should not wait to play until fully downloaded. This is a partial backout of 717872 with the intent to re-enable using the separate FrameAnimator class. Hide the new approach behind #define USE_FRAME_ANIMATOR for now. r=bgirard

image/src/RasterImage.cpp

@@ -387,6 +387,9 @@ RasterImage::RasterImage(imgStatusTracker* aStatusTracker,
   mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
   mMultipartDecodedFrame(nullptr),
   mAnim(nullptr),
+#ifndef USE_FRAME_ANIMATOR
+  mLoopCount(-1),
+#endif
   mLockCount(0),
   mDecodeCount(0),
 #ifdef DEBUG
@@ -524,6 +527,137 @@ RasterImage::Init(const char* aMimeType,
   return NS_OK;
 }
 
+#ifndef USE_FRAME_ANIMATOR
+uint32_t
+RasterImage::GetSingleLoopTime() const
+{
+  if (!mAnim) {
+    return 0;
+  }
+
+  // If we aren't done decoding, we don't know the image's full play time.
+  if (!mHasBeenDecoded) {
+    return 0;
+  }
+
+  // If we're not looping, a single loop time has no meaning
+  if (mLoopCount == 0) {
+    return 0;
+  }
+
+  uint32_t looptime = 0;
+  for (uint32_t i = 0; i < GetNumFrames(); ++i) {
+    int32_t timeout = mFrameBlender.RawGetFrame(i)->GetTimeout();
+    if (timeout > 0) {
+      looptime += static_cast<uint32_t>(timeout);
+    } else {
+      // If we have a frame that never times out, we're probably in an error
+      // case, but let's handle it more gracefully.
+      NS_WARNING("Negative frame timeout - how did this happen?");
+      return 0;
+    }
+  }
+
+  return looptime;
+}
+
+bool
+RasterImage::AdvanceFrame(TimeStamp aTime, nsIntRect* aDirtyRect)
+{
+  NS_ASSERTION(aTime <= TimeStamp::Now(),
+               "Given time appears to be in the future");
+
+  uint32_t currentFrameIndex = mAnim->currentAnimationFrameIndex;
+  uint32_t nextFrameIndex = mAnim->currentAnimationFrameIndex + 1;
+  uint32_t timeout = 0;
+
+  // Figure out if we have the next full frame. This is more complicated than
+  // just checking GetNumFrames() because decoders append their frames
+  // before they're filled in.
+  NS_ABORT_IF_FALSE(mDecoder || nextFrameIndex <= GetNumFrames(),
+                    "How did we get 2 indices too far by incrementing?");
+
+  // If we don't have a decoder, we know we've got everything we're going to
+  // get. If we do, we only display fully-downloaded frames; everything else
+  // gets delayed.
+  bool haveFullNextFrame = (mMultipart && mBytesDecoded == 0) || !mDecoder ||
+                            nextFrameIndex < mDecoder->GetCompleteFrameCount();
+
+  // If we're done decoding the next frame, go ahead and display it now and
+  // reinit with the next frame's delay time.
+  if (haveFullNextFrame) {
+    if (GetNumFrames() == nextFrameIndex) {
+      // End of Animation, unless we are looping forever
+
+      // If animation mode is "loop once", it's time to stop animating
+      if (mAnimationMode == kLoopOnceAnimMode || mLoopCount == 0) {
+        mAnimationFinished = true;
+        EvaluateAnimation();
+      }
+
+      nextFrameIndex = 0;
+
+      if (mLoopCount > 0) {
+        mLoopCount--;
+      }
+
+      if (!mAnimating) {
+        // break out early if we are actually done animating
+        return false;
+      }
+    }
+
+    timeout = mFrameBlender.GetFrame(nextFrameIndex)->GetTimeout();
+
+  } else {
+    // Uh oh, the frame we want to show is currently being decoded (partial)
+    // Wait until the next refresh driver tick and try again
+    return false;
+  }
+
+  if (!(timeout > 0)) {
+    mAnimationFinished = true;
+    EvaluateAnimation();
+  }
+
+  if (nextFrameIndex == 0) {
+    *aDirtyRect = mAnim->firstFrameRefreshArea;
+  } else {
+    // Change frame
+    if (!mFrameBlender.DoBlend(aDirtyRect, currentFrameIndex, nextFrameIndex)) {
+      // something went wrong, move on to next
+      NS_WARNING("RasterImage::AdvanceFrame(): Compositing of frame failed");
+      mFrameBlender.RawGetFrame(nextFrameIndex)->SetCompositingFailed(true);
+      mAnim->currentAnimationFrameTime = GetCurrentImgFrameEndTime();
+      mAnim->currentAnimationFrameIndex = nextFrameIndex;
+      return false;
+    }
+
+    mFrameBlender.RawGetFrame(nextFrameIndex)->SetCompositingFailed(false);
+  }
+
+  mAnim->currentAnimationFrameTime = GetCurrentImgFrameEndTime();
+
+  // If we can get closer to the current time by a multiple of the image's loop
+  // time, we should.
+  uint32_t loopTime = GetSingleLoopTime();
+  if (loopTime > 0) {
+    TimeDuration delay = aTime - mAnim->currentAnimationFrameTime;
+    if (delay.ToMilliseconds() > loopTime) {
+      // Explicitly use integer division to get the floor of the number of
+      // loops.
+      uint32_t loops = static_cast<uint32_t>(delay.ToMilliseconds()) / loopTime;
+      mAnim->currentAnimationFrameTime += TimeDuration::FromMilliseconds(loops * loopTime);
+    }
+  }
+
+  // Set currentAnimationFrameIndex at the last possible moment
+  mAnim->currentAnimationFrameIndex = nextFrameIndex;
+
+  return true;
+}
+#endif
+
 //******************************************************************************
 // [notxpcom] void requestRefresh ([const] in TimeStamp aTime);
 NS_IMETHODIMP_(void)
@@ -535,12 +669,45 @@ RasterImage::RequestRefresh(const mozilla::TimeStamp& aTime)
 
   EvaluateAnimation();
 
+#ifdef USE_FRAME_ANIMATOR
   FrameAnimator::RefreshResult res;
   if (mAnim) {
     res = mAnim->RequestRefresh(aTime);
   }
+#else
+  // only advance the frame if the current time is greater than or
+  // equal to the current frame's end time.
+  TimeStamp currentFrameEndTime = GetCurrentImgFrameEndTime();
+  bool frameAdvanced = false;
 
+  // The dirtyRect variable will contain an accumulation of the sub-rectangles
+  // that are dirty for each frame we advance in AdvanceFrame().
+  nsIntRect dirtyRect;
+
+  while (currentFrameEndTime <= aTime) {
+    TimeStamp oldFrameEndTime = currentFrameEndTime;
+    nsIntRect frameDirtyRect;
+    bool didAdvance = AdvanceFrame(aTime, &frameDirtyRect);
+    frameAdvanced = frameAdvanced || didAdvance;
+    currentFrameEndTime = GetCurrentImgFrameEndTime();
+
+    // Accumulate the dirty area.
+    dirtyRect = dirtyRect.Union(frameDirtyRect);
+
+    // if we didn't advance a frame, and our frame end time didn't change,
+    // then we need to break out of this loop & wait for the frame(s)
+    // to finish downloading
+    if (!didAdvance && (currentFrameEndTime == oldFrameEndTime)) {
+      break;
+    }
+  }
+#endif
+
+#ifdef USE_FRAME_ANIMATOR
   if (res.frameAdvanced) {
+#else
+  if (frameAdvanced) {
+#endif
     // Notify listeners that our frame has actually changed, but do this only
     // once for all frames that we've now passed (if AdvanceFrame() was called
     // more than once).
@@ -552,14 +719,22 @@ RasterImage::RequestRefresh(const mozilla::TimeStamp& aTime)
 
     // Explicitly call this on mStatusTracker so we're sure to not interfere
     // with the decoding process
-    if (mStatusTracker)
+#ifdef USE_FRAME_ANIMATOR
+    if (mStatusTracker) {
       mStatusTracker->FrameChanged(&res.dirtyRect);
+    }
   }
 
   if (res.animationFinished) {
     mAnimationFinished = true;
     EvaluateAnimation();
   }
+#else
+    if (mStatusTracker) {
+      mStatusTracker->FrameChanged(&dirtyRect);
+    }
+  }
+#endif
 }
 
 //******************************************************************************
@@ -687,12 +862,42 @@ RasterImage::GetDrawableImgFrame(uint32_t framenum)
 uint32_t
 RasterImage::GetCurrentImgFrameIndex() const
 {
-  if (mAnim)
+  if (mAnim) {
+#ifdef USE_FRAME_ANIMATOR
     return mAnim->GetCurrentAnimationFrameIndex();
+#else
+    return mAnim->currentAnimationFrameIndex;
+#endif
+  }
 
   return 0;
 }
 
+#ifndef USE_FRAME_ANIMATOR
+TimeStamp
+RasterImage::GetCurrentImgFrameEndTime() const
+{
+  imgFrame* currentFrame = mFrameBlender.RawGetFrame(mAnim->currentAnimationFrameIndex);
+  TimeStamp currentFrameTime = mAnim->currentAnimationFrameTime;
+  int64_t timeout = currentFrame->GetTimeout();
+
+  if (timeout < 0) {
+    // We need to return a sentinel value in this case, because our logic
+    // doesn't work correctly if we have a negative timeout value. The reason
+    // this positive infinity was chosen was because it works with the loop in
+    // RequestRefresh() above.
+    return TimeStamp() +
+           TimeDuration::FromMilliseconds(static_cast<double>(UINT64_MAX));
+  }
+
+  TimeDuration durationOfTimeout =
+    TimeDuration::FromMilliseconds(static_cast<double>(timeout));
+  TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
+
+  return currentFrameEndTime;
+}
+#endif
+
 imgFrame*
 RasterImage::GetCurrentImgFrame()
 {
@@ -1062,6 +1267,7 @@ RasterImage::OutOfProcessSizeOfDecoded() const
                                                  NULL);
 }
 
+#ifdef USE_FRAME_ANIMATOR
 void
 RasterImage::EnsureAnimExists()
 {
@@ -1085,6 +1291,7 @@ RasterImage::EnsureAnimExists()
     CurrentStatusTracker().RecordImageIsAnimated();
   }
 }
+#endif
 
 nsresult
 RasterImage::InternalAddFrameHelper(uint32_t framenum, imgFrame *aFrame,
@@ -1166,13 +1373,23 @@ RasterImage::InternalAddFrame(uint32_t framenum,
     int32_t frameDisposalMethod = mFrameBlender.RawGetFrame(0)->GetFrameDisposalMethod();
     if (frameDisposalMethod == FrameBlender::kDisposeClear ||
         frameDisposalMethod == FrameBlender::kDisposeRestorePrevious)
+#ifdef USE_FRAME_ANIMATOR
       mAnim->SetFirstFrameRefreshArea(mFrameBlender.RawGetFrame(0)->GetRect());
+#else
+      mAnim->firstFrameRefreshArea = mFrameBlender.RawGetFrame(0)->GetRect();
+#endif
   }
 
   // Calculate firstFrameRefreshArea
   // Some gifs are huge but only have a small area that they animate
   // We only need to refresh that small area when Frame 0 comes around again
+#ifdef USE_FRAME_ANIMATOR
   mAnim->UnionFirstFrameRefreshArea(frame->GetRect());
+#else
+  nsIntRect frameRect = frame->GetRect();
+  mAnim->firstFrameRefreshArea.UnionRect(mAnim->firstFrameRefreshArea,
+                                         frameRect);
+#endif
 
   rv = InternalAddFrameHelper(framenum, frame.forget(), imageData, imageLength,
                               paletteData, paletteLength, aRetFrame);
@@ -1395,13 +1612,16 @@ RasterImage::DecodingComplete()
     }
   }
 
+#ifdef USE_FRAME_ANIMATOR
   if (mAnim) {
     mAnim->SetDoneDecoding(true);
   }
+#endif
 
   return NS_OK;
 }
 
+#ifdef USE_FRAME_ANIMATOR
 NS_IMETHODIMP
 RasterImage::SetAnimationMode(uint16_t aAnimationMode)
 {
@@ -1410,6 +1630,7 @@ RasterImage::SetAnimationMode(uint16_t aAnimationMode)
   }
   return SetAnimationModeInternal(aAnimationMode);
 }
+#endif
 
 //******************************************************************************
 /* void StartAnimation () */
@@ -1432,7 +1653,13 @@ RasterImage::StartAnimation()
 
     // We need to set the time that this initial frame was first displayed, as
     // this is used in AdvanceFrame().
+#ifdef USE_FRAME_ANIMATOR
     mAnim->InitAnimationFrameTimeIfNecessary();
+#else
+    if (mAnim->currentAnimationFrameTime.IsNull()) {
+      mAnim->currentAnimationFrameTime = TimeStamp::Now();
+    }
+#endif
   }
 
   return NS_OK;
@@ -1459,9 +1686,15 @@ RasterImage::ResetAnimation()
   if (mError)
     return NS_ERROR_FAILURE;
 
+#ifdef USE_FRAME_ANIMATOR
   if (mAnimationMode == kDontAnimMode ||
       !mAnim || mAnim->GetCurrentAnimationFrameIndex() == 0)
     return NS_OK;
+#else
+  if (mAnimationMode == kDontAnimMode ||
+      !mAnim || mAnim->currentAnimationFrameIndex == 0)
+    return NS_OK;
+#endif
 
   mAnimationFinished = false;
 
@@ -1469,10 +1702,14 @@ RasterImage::ResetAnimation()
     StopAnimation();
 
   mFrameBlender.ResetAnimation();
+#ifdef USE_FRAME_ANIMATOR
   if (mAnim) {
     mAnim->ResetAnimation();
   }
+#else
 
+  mAnim->currentAnimationFrameIndex = 0;
+#endif
   UpdateImageContainer();
 
   // Note - We probably want to kick off a redecode somewhere around here when
@@ -1480,8 +1717,12 @@ RasterImage::ResetAnimation()
 
   // Update display if we were animating before
   if (mAnimating && mStatusTracker) {
+#ifdef USE_FRAME_ANIMATOR
     nsIntRect rect = mAnim->GetFirstFrameRefreshArea();
     mStatusTracker->FrameChanged(&rect);
+#else
+    mStatusTracker->FrameChanged(&(mAnim->firstFrameRefreshArea));
+#endif
   }
 
   if (ShouldAnimate()) {
@@ -1503,16 +1744,26 @@ RasterImage::SetAnimationStartTime(const mozilla::TimeStamp& aTime)
   if (mError || mAnimating || !mAnim)
     return;
 
+#ifdef USE_FRAME_ANIMATOR
   mAnim->SetAnimationFrameTime(aTime);
+#else
+  mAnim->currentAnimationFrameTime = aTime;
+#endif
 }
 
 NS_IMETHODIMP_(float)
 RasterImage::GetFrameIndex(uint32_t aWhichFrame)
 {
   MOZ_ASSERT(aWhichFrame <= FRAME_MAX_VALUE, "Invalid argument");
+#ifdef USE_FRAME_ANIMATOR
   return (aWhichFrame == FRAME_FIRST || !mAnim)
          ? 0.0f
          : mAnim->GetCurrentAnimationFrameIndex();
+#else
+  return (aWhichFrame == FRAME_FIRST || !mAnim)
+         ? 0.0f
+         : mAnim->currentAnimationFrameIndex;
+#endif
 }
 
 void
@@ -1521,9 +1772,17 @@ RasterImage::SetLoopCount(int32_t aLoopCount)
   if (mError)
     return;
 
+#ifdef USE_FRAME_ANIMATOR
   if (mAnim) {
     mAnim->SetLoopCount(aLoopCount);
   }
+#else
+  // -1  infinite
+  //  0  no looping, one iteration
+  //  1  one loop, two iterations
+  //  ...
+  mLoopCount = aLoopCount;
+#endif
 }
 
 nsresult
@@ -1795,9 +2054,11 @@ RasterImage::OnNewSourceData()
   mWantFullDecode = true;
   mDecodeRequest = nullptr;
 
+#ifdef USE_FRAME_ANIMATOR
   if (mAnim) {
     mAnim->SetDoneDecoding(false);
   }
+#endif
 
   // We always need the size first.
   rv = InitDecoder(/* aDoSizeDecode = */ true);

image/src/RasterImage.h

@@ -41,6 +41,10 @@
   #include "imgIContainerDebug.h"
 #endif
 
+// This will enable FrameAnimator approach to image animation.  Before doing
+// so, make sure bug 899861 symptoms are gone.
+// #define USE_FRAME_ANIMATOR 1
+
 class nsIInputStream;
 class nsIThreadPool;
 
@@ -321,6 +325,22 @@ private:
 
   nsresult OnImageDataCompleteCore(nsIRequest* aRequest, nsISupports*, nsresult aStatus);
 
+#ifndef USE_FRAME_ANIMATOR
+  struct Anim
+  {
+    //! Area of the first frame that needs to be redrawn on subsequent loops.
+    nsIntRect                  firstFrameRefreshArea;
+    uint32_t                   currentAnimationFrameIndex; // 0 to numFrames-1
+
+    // the time that the animation advanced to the current frame
+    TimeStamp                  currentAnimationFrameTime;
+
+    Anim() :
+      currentAnimationFrameIndex(0)
+    {}
+  };
+#endif
+
   /**
    * Each RasterImage has a pointer to one or zero heap-allocated
    * DecodeRequests.
@@ -528,6 +548,35 @@ private:
                      uint32_t aFlags,
                      gfxImageSurface **_retval);
 
+#ifndef USE_FRAME_ANIMATOR
+  /**
+   * Advances the animation. Typically, this will advance a single frame, but it
+   * may advance multiple frames. This may happen if we have infrequently
+   * "ticking" refresh drivers (e.g. in background tabs), or extremely short-
+   * lived animation frames.
+   *
+   * @param aTime the time that the animation should advance to. This will
+   *              typically be <= TimeStamp::Now().
+   *
+   * @param [out] aDirtyRect a pointer to an nsIntRect which encapsulates the
+   *        area to be repainted after the frame is advanced.
+   *
+   * @returns true, if the frame was successfully advanced, false if it was not
+   *          able to be advanced (e.g. the frame to which we want to advance is
+   *          still decoding). Note: If false is returned, then aDirtyRect will
+   *          remain unmodified.
+   */
+  bool AdvanceFrame(mozilla::TimeStamp aTime, nsIntRect* aDirtyRect);
+
+  /**
+   * Gets the length of a single loop of this image, in milliseconds.
+   *
+   * If this image is not finished decoding, is not animated, or it is animated
+   * but does not loop, returns 0.
+   */
+  uint32_t GetSingleLoopTime() const;
+#endif
+
   /**
    * Deletes and nulls out the frame in mFrames[framenum].
    *
@@ -543,11 +592,39 @@ private:
   imgFrame* GetDrawableImgFrame(uint32_t framenum);
   imgFrame* GetCurrentImgFrame();
   uint32_t GetCurrentImgFrameIndex() const;
+#ifndef USE_FRAME_ANIMATOR
+  mozilla::TimeStamp GetCurrentImgFrameEndTime() const;
+#endif
 
   size_t SizeOfDecodedWithComputedFallbackIfHeap(gfxASurface::MemoryLocation aLocation,
                                                  mozilla::MallocSizeOf aMallocSizeOf) const;
 
+#ifdef USE_FRAME_ANIMATOR
   void EnsureAnimExists();
+#else
+  inline void EnsureAnimExists()
+  {
+    if (!mAnim) {
+
+      // Create the animation context
+      mAnim = new Anim();
+
+      // We don't support discarding animated images (See bug 414259).
+      // Lock the image and throw away the key.
+      //
+      // Note that this is inefficient, since we could get rid of the source
+      // data too. However, doing this is actually hard, because we're probably
+      // calling ensureAnimExists mid-decode, and thus we're decoding out of
+      // the source buffer. Since we're going to fix this anyway later, and
+      // since we didn't kill the source data in the old world either, locking
+      // is acceptable for the moment.
+      LockImage();
+
+      // Notify our observers that we are starting animation.
+      CurrentStatusTracker().RecordImageIsAnimated();
+    }
+  }
+#endif
 
   nsresult InternalAddFrameHelper(uint32_t framenum, imgFrame *frame,
                                   uint8_t **imageData, uint32_t *imageLength,
@@ -605,7 +682,14 @@ private: // data
   // IMPORTANT: if you use mAnim in a method, call EnsureImageIsDecoded() first to ensure
   // that the frames actually exist (they may have been discarded to save memory, or
   // we maybe decoding on draw).
+#ifdef USE_FRAME_ANIMATOR
   FrameAnimator* mAnim;
+#else
+  RasterImage::Anim*        mAnim;
+
+  //! # loops remaining before animation stops (-1 no stop)
+  int32_t                    mLoopCount;
+#endif
 
   // Discard members
   uint32_t                   mLockCount;
@@ -722,6 +806,12 @@ inline NS_IMETHODIMP RasterImage::GetAnimationMode(uint16_t *aAnimationMode) {
   return GetAnimationModeInternal(aAnimationMode);
 }
 
+#ifndef USE_FRAME_ANIMATOR
+inline NS_IMETHODIMP RasterImage::SetAnimationMode(uint16_t aAnimationMode) {
+  return SetAnimationModeInternal(aAnimationMode);
+}
+#endif
+
 // Asynchronous Decode Requestor
 //
 // We use this class when someone calls requestDecode() from within a decode