From ddf84be82308af148f500e0a79ac63bfbabf6293 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Fri, 4 Mar 2016 21:54:00 -0600 Subject: [PATCH] Bug 1251405. Part 1. Fix a significant signed/unsigned mismatch in handling the return value of FrameAnimator::GetSingleLoopTime. r=edwin GetSingleLoopTime returns -1 on exceptional cases but we used an unsigned int to hold the return value in AdvanceFrame. So the |loopTime > 0| check would succeed. Fortunately the |delay.ToMilliseconds() > loopTime| check would fail because loopTime was MAX_UNIT32, so we didn't do anything incorrect. http://hg.mozilla.org/mozilla-central/rev/263980931d1b (bug 890743) changed GetSingleLoopTime from returning 0 (and uint32_t) to -1 (and int32_t) on exceptional cases. But the caller of GetSingleLoopTime wasn't updated. --- image/FrameAnimator.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/image/FrameAnimator.cpp b/image/FrameAnimator.cpp index a44413433e2..0f3c3436d50 100644 --- a/image/FrameAnimator.cpp +++ b/image/FrameAnimator.cpp @@ -33,7 +33,7 @@ FrameAnimator::GetSingleLoopTime() const return -1; } - uint32_t looptime = 0; + int32_t looptime = 0; for (uint32_t i = 0; i < mImage->GetNumFrames(); ++i) { int32_t timeout = GetTimeoutForFrame(i); if (timeout >= 0) { @@ -180,9 +180,14 @@ FrameAnimator::AdvanceFrame(TimeStamp aTime) mCurrentAnimationFrameTime = 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(); + // time, we should. We need to be done decoding in order to know the full loop + // time though! + int32_t loopTime = GetSingleLoopTime(); if (loopTime > 0) { + // We shouldn't be advancing by a whole loop unless we are decoded and know + // what a full loop actually is. GetSingleLoopTime should return -1 so this + // never happens. + MOZ_ASSERT(mDoneDecoding); TimeDuration delay = aTime - mCurrentAnimationFrameTime; if (delay.ToMilliseconds() > loopTime) { // Explicitly use integer division to get the floor of the number of