Closed Bug 905793 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: ImageLib, defect)

25 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file)

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.
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?
Flags: needinfo?(seth)
I can't tell if this is an overkill and we should do this update elsewhere, or in fewer cases.
Attachment #790951 - Flags: review?(seth)
Attachment #790951 - Flags: review?(jmuizelaar)
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.)
Attachment #790951 - Flags: review?(seth) → review+
Flags: needinfo?(seth)
Comment on attachment 790951 [details] [diff] [review]
Send ImageUpdated() in PostFrameStop().

Jeff's good with Seth's review, taking him off the review?
Attachment #790951 - Flags: review?(jmuizelaar)
https://hg.mozilla.org/mozilla-central/rev/4a5379f0776d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.