905793 - imgFrame::mDecoded could be out of date, rendering imgFrame::ImageComplete() to be wrong

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

imgFrame::mDecoded appears to only be updated in imgFrame::ImageUpdated, which is in turn only called from Decoder::PostInvalidation(), which is only mean for mid frame calls. So, mDecoded ends up being wrong for images that decode completely and only go through Decoder::PostFrameStop.  For those cases, imgFrame::ImageComplete() result will be wrong.

This was the underlying cause for bug 899861 - we were testing if the frames in the image sequence were "ImageComplete" and they never were, because each frame in the sequence would decode fast enough.  The problem doesn't seem to be limited to image sequences, but in other scenarios the decoding is done when the image is done, so we do the right thing because the decoding is completed.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

FlushImageData() also doesn't get called beyond the first frame, so we don't think any frames are "ImageComplete" until the very end.

Seth, does the above make sense, or am I reading the code wrong?

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Send ImageUpdated() in PostFrameStop().

I can't tell if this is an overkill and we should do this update elsewhere, or in fewer cases.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 790951 [details] [diff] [review]
Send ImageUpdated() in PostFrameStop().

Review of attachment 790951 [details] [diff] [review]:
-----------------------------------------------------------------

As far as I can tell, this is OK. The worst consequence seems to be that you take mDirtyMutex an extra time on the first frame. (Which seems inevitable.)

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 790951 [details] [diff] [review]
Send ImageUpdated() in PostFrameStop().

Jeff's good with Seth's review, taking him off the review?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a5379f0776d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4a5379f0776d