FrameAnimator::GetCompositedFrame is only ever called with the current animation frame index. This is good because it can return invalid results if it is called for some other frame number.
This means that in RasterImage::LookupFrame when we are asked to do a sync decode (if needed) we use WaitUntilComplete to wait until the frame is finished decoding. But we would actually return after the next progressive pass notified the monitor to wake up. Thus, we would draw a not-fully-decoded image even though the sync decode flag was passed.
The change in FrameAnimator means that we won't draw the next frame in an animated image until all progressive passes of that image are complete. This seems like what we want anyways.
There is one real use of IsImageComplete left, in imgFrame::Draw, where we need to know if the decoded image data covers the whole image frame. (There are a couple of uses of IsImageComplete in asserts.)
When storing ms, 32 bit ints can hold 2^32/1000/60/60/24 ~= 49 days. It's quite conceivable that someone would leave a tab in the background for 50 days.
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.
Before the previous patch we would (wrongly) loop through the decoded frames even though we didn't have all of the frames of the animation. This had the beneficial side effect of advancing mCurrentAnimationFrameTime to aTime (the current time). With the previous patch we stop at the last decoded frame and don't advance mCurrentAnimationFrameTime, so it can lag behind. The problem with this is that when we have finished decoding we will then try to catch mCurrentAnimationFrameTime up, and this will jump us to a random point in the animation. So we need to advance mCurrentAnimationFrameTime ourselves.
If we were blocked on network/decoding then displaying the last available decoded frame is the correct frame to be displaying. So we are up to date. So we advance mCurrentAnimationFrameTime to the current time.
mImage->GetNumFrames() is the current number of decoded frames (that the RasterImage knows about), so it only represents the last frame of the animation if we are done decoding.
If we are not fully decoded, and we are on the last decoded frame, just stay on the last decoded frame. When more frames get decoded (or we determine that we are the last frame of the animation) we will advance.
One might expect that if |nextFrameIndex == mImage->GetNumFrames()| then |GetRawFrame(nextFrameIndex)| would return a null surface. But that is not the case because the decoding thread can insert frames into the surface cache that the RasterImage hasn't acknowledged yet (because it has to do so on the main thread, which we are currently running on).
This is why moving animated images to the surface cache is likely the cause of this bug.
This introduces an issue that is explained in, and fixed by the next patch.
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
--HG--
rename : mfbt/nsRefPtr.h => mfbt/RefPtr.h
imgFrame::SizeOfExcludingThis() measures heap and non-heap memory in a very
complex way. This patch simplifies it and removes gfxMemoryLocation in the
process. (gfxMemoryLocation::OUT_OF_PROCESS was unused.)
--HG--
extra : rebase_source : 72af38fa438b4b42df02231bcf2fa731d247b60d
Andrew Osmond
Timothy Nikkel
Seth Fowler
Nicholas Nethercote
Chris Peterson
David Rajchenbach-Teller
Sebastian Hengst
Nathan Froyd
Mihai Volmer
Carsten "Tomcat" Book
Birunthan Mohanathas