Bug 1288040 (Part 5) - Wrap frame timeout values in a FrameTimeout type that ensures they're normalized. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 19 Jul 2016 16:22:34 -0700
changeset 331023 3760df5754587ef476f339032068eb22b682c87e
parent 331022 b979622ece878a5eceb812a7ded7bad6fe8acbb1
child 331024 725304d2b48bcdd9924c0b0300e77d99f913f84d
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1288040
milestone50.0a1
Bug 1288040 (Part 5) - Wrap frame timeout values in a FrameTimeout type that ensures they're normalized. r=edwin
image/Decoder.cpp
image/Decoder.h
image/FrameAnimator.cpp
image/FrameAnimator.h
image/ImageMetadata.h
image/RasterImage.cpp
image/decoders/nsGIFDecoder2.cpp
image/decoders/nsPNGDecoder.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -406,29 +406,30 @@ Decoder::PostSize(int32_t aWidth,
 
 void
 Decoder::PostHasTransparency()
 {
   mProgress |= FLAG_HAS_TRANSPARENCY;
 }
 
 void
-Decoder::PostIsAnimated(int32_t aFirstFrameTimeout)
+Decoder::PostIsAnimated(FrameTimeout aFirstFrameTimeout)
 {
   mProgress |= FLAG_IS_ANIMATED;
   mImageMetadata.SetHasAnimation();
   mImageMetadata.SetFirstFrameTimeout(aFirstFrameTimeout);
 }
 
 void
 Decoder::PostFrameStop(Opacity aFrameOpacity
                          /* = Opacity::SOME_TRANSPARENCY */,
                        DisposalMethod aDisposalMethod
                          /* = DisposalMethod::KEEP */,
-                       int32_t aTimeout         /* = 0 */,
+                       FrameTimeout aTimeout
+                         /* = FrameTimeout::FromRawMilliseconds(0) */,
                        BlendMethod aBlendMethod /* = BlendMethod::OVER */,
                        const Maybe<nsIntRect>& aBlendRect /* = Nothing() */)
 {
   // We should be mid-frame
   MOZ_ASSERT(!IsMetadataDecode(), "Stopping frame during metadata decode");
   MOZ_ASSERT(mInFrame, "Stopping frame when we didn't start one");
   MOZ_ASSERT(mCurrentFrame, "Stopping frame when we don't have one");
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -329,26 +329,26 @@ protected:
   // until the entire frame has been decoded, decoders may take into account the
   // actual contents of the frame and give a more accurate result.
   void PostHasTransparency();
 
   // Called by decoders if they determine that the image is animated.
   //
   // @param aTimeout The time for which the first frame should be shown before
   //                 we advance to the next frame.
-  void PostIsAnimated(int32_t aFirstFrameTimeout);
+  void PostIsAnimated(FrameTimeout aFirstFrameTimeout);
 
   // Called by decoders when they end a frame. Informs the image, sends
   // notifications, and does internal book-keeping.
   // Specify whether this frame is opaque as an optimization.
   // For animated images, specify the disposal, blend method and timeout for
   // this frame.
   void PostFrameStop(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY,
                      DisposalMethod aDisposalMethod = DisposalMethod::KEEP,
-                     int32_t aTimeout = 0,
+                     FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0),
                      BlendMethod aBlendMethod = BlendMethod::OVER,
                      const Maybe<nsIntRect>& aBlendRect = Nothing());
 
   /**
    * Called by the decoders when they have a region to invalidate. We may not
    * actually pass these invalidations on right away.
    *
    * @param aRect The invalidation rect in the coordinate system of the unscaled
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -89,50 +89,50 @@ FrameAnimator::GetSingleLoopTime(Animati
 
   // If we're not looping, a single loop time has no meaning
   if (aState.mAnimationMode != imgIContainer::kNormalAnimMode) {
     return -1;
   }
 
   int32_t looptime = 0;
   for (uint32_t i = 0; i < mImage->GetNumFrames(); ++i) {
-    int32_t timeout = GetTimeoutForFrame(aState, i);
-    if (timeout >= 0) {
-      looptime += static_cast<uint32_t>(timeout);
-    } else {
+    FrameTimeout timeout = GetTimeoutForFrame(aState, i);
+    if (timeout == FrameTimeout::Forever()) {
       // 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?");
+      NS_WARNING("Infinite frame timeout - how did this happen?");
       return -1;
     }
+
+    looptime += timeout.AsMilliseconds();
   }
 
   return looptime;
 }
 
 TimeStamp
 FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState) const
 {
   TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime;
-  int32_t timeout =
+  FrameTimeout timeout =
     GetTimeoutForFrame(aState, aState.mCurrentAnimationFrameIndex);
 
-  if (timeout < 0) {
+  if (timeout == FrameTimeout::Forever()) {
     // We need to return a sentinel value in this case, because our logic
-    // doesn't work correctly if we have a negative timeout value. We use
-    // one year in the future as the sentinel because it works with the loop
-    // in RequestRefresh() below.
+    // doesn't work correctly if we have an infinitely long timeout. We use one
+    // year in the future as the sentinel because it works with the loop in
+    // RequestRefresh() below.
     // XXX(seth): It'd be preferable to make our logic work correctly with
-    // negative timeouts.
+    // infinitely long timeouts.
     return TimeStamp::NowLoRes() +
            TimeDuration::FromMilliseconds(31536000.0);
   }
 
   TimeDuration durationOfTimeout =
-    TimeDuration::FromMilliseconds(static_cast<double>(timeout));
+    TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds()));
   TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
 
   return currentFrameEndTime;
 }
 
 FrameAnimator::RefreshResult
 FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
 {
@@ -205,18 +205,17 @@ FrameAnimator::AdvanceFrame(AnimationSta
                     (nextFrame && nextFrame->IsFinished());
 
   if (!canDisplay) {
     // Uh oh, the frame we want to show is currently being decoded (partial)
     // Wait until the next refresh driver tick and try again
     return ret;
   }
 
-  // Bad data
-  if (GetTimeoutForFrame(aState, nextFrameIndex) < 0) {
+  if (GetTimeoutForFrame(aState, nextFrameIndex) == FrameTimeout::Forever()) {
     ret.animationFinished = true;
   }
 
   if (nextFrameIndex == 0) {
     ret.dirtyRect = aState.mFirstFrameRefreshArea;
   } else {
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
 
@@ -312,50 +311,31 @@ FrameAnimator::GetCompositedFrame(uint32
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
                                           aFrameNum));
   MOZ_ASSERT(!result || !result.DrawableRef()->GetIsPaletted(),
              "About to return a paletted frame");
   return result;
 }
 
-int32_t
+FrameTimeout
 FrameAnimator::GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const
 {
-  int32_t rawTimeout = 0;
-
   RawAccessFrameRef frame = GetRawFrame(aFrameNum);
   if (frame) {
     AnimationData data = frame->GetAnimationData();
-    rawTimeout = data.mRawTimeout;
-  } else if (aFrameNum == 0) {
-    rawTimeout = aState.mFirstFrameTimeout;
-  } else {
-    NS_WARNING("No frame; called GetTimeoutForFrame too early?");
-    return 100;
+    return data.mTimeout;
   }
 
-  // Ensure a minimal time between updates so we don't throttle the UI thread.
-  // consider 0 == unspecified and make it fast but not too fast.  Unless we
-  // have a single loop GIF. See bug 890743, bug 125137, bug 139677, and bug
-  // 207059. The behavior of recent IE and Opera versions seems to be:
-  // IE 6/Win:
-  //   10 - 50ms go 100ms
-  //   >50ms go correct speed
-  // Opera 7 final/Win:
-  //   10ms goes 100ms
-  //   >10ms go correct speed
-  // It seems that there are broken tools out there that set a 0ms or 10ms
-  // timeout when they really want a "default" one.  So munge values in that
-  // range.
-  if (rawTimeout >= 0 && rawTimeout <= 10) {
-    return 100;
+  if (aFrameNum == 0) {
+    return aState.mFirstFrameTimeout;
   }
 
-  return rawTimeout;
+  NS_WARNING("No frame; called GetTimeoutForFrame too early?");
+  return FrameTimeout::FromRawMilliseconds(100);
 }
 
 static void
 DoCollectSizeOfCompositingSurfaces(const RawAccessFrameRef& aSurface,
                                    SurfaceMemoryCounterType aType,
                                    nsTArray<SurfaceMemoryCounter>& aCounters,
                                    MallocSizeOf aMallocSizeOf)
 {
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -24,17 +24,17 @@ class RasterImage;
 
 class AnimationState
 {
 public:
   explicit AnimationState(uint16_t aAnimationMode)
     : mCurrentAnimationFrameIndex(0)
     , mLoopRemainingCount(-1)
     , mLoopCount(-1)
-    , mFirstFrameTimeout(0)
+    , mFirstFrameTimeout(FrameTimeout::FromRawMilliseconds(0))
     , mAnimationMode(aAnimationMode)
     , mDoneDecoding(false)
   { }
 
   /**
    * Call when this image is finished decoding so we know that there aren't any
    * more frames coming.
    */
@@ -86,17 +86,17 @@ public:
    */
   void SetLoopCount(int32_t aLoopCount) { mLoopCount = aLoopCount; }
   int32_t LoopCount() const { return mLoopCount; }
 
   /*
    * Set the timeout for the first frame. This is used to allow animation
    * scheduling even before a full decode runs for this image.
    */
-  void SetFirstFrameTimeout(int32_t aTimeout) { mFirstFrameTimeout = aTimeout; }
+  void SetFirstFrameTimeout(FrameTimeout aTimeout) { mFirstFrameTimeout = aTimeout; }
 
 private:
   friend class FrameAnimator;
 
   //! Area of the first frame that needs to be redrawn on subsequent loops.
   nsIntRect mFirstFrameRefreshArea;
 
   //! the time that the animation advanced to the current frame
@@ -107,17 +107,17 @@ private:
 
   //! number of loops remaining before animation stops (-1 no stop)
   int32_t mLoopRemainingCount;
 
   //! The total number of loops for the image.
   int32_t mLoopCount;
 
   //! The timeout for the first frame of this image.
-  int32_t mFirstFrameTimeout;
+  FrameTimeout mFirstFrameTimeout;
 
   //! The animation mode of this image. Constants defined in imgIContainer.
   uint16_t mAnimationMode;
 
   //! Whether this image is done being decoded.
   bool mDoneDecoding;
 };
 
