Closed Bug 1346510 Opened 5 years ago Closed 5 years ago

don't allow discarding of animated images


(Core :: ImageLib, enhancement)

Not set



Tracking Status
firefox55 --- fixed


(Reporter: tnikkel, Assigned: tnikkel)




(2 files)

Attached file testcase.html
Plot twist! It is currently possible (and has been for quite a while) to discard animated images. All we need is the follow sequence of events.

1. Decode an animated image.
2. Move the animated image out of view (so it is not painted).
3. Call canvas.drawImage on the animated image (or anything else that asks for a first frame only decode). This creates a static entry in the surface cache for this first frame in addition to the animated entry. Because it is a static request we will also start a first frame decode. RasterImage::Decode calls SurfaceCache::UnlockEntries

and bam, the animated frames are now unlocked (even though the RasterImage, and it's entry in the surface cache is still locked.
4. Switch tabs, open about:memory and minimize memory to actual throw away the animated frames.
5. Switch back to the image tab, scroll the image back into view, it will not animate, it will just show the last composited frame forever.

The attached testcase reproduces this, scroll down, click the button, then change tabs and minimize memory.

This is probably responsible for some (most?) of the crashes from bug 1120279.
Attached patch patchSplinter Review
Attachment #8846264 - Flags: review?(aosmond)
Comment on attachment 8846264 [details] [diff] [review]

Review of attachment 8846264 [details] [diff] [review]:

Nice catch!
Attachment #8846264 - Flags: review?(aosmond) → review+
Pushed by
Don't allow the surface cache to unlock the animated frames of an animated image (when discarding of animated images is disabled). r=aosmond
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.