Bug 1343341. Change GetTimeoutForFrame to return a Maybe, and make all callers deal with a lack of a return value. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 23 Mar 2017 00:02:54 -0500
changeset 397250 af363f677224cd97a021cfea7820742aab25e915
parent 397249 ea7ac56cdb2efde6b4df1c530ab3333693666c8a
child 397251 b9593885460519d05b4ab2d50efaa2a95cc1bced
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1343341
milestone55.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 1343341. Change GetTimeoutForFrame to return a Maybe, and make all callers deal with a lack of a return value. r=aosmond Do this to allow GetTimeoutForFrame to be called for frames that haven't been decoded yet. Propagate a Maybe result where it makes sense. The remaining callers just bail if they get no return value. Many of them can just assert that they get a return value because they already got the same frame, so the timeout has to be available. The logic is a little tricky because we have "Forever" timeouts that were sort of treated as error cases.
image/FrameAnimator.cpp
image/FrameAnimator.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -136,38 +136,43 @@ AnimationState::LoopLength() const
   return *mLoopLength;
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // FrameAnimator implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
-TimeStamp
+Maybe<TimeStamp>
 FrameAnimator::GetCurrentImgFrameEndTime(AnimationState& aState) const
 {
   TimeStamp currentFrameTime = aState.mCurrentAnimationFrameTime;
-  FrameTimeout timeout = GetTimeoutForFrame(aState.mCurrentAnimationFrameIndex);
+  Maybe<FrameTimeout> timeout = GetTimeoutForFrame(aState, aState.mCurrentAnimationFrameIndex);
 
-  if (timeout == FrameTimeout::Forever()) {
+  if (timeout.isNothing()) {
+    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    return Nothing();
+  }
+
+  if (*timeout == FrameTimeout::Forever()) {
     // We need to return a sentinel value in this case, because our logic
     // 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
     // infinitely long timeouts.
-    return TimeStamp::NowLoRes() +
-           TimeDuration::FromMilliseconds(31536000.0);
+    return Some(TimeStamp::NowLoRes() +
+                TimeDuration::FromMilliseconds(31536000.0));
   }
 
   TimeDuration durationOfTimeout =
-    TimeDuration::FromMilliseconds(double(timeout.AsMilliseconds()));
+    TimeDuration::FromMilliseconds(double(timeout->AsMilliseconds()));
   TimeStamp currentFrameEndTime = currentFrameTime + durationOfTimeout;
 
-  return currentFrameEndTime;
+  return Some(currentFrameEndTime);
 }
 
 RefreshResult
 FrameAnimator::AdvanceFrame(AnimationState& aState, TimeStamp aTime)
 {
   NS_ASSERTION(aTime <= TimeStamp::Now(),
                "Given time appears to be in the future");
   PROFILER_LABEL_FUNC(js::ProfileEntry::Category::GRAPHICS);
@@ -233,40 +238,48 @@ FrameAnimator::AdvanceFrame(AnimationSta
   // failure) we would have discarded all the old frames and may not yet have
   // the new ones.
   if (!nextFrame || !nextFrame->IsFinished()) {
     // 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;
   }
 
-  if (GetTimeoutForFrame(nextFrameIndex) == FrameTimeout::Forever()) {
+  Maybe<FrameTimeout> nextFrameTimeout = GetTimeoutForFrame(aState, nextFrameIndex);
+  // GetTimeoutForFrame can only return none if frame doesn't exist,
+  // but we just got it above.
+  MOZ_ASSERT(nextFrameTimeout.isSome());
+  if (*nextFrameTimeout == FrameTimeout::Forever()) {
     ret.mAnimationFinished = true;
   }
 
   if (nextFrameIndex == 0) {
     ret.mDirtyRect = aState.FirstFrameRefreshArea();
   } else {
     MOZ_ASSERT(nextFrameIndex == currentFrameIndex + 1);
 
     // Change frame
     if (!DoBlend(&ret.mDirtyRect, currentFrameIndex, nextFrameIndex)) {
       // something went wrong, move on to next
       NS_WARNING("FrameAnimator::AdvanceFrame(): Compositing of frame failed");
       nextFrame->SetCompositingFailed(true);
-      aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState);
+      Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+      MOZ_ASSERT(currentFrameEndTime.isSome());
+      aState.mCurrentAnimationFrameTime = *currentFrameEndTime;
       aState.mCurrentAnimationFrameIndex = nextFrameIndex;
 
       return ret;
     }
 
     nextFrame->SetCompositingFailed(false);
   }
 
-  aState.mCurrentAnimationFrameTime = GetCurrentImgFrameEndTime(aState);
+  Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+  MOZ_ASSERT(currentFrameEndTime.isSome());
+  aState.mCurrentAnimationFrameTime = *currentFrameEndTime;
 
   // If we can get closer to the current time by a multiple of the image's loop
   // time, we should. We can only do this if we're done decoding; otherwise, we
   // don't know the full loop length, and LoopLength() will have to return
   // FrameTimeout::Forever().
   FrameTimeout loopTime = aState.LoopLength();
   if (loopTime != FrameTimeout::Forever()) {
     TimeDuration delay = aTime - aState.mCurrentAnimationFrameTime;
@@ -296,38 +309,48 @@ FrameAnimator::RequestRefresh(AnimationS
   RefreshResult ret;
 
   if (aState.IsDiscarded()) {
     return ret;
   }
 
   // only advance the frame if the current time is greater than or
   // equal to the current frame's end time.
-  TimeStamp currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+  Maybe<TimeStamp> currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+  if (currentFrameEndTime.isNothing()) {
+    MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
+    MOZ_ASSERT(aState.GetHasBeenDecoded() && !aState.GetIsCurrentlyDecoded());
+    MOZ_ASSERT(aState.mCompositedFrameInvalid);
+    // Nothing we can do but wait for our previous current frame to be decoded
+    // again so we can determine what to do next.
+    return ret;
+  }
 
-  while (currentFrameEndTime <= aTime) {
-    TimeStamp oldFrameEndTime = currentFrameEndTime;
+  while (*currentFrameEndTime <= aTime) {
+    TimeStamp oldFrameEndTime = *currentFrameEndTime;
 
     RefreshResult frameRes = AdvanceFrame(aState, aTime);
 
     // Accumulate our result for returning to callers.
     ret.Accumulate(frameRes);
 
     currentFrameEndTime = GetCurrentImgFrameEndTime(aState);
+    // AdvanceFrame can't advance to a frame that doesn't exist yet.
+    MOZ_ASSERT(currentFrameEndTime.isSome());
 
     // 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 (!frameRes.mFrameAdvanced && (currentFrameEndTime == oldFrameEndTime)) {
+    if (!frameRes.mFrameAdvanced && (*currentFrameEndTime == oldFrameEndTime)) {
       break;
     }
   }
 
   // Advanced to the correct frame, the composited frame is now valid to be drawn.
-  if (currentFrameEndTime > aTime) {
+  if (*currentFrameEndTime > aTime) {
     aState.mCompositedFrameInvalid = false;
   }
 
   MOZ_ASSERT(!aState.mIsCurrentlyDecoded || !aState.mCompositedFrameInvalid);
 
   return ret;
 }
 
@@ -366,27 +389,28 @@ FrameAnimator::GetCompositedFrame(Animat
   }
 
   MOZ_ASSERT(!result.Surface()->GetIsPaletted(),
              "About to return a paletted frame");
 
   return result;
 }
 
-FrameTimeout
-FrameAnimator::GetTimeoutForFrame(uint32_t aFrameNum) const
+Maybe<FrameTimeout>
+FrameAnimator::GetTimeoutForFrame(AnimationState& aState,
+                                  uint32_t aFrameNum) const
 {
   RawAccessFrameRef frame = GetRawFrame(aFrameNum);
   if (frame) {
     AnimationData data = frame->GetAnimationData();
-    return data.mTimeout;
+    return Some(data.mTimeout);
   }
 
-  NS_WARNING("No frame; called GetTimeoutForFrame too early?");
-  return FrameTimeout::FromRawMilliseconds(100);
+  MOZ_ASSERT(aState.mHasBeenDecoded && !aState.mIsCurrentlyDecoded);
+  return Nothing();
 }
 
 static void
 DoCollectSizeOfCompositingSurfaces(const RawAccessFrameRef& aSurface,
                                    SurfaceMemoryCounterType aType,
                                    nsTArray<SurfaceMemoryCounter>& aCounters,
                                    MallocSizeOf aMallocSizeOf)
 {
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -307,25 +307,27 @@ private: // methods
    */
   RefreshResult AdvanceFrame(AnimationState& aState, TimeStamp aTime);
 
   /**
    * Get the @aIndex-th frame in the frame index, ignoring results of blending.
    */
   RawAccessFrameRef GetRawFrame(uint32_t aFrameNum) const;
 
-  /// @return the given frame's timeout.
-  FrameTimeout GetTimeoutForFrame(uint32_t aFrameNum) const;
+  /// @return the given frame's timeout if it is available
+  Maybe<FrameTimeout> GetTimeoutForFrame(AnimationState& aState,
+                                         uint32_t aFrameNum) const;
 
   /**
    * Get the time the frame we're currently displaying is supposed to end.
    *
-   * In the error case, returns an "infinity" timestamp.
+   * In the error case (like if the requested frame is not currently
+   * decoded), returns None().
    */
-  TimeStamp GetCurrentImgFrameEndTime(AnimationState& aState) const;
+  Maybe<TimeStamp> GetCurrentImgFrameEndTime(AnimationState& aState) const;
 
   bool DoBlend(gfx::IntRect* aDirtyRect,
                uint32_t aPrevFrameIndex,
                uint32_t aNextFrameIndex);
 
   /** Clears an area of <aFrame> with transparent black.
    *
    * @param aFrameData Target Frame data