When an animated image has been discarded, we avoided marking the
composited frame invalid unless it had been previously decoded. Most of
the time this was fine, but if the animated image was still decoding for
the first time, then we still had a composited frame lingering that we
did not mark as invalid. As a result, when we called
RasterImage::LookupFrame (and indirectly
FrameAnimator::GetCompositedFrame), it would always return the
composited frame. This meant that RasterImage::Decode would never be
called to trigger a redecode. At the same time,
FrameAnimator::RequestRefresh would not cause us to advance the frame
because the state was still discarded.
With this patch we separate out the concepts of "has ever requested to
be decoded" and "has ever completed decoding." The former is now used to
control whether or not a composited frame is marked as invalid after we
discover we currently have no surface for the animation -- this solves
the animation remaining frozen as we now request the redecode as
expected. The latter remains used to determine if we actually know the
total number of frames.
We draw nothing when the composited frame is invalid, so when we mark it valid we should invalidate. Usually the action that causes the composited frame to be valid will invalidate (ie RequestRefresh).
If the SurfaceCache discards our frames on another thread, the runnable that notifies us of that discard could race with a decode complete notification. So we can't rely on any ordering of SetDiscarded and NotifyDecodeComplete. Thus we must derive our state purely from the SurfaceCache (and mAnimationFinished from RasterImage).
We also update the image state in RequestRefresh (the main place where we use the state that is updated).
The other main place we use the state is GetCompositedFrame, but we don't update the state there. It should be fine because the only time this might lag behind reality is if the frames are discarded, and it should be fine to continue drawing the composited frame until the discard notification arrives.
The way that we tell that an animated image has all of its frames complete in the surface cache is less than ideal.
The SurfaceCache can discard on any thread at any time. So we could be in the middle of advancing frames of a fully decoded animated image and then the frames could disappear out from under us.
Making the code deal with that kind of a situation would make the logic very complicated. So instead just look up the frames once and pass them around, that way they never change during while we are advancing the frame.
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.
When we allow animated images to be discarded we still want to track if the image has been fully decoded before, but it would be confusing to say that it is "done decoding" because that sounds like the image is currently decoded, even though it could be discarded at the time.
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.
Also add crashtest with a corrupt animated PNG image that causes a leak to be reported (thanks to these MOZ_COUNT calls). This leak is fixed by the patch on separate bug 1237709.