@@ -176,23 +176,18 @@ public:
 
   /**
    * If we have a composited frame for @aFrameNum, returns it. Otherwise,
    * returns an empty LookupResult. It is an error to call this method with
    * aFrameNum == 0, because the first frame is never composited.
    */
   LookupResult GetCompositedFrame(uint32_t aFrameNum);
 
-  /*
-   * Returns the frame's adjusted timeout. If the animation loops and the
-   * timeout falls in between a certain range then the timeout is adjusted so
-   * that it's never 0. If the animation does not loop then no adjustments are
-   * made.
-   */
-  int32_t GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const;
+  /// @return the given frame's timeout.
+  FrameTimeout GetTimeoutForFrame(AnimationState& aState, uint32_t aFrameNum) const;
 
   /**
    * Collect an accounting of the memory occupied by the compositing surfaces we
    * use during animation playback. All of the actual animation frames are
    * stored in the SurfaceCache, so we don't need to report them here.
    */
   void CollectSizeOfCompositingSurfaces(nsTArray<SurfaceMemoryCounter>& aCounters,
                                         MallocSizeOf aMallocSizeOf) const;
--- a/image/ImageMetadata.h
+++ b/image/ImageMetadata.h
@@ -18,35 +18,35 @@ namespace image {
 class RasterImage;
 
 // The metadata about an image that decoders accumulate as they decode.
 class ImageMetadata
 {
 public:
   ImageMetadata()
     : mLoopCount(-1)
-    , mFirstFrameTimeout(0)
+    , mFirstFrameTimeout(FrameTimeout::Forever())
     , mHasAnimation(false)
   { }
 
   void SetHotspot(uint16_t aHotspotX, uint16_t aHotspotY)
   {
     mHotspot = Some(gfx::IntPoint(aHotspotX, aHotspotY));
   }
   gfx::IntPoint GetHotspot() const { return *mHotspot; }
   bool HasHotspot() const { return mHotspot.isSome(); }
 
   void SetLoopCount(int32_t loopcount)
   {
     mLoopCount = loopcount;
   }
   int32_t GetLoopCount() const { return mLoopCount; }
 
-  void SetFirstFrameTimeout(int32_t aTimeout) { mFirstFrameTimeout = aTimeout; }
-  int32_t GetFirstFrameTimeout() const { return mFirstFrameTimeout; }
+  void SetFirstFrameTimeout(FrameTimeout aTimeout) { mFirstFrameTimeout = aTimeout; }
+  FrameTimeout GetFirstFrameTimeout() const { return mFirstFrameTimeout; }
 
   void SetSize(int32_t width, int32_t height, Orientation orientation)
   {
     if (!HasSize()) {
       mSize.emplace(nsIntSize(width, height));
       mOrientation.emplace(orientation);
     }
   }
@@ -61,17 +61,17 @@ public:
 private:
   /// The hotspot found on cursors, if present.
   Maybe<gfx::IntPoint> mHotspot;
 
   /// The loop count for animated images, or -1 for infinite loop.
   int32_t mLoopCount;
 
   /// The timeout of an animated image's first frame.
-  int32_t mFirstFrameTimeout;
+  FrameTimeout mFirstFrameTimeout;
 
   Maybe<nsIntSize> mSize;
   Maybe<Orientation> mOrientation;
 
   bool mHasAnimation : 1;
 };
 
 } // namespace image
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -475,17 +475,18 @@ RasterImage::GetFirstFrameDelay()
 
   bool animated = false;
   if (NS_FAILED(GetAnimated(&animated)) || !animated) {
     return -1;
   }
 
   MOZ_ASSERT(mAnimationState, "Animated images should have an AnimationState");
   MOZ_ASSERT(mFrameAnimator, "Animated images should have a FrameAnimator");
