Bug 172461 – A big animated image can be decoded as a large static image before receiving all the data

• Component: Images
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

This case happened while testing a page with 11 large animated images and it happened only with an image with size = 75MB and 377 frames. So getting a layout test for this scenario will not be easy.

This scenario leads to assertions in BitmapImage::imageFrameAvailableAtIndex() and BitmapImage::draw() since these function expects the decoding of an animated image is completely separate from the beginning and from the beginning. We need to handle the case of decoding the first frame for the first repetition different from decoding the other frames and from the other repetitions.

[Said Abou-Hallawa]

Created attachment 310897 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/32332901>

[Radar WebKit Bug Importer]

<rdar://problem/32332897>

[Simon Fraser (smfr)]

Comment on attachment 310897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310897&action=review

> Source/WebCore/ChangeLog:3
> +        A big animated image can be decoded as a large static image before receiving all the data

I don't understand this title. Is it that we fail to animate the image?

> Source/WebCore/ChangeLog:10
> +        A big animated image can be recognized as a large static image. Then the
> +        real frameCount() is fetched once more data is received and the animation
> +        may start even before finishing the decoding of the first frame.

Don't we get the metadata before the first frame, though?

[Said Abou-Hallawa]

Comment on attachment 310897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310897&action=review

>> Source/WebCore/ChangeLog:3
>> +        A big animated image can be decoded as a large static image before receiving all the data
> 
> I don't understand this title. Is it that we fail to animate the image?

I meant the image can be recognized as a static large image because its frameCount() is 1 so we request decoding it as a static large image. Then when more data is received, the value of frameCount becomes > 1 so it begins to animate. When the first frame finishes decoding, BitmapImage::imageFrameAvailableAtIndex() gets called. This function expects to receive the nextFrame not the currentFrame for animated images.

>> Source/WebCore/ChangeLog:10
>> +        may start even before finishing the decoding of the first frame.
> 
> Don't we get the metadata before the first frame, though?

Yes we do. But some of the metadata can change when more data is received. One of these metadata is the frameCount(). See ImageFrameCache::clearMetadata().

[Simon Fraser (smfr)]

Comment on attachment 310897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=310897&action=review

>>> Source/WebCore/ChangeLog:3
>>> +        A big animated image can be decoded as a large static image before receiving all the data
>> 
>> I don't understand this title. Is it that we fail to animate the image?
> 
> I meant the image can be recognized as a static large image because its frameCount() is 1 so we request decoding it as a static large image. Then when more data is received, the value of frameCount becomes > 1 so it begins to animate. When the first frame finishes decoding, BitmapImage::imageFrameAvailableAtIndex() gets called. This function expects to receive the nextFrame not the currentFrame for animated images.

It seems like the better title would be "Avoid moving to the second frame of an animated image before the first frame has finished decoding".

This stuff about "recognizing" that an image is animated is confusing.

[Said Abou-Hallawa]

Created attachment 310960 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 310960 [details]
Patch

Clearing flags on attachment: 310960

Committed r217262: <http://trac.webkit.org/changeset/217262>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.