Bug 172325 – [REGRESSION](r216901): Delete ImageDecoder if BitmapImage::destroyDecodedData() was called to destroy all the decoded frames

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Said Abou-Hallawa]

In the change <http://trac.webkit.org/changeset/216901>, the function BitmapImage::destroyDecodedData() was changed such that if destroyAll was true but BitmapImage::destroyDecodedData() returns false, destroyAll was set to false which would lead to not calling ImageSource::clear(). ImageSource::clear() deletes the current ImageDecoder and creates a new one if the Image::data() is not null. Not calling ImageSource::clear() from BitmapImage::destroyDecodedData() when the passed destroyAll is true can cause the following problems:

1) CachedImage::didReplaceSharedBufferContents() calls m_image->destroyDecodedData(true) when the data SharedBuffer is switched and it assumes the current ImageDecoder will be deleted and a new one will be created with the new ShareBuffer.
2) For large images, the ImageDecoder may keep large buffers for raster data. Under memory pressure, the MemoryCache will request all the images to release their decoded frames. Because of https://bugs.webkit.org/show_bug.cgi?id=170640, we can't delete the current decoded frame. But deleting the ImageDecoder itself will release the raster data which will not be needed as long the current decoded frame is still cached.

However for animated images, it is okay not to call ImageSource::clear(). Animating an image happens after receiving all its data. So problem (1) is not a concern here. But deleting the ImageDecoder while animating an image will cause the animation to jitter because the new ImageDecoder has to decode all the frames from 0..currentFrame to be able to decode the nextFrame if it's equal to (currentFrame + 1).

[Said Abou-Hallawa]

Created attachment 310589 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 310589 [details]
Patch

r=me but this code is becoming really hard to reason about.

[Said Abou-Hallawa]

<rdar://problem/32243153>

[WebKit Commit Bot]

Comment on attachment 310589 [details]
Patch

Clearing flags on attachment: 310589

Committed r217096: <http://trac.webkit.org/changeset/217096>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.