The GIF decoder should yield at the beginning of a new frame, not at the end of the previous frame

RESOLVED FIXED in Firefox 51

Status

()

Core
ImageLib
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: seth, Assigned: seth)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
In bug 1287691 we added code that yields after each frame of an animated image, but there's a minor issue in the way that it was implemented for the GIF decoder. In the GIF case, corrupt GIFs can falsely claim they're not animated; we only discover that they are when, during the decoding process, we discover a second frame. RasterImage has special code that recovers in these cases and redecodes the GIF as an animated image. This presents a quandary, though, because:

- We want to be able to assert that decodes of single-frame images finish after decoding the first frame, without yielding.

- If we yield before we encounter the second frame, we won't get the PostIsAnimated() call that we use to detect the corrupt GIF issue discussed above.

The solution is to yield in the GIF decoder at the beginning of the next frame, not at the end of the previous frame. Obviously these are pretty much the same thing, but by waiting until the beginning of the next frame, we can ensure that PostIsAnimated() gets called if need be.

This actually isn't a problem right now because we don't make the assertion mentioned above, but we'll want to start doing that soon, so we should fix this.
(Assignee)

Updated

a year ago
Summary: yielding GIF decoder → The GIF decoder should yield at the beginning of a new frame, not at the end of the previous frame
(Assignee)

Comment 1

a year ago
Created attachment 8776741 [details] [diff] [review]
Yield in the GIF decoder at the beginning of a new frame, not at the end of the previous frame.

Here's the patch. Just implements what comment 0 describes. Note that this is
already how the PNG decoder does it, so another advantage is that it brings the
behavior of the two animated images decoders closer together.
Attachment #8776741 - Flags: review?(edwin)
(Assignee)

Comment 2

a year ago
Try job here:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1cb569f1c82f
Attachment #8776741 - Flags: review?(edwin) → review+

Comment 3

a year ago
Pushed by seth.bugzilla@blackhail.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8248ff77be0f
Yield in the GIF decoder at the beginning of a new frame, not at the end of the previous frame. r=edwin

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8248ff77be0f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.