-  return mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0);
+  return mFrameAnimator->
+    GetTimeoutForFrame(*mAnimationState, 0).AsEncodedValueDeprecated();
 }
 
 already_AddRefed<SourceSurface>
 RasterImage::CopyFrame(uint32_t aWhichFrame, uint32_t aFlags)
 {
   if (aWhichFrame > FRAME_MAX_VALUE) {
     return nullptr;
   }
@@ -919,20 +920,20 @@ RasterImage::StartAnimation()
   mPendingAnimation = !mAnimationState || GetNumFrames() < 2;
   if (mPendingAnimation) {
     return NS_OK;
   }
 
   MOZ_ASSERT(mFrameAnimator,
              "Should have a FrameAnimator if we have AnimationState");
 
-  // If the first frame has a timeout of -1, it means we should display it
-  // forever. Don't bother starting to animate in this case.
+  // Don't bother to animate if we're displaying the first frame forever.
   if (GetCurrentFrameIndex() == 0 &&
-      mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0) < 0) {
+      mFrameAnimator->GetTimeoutForFrame(*mAnimationState, 0)
+        == FrameTimeout::Forever()) {
     mAnimationFinished = true;
     return NS_ERROR_ABORT;
   }
 
   // We need to set the time that this initial frame was first displayed, as
   // this is used in AdvanceFrame().
   mAnimationState->InitAnimationFrameTimeIfNecessary();
 
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -251,17 +251,17 @@ nsGIFDecoder2::EndImageFrame()
   // append frames in BeginImageFrame(). This ensures that images_decoded
   // always refers to the frame in mImage we're currently decoding,
   // even if some of them weren't decoded properly and thus are blank.
   mGIFStruct.images_decoded++;
 
   // Tell the superclass we finished a frame
   PostFrameStop(opacity,
                 DisposalMethod(mGIFStruct.disposal_method),
-                mGIFStruct.delay_time);
+                FrameTimeout::FromRawMilliseconds(mGIFStruct.delay_time));
 
   // Reset the transparent pixel
   if (mOldColor) {
     mColormap[mGIFStruct.tpixel] = mOldColor;
     mOldColor = 0;
   }
 
   mCurrentFrameIndex = -1;
@@ -690,17 +690,17 @@ nsGIFDecoder2::ReadGraphicControlExtensi
   if (method == DisposalMethod::CLEAR_ALL || method == DisposalMethod::CLEAR) {
     // We may have to display the background under this image during animation
     // playback, so we regard it as transparent.
     PostHasTransparency();
   }
 
   mGIFStruct.delay_time = LittleEndian::readUint16(aData + 1) * 10;
   if (mGIFStruct.delay_time > 0) {
-    PostIsAnimated(mGIFStruct.delay_time);
+    PostIsAnimated(FrameTimeout::FromRawMilliseconds(mGIFStruct.delay_time));
   }
 
   return Transition::To(State::SKIP_SUB_BLOCKS, SUB_BLOCK_HEADER_LEN);
 }
 
 LexerTransition<nsGIFDecoder2::State>
 nsGIFDecoder2::ReadApplicationIdentifier(const char* aData)
 {
@@ -760,17 +760,17 @@ LexerTransition<nsGIFDecoder2::State>
 nsGIFDecoder2::ReadImageDescriptor(const char* aData)
 {
   if (mGIFStruct.images_decoded == 1) {
     if (!HasAnimation()) {
       // We should've already called PostIsAnimated(); this must be a corrupt
       // animated image with a first frame timeout of zero. Signal that we're
       // animated now, before the first-frame decode early exit below, so that
       // RasterImage can detect that this happened.
-      PostIsAnimated(0);
+      PostIsAnimated(FrameTimeout::FromRawMilliseconds(0));
     }
 
     if (IsFirstFrameDecode()) {
       // We're about to get a second frame, but we only want the first. Stop
       // decoding now.
       FinishInternal();
       return Transition::TerminateSuccess();
     }
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -257,17 +257,18 @@ nsPNGDecoder::EndImageFrame()
 
   mNumFrames++;
 
   Opacity opacity = Opacity::SOME_TRANSPARENCY;
   if (format == gfx::SurfaceFormat::B8G8R8X8) {
     opacity = Opacity::FULLY_OPAQUE;
   }
 
-  PostFrameStop(opacity, mAnimInfo.mDispose, mAnimInfo.mTimeout,
+  PostFrameStop(opacity, mAnimInfo.mDispose,
+                FrameTimeout::FromRawMilliseconds(mAnimInfo.mTimeout),
                 mAnimInfo.mBlend, Some(mFrameRect));
 }
 
 nsresult
 nsPNGDecoder::InitInternal()
 {
   mCMSMode = gfxPlatform::GetCMSMode();
   if (GetSurfaceFlags() & SurfaceFlags::NO_COLORSPACE_CONVERSION) {
@@ -670,17 +671,18 @@ nsPNGDecoder::info_callback(png_structp 
     decoder->format = gfx::SurfaceFormat::B8G8R8A8;
   } else {
     png_longjmp(decoder->mPNG, 1); // invalid number of channels
   }
 
 #ifdef PNG_APNG_SUPPORTED
   bool isAnimated = png_get_valid(png_ptr, info_ptr, PNG_INFO_acTL);
   if (isAnimated) {
-    decoder->PostIsAnimated(GetNextFrameDelay(png_ptr, info_ptr));
+    int32_t rawTimeout = GetNextFrameDelay(png_ptr, info_ptr);
+    decoder->PostIsAnimated(FrameTimeout::FromRawMilliseconds(rawTimeout));
 
     if (decoder->mDownscaler && !decoder->IsFirstFrameDecode()) {
       MOZ_ASSERT_UNREACHABLE("Doing downscale-during-decode "
                              "for an animated image?");
       png_longjmp(decoder->mPNG, 1);  // Abort the decode.
     }
   }
 #endif
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -157,17 +157,17 @@ static bool AllowedImageAndFrameDimensio
   }
   return true;
 }
 
 imgFrame::imgFrame()
   : mMonitor("imgFrame")
   , mDecoded(0, 0, 0, 0)
   , mLockCount(0)
-  , mTimeout(100)
+  , mTimeout(FrameTimeout::FromRawMilliseconds(100))
   , mDisposalMethod(DisposalMethod::NOT_SPECIFIED)
   , mBlendMethod(BlendMethod::OVER)
   , mHasNoAlpha(false)
   , mAborted(false)
   , mFinished(false)
   , mOptimizable(false)
   , mPalettedImageData(nullptr)
   , mPaletteDepth(0)
@@ -702,29 +702,30 @@ imgFrame::ImageUpdatedInternal(const nsI
   mDecoded.IntersectRect(mDecoded, mFrameRect);
 
   return NS_OK;
 }
 
 void
 imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
                  DisposalMethod aDisposalMethod /* = DisposalMethod::KEEP */,
-                 int32_t aRawTimeout /* = 0 */,
+                 FrameTimeout aTimeout
+                   /* = FrameTimeout::FromRawMilliseconds(0) */,
                  BlendMethod aBlendMethod /* = BlendMethod::OVER */,
                  const Maybe<IntRect>& aBlendRect /* = Nothing() */)
 {
   MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(mLockCount > 0, "Image data should be locked");
 
   if (aFrameOpacity == Opacity::FULLY_OPAQUE) {
     mHasNoAlpha = true;
   }
 
   mDisposalMethod = aDisposalMethod;
-  mTimeout = aRawTimeout;
+  mTimeout = aTimeout;
   mBlendMethod = aBlendMethod;
   mBlendRect = aBlendRect;
   ImageUpdatedInternal(GetRect());
   mFinished = true;
 
   // The image is now complete, wake up anyone who's waiting.
   mMonitor.NotifyAll();
 }
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -42,42 +42,142 @@ enum class DisposalMethod : int8_t {
 };
 
 enum class Opacity : uint8_t {
   FULLY_OPAQUE,
   SOME_TRANSPARENCY
 };
 
 /**
+ * FrameTimeout wraps a frame timeout value (measured in milliseconds) after
+ * first normalizing it. This normalization is necessary because some tools
+ * generate incorrect frame timeout values which we nevertheless have to
+ * support. For this reason, code that deals with frame timeouts should always
+ * use a FrameTimeout value rather than the raw value from the image header.
+ */
+struct FrameTimeout
+{
+  /**
+   * @return a FrameTimeout of zero. This should be used only for math
+   * involving FrameTimeout values. You can't obtain a zero FrameTimeout from
+   * FromRawMilliseconds().
+   */
+  static FrameTimeout Zero() { return FrameTimeout(0); }
+
+  /// @return an infinite FrameTimeout.
+  static FrameTimeout Forever() { return FrameTimeout(-1); }
+
+  /// @return a FrameTimeout obtained by normalizing a raw timeout value.
+  static FrameTimeout FromRawMilliseconds(int32_t aRawMilliseconds)
+  {
+    // Normalize all infinite timeouts to the same value.
+    if (aRawMilliseconds < 0) {
+      return FrameTimeout::Forever();
+    }
+
+    // Very small timeout values are problematic for two reasons: we don't want
+    // to burn energy redrawing animated images extremely fast, and broken tools
+    // generate these values when they actually want a "default" value, so such
+    // images won't play back right without normalization. For some context,
+    // see bug 890743, bug 125137, bug 139677, and bug 207059. The historical
+    // behavior of IE and Opera was:
+    //   IE 6/Win:
+    //     10 - 50ms is normalized to 100ms.
+    //     >50ms is used unnormalized.
+    //   Opera 7 final/Win:
+    //     10ms is normalized to 100ms.
+    //     >10ms is used unnormalized.
+    if (aRawMilliseconds >= 0 && aRawMilliseconds <= 10 ) {
+      return FrameTimeout(100);
+    }
+
+    // The provided timeout value is OK as-is.
+    return FrameTimeout(aRawMilliseconds);
+  }
+
+  bool operator==(const FrameTimeout& aOther) const
+  {
+    return mTimeout == aOther.mTimeout;
+  }
+
+  bool operator!=(const FrameTimeout& aOther) const { return !(*this == aOther); }
+
+  FrameTimeout operator+(const FrameTimeout& aOther)
+  {
+    if (*this == Forever() || aOther == Forever()) {
+      return Forever();
+    }
+
+    return FrameTimeout(mTimeout + aOther.mTimeout);
+  }
+
+  FrameTimeout& operator+=(const FrameTimeout& aOther)
+  {
+    *this = *this + aOther;
+    return *this;
+  }
+
+  /**
+   * @return this FrameTimeout's value in milliseconds. Illegal to call on a
+   * an infinite FrameTimeout value.
+   */
+  uint32_t AsMilliseconds() const
+  {
+    if (*this == Forever()) {
+      MOZ_ASSERT_UNREACHABLE("Calling AsMilliseconds() on an infinite FrameTimeout");
+      return 100;  // Fail to something sane.
+    }
+
+    return uint32_t(mTimeout);
+  }
+
+  /**
+   * @return this FrameTimeout value encoded so that non-negative values
+   * represent a timeout in milliseconds, and -1 represents an infinite
+   * timeout.
+   *
+   * XXX(seth): This is a backwards compatibility hack that should be removed.
+   */
+  int32_t AsEncodedValueDeprecated() const { return mTimeout; }
+
+private:
+  explicit FrameTimeout(int32_t aTimeout)
+    : mTimeout(aTimeout)
+  { }
+
+  int32_t mTimeout;
+};
+
+/**
  * AnimationData contains all of the information necessary for using an imgFrame
  * as part of an animation.
  *
  * It includes pointers to the raw image data of the underlying imgFrame, but
  * does not own that data. A RawAccessFrameRef for the underlying imgFrame must
  * outlive the AnimationData for it to remain valid.
  */
 struct AnimationData
 {
   AnimationData(uint8_t* aRawData, uint32_t aPaletteDataLength,
-                int32_t aRawTimeout, const nsIntRect& aRect,
+                FrameTimeout aTimeout, const nsIntRect& aRect,
                 BlendMethod aBlendMethod, const Maybe<gfx::IntRect>& aBlendRect,
                 DisposalMethod aDisposalMethod, bool aHasAlpha)
     : mRawData(aRawData)
     , mPaletteDataLength(aPaletteDataLength)
-    , mRawTimeout(aRawTimeout)
+    , mTimeout(aTimeout)
     , mRect(aRect)
     , mBlendMethod(aBlendMethod)
     , mBlendRect(aBlendRect)
     , mDisposalMethod(aDisposalMethod)
     , mHasAlpha(aHasAlpha)
   { }
 
   uint8_t* mRawData;
   uint32_t mPaletteDataLength;
-  int32_t mRawTimeout;
+  FrameTimeout mTimeout;
   nsIntRect mRect;
   BlendMethod mBlendMethod;
   Maybe<gfx::IntRect> mBlendRect;
   DisposalMethod mDisposalMethod;
   bool mHasAlpha;
 };
 
 class imgFrame
@@ -162,29 +262,27 @@ public:
    *
    * You must always call either Finish() or Abort() before releasing the last
    * RawAccessFrameRef pointing to an imgFrame.
    *
    * @param aFrameOpacity    Whether this imgFrame is opaque.
    * @param aDisposalMethod  For animation frames, how this imgFrame is cleared
    *                         from the compositing frame before the next frame is
    *                         displayed.
-   * @param aRawTimeout      For animation frames, the timeout in milliseconds
-   *                         before the next frame is displayed. This timeout is
-   *                         not necessarily the timeout that will actually be
-   *                         used; see FrameAnimator::GetTimeoutForFrame.
+   * @param aTimeout         For animation frames, the timeout before the next
+   *                         frame is displayed.
    * @param aBlendMethod     For animation frames, a blending method to be used
    *                         when compositing this frame.
    * @param aBlendRect       For animation frames, if present, the subrect in
    *                         which @aBlendMethod applies. Outside of this
    *                         subrect, BlendMethod::OVER is always used.
    */
   void Finish(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY,
               DisposalMethod aDisposalMethod = DisposalMethod::KEEP,
-              int32_t aRawTimeout = 0,
+              FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0),
               BlendMethod aBlendMethod = BlendMethod::OVER,
               const Maybe<IntRect>& aBlendRect = Nothing());
 
   /**
    * Mark this imgFrame as aborted. This informs the imgFrame that if it isn't
    * completely decoded now, it never will be.
    *
    * You must always call either Finish() or Abort() before releasing the last
@@ -307,18 +405,18 @@ private: // data
   RefPtr<VolatileBuffer> mVBuf;
   VolatileBufferPtr<uint8_t> mVBufPtr;
 
   nsIntRect mDecoded;
 
   //! Number of RawAccessFrameRefs currently alive for this imgFrame.
   int32_t mLockCount;
 
-  //! Raw timeout for this frame. (See FrameAnimator::GetTimeoutForFrame.)
-  int32_t mTimeout; // -1 means display forever.
+  //! The timeout for this frame.
+  FrameTimeout mTimeout;
 
   DisposalMethod mDisposalMethod;
   BlendMethod    mBlendMethod;
   Maybe<IntRect> mBlendRect;
   SurfaceFormat  mFormat;
 
   bool mHasNoAlpha;
   bool mAborted;