Bug 275999
| Summary: | REGRESSION(276827@main): Prevent BitmapImageSource from accessing a null ImageDecoder | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> |
| Component: | Images | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | sabouhallawa, webkit-bug-importer |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=265363 | ||
Said Abou-Hallawa
If BitmapImageSource returns EncodedDataStatus::Complete to CachedImage once and was able to get the size of this image, this owner BitmapImage will be marked a valid (not broken) image. It is supposed to always have a valid m_decoder pointer even if we call resetData() which deletes m_decoder because of memory pressure. resetData() will try to recreate the ImageDecoder from the Image encoded data. And it is supposed to be able to recreate it since it did create it before with the same encoded data.
It seems this assumption is not always correct. And we may not be able to recreate the ImageDecoder because of memory limitations.
Before 276827@main we were able to handle this case safely. Below is the calling sequence we will go through to decode and draw an ImageFrame. In the old ImageSource::frameAtIndexCacheIfNeeded() if the decoder is null, we used to return null NativeImage without accessing the decoder:
ImageDrawResult BitmapImage::draw(GraphicsContext& context, const FloatRect& destRect, const FloatRect& requestedSrcRect, ImagePaintingOptions options)
{
...
image = nativeImageAtIndexCacheIfNeeded(m_currentFrame, m_currentSubsamplingLevel, options.decodingMode());
...
}
RefPtr<NativeImage> BitmapImage::nativeImageAtIndexCacheIfNeeded(size_t index, SubsamplingLevel subsamplingLevel, const DecodingOptions& decodingOptions)
{
...
return m_source->frameImageAtIndexCacheIfNeeded(index, subsamplingLevel, decodingOptions);
}
RefPtr<NativeImage> ImageSource::frameImageAtIndexCacheIfNeeded(size_t index, SubsamplingLevel subsamplingLevel, const DecodingOptions& decodingOptions)
{
return frameAtIndexCacheIfNeeded(index, ImageFrame::Caching::MetadataAndImage, subsamplingLevel, decodingOptions).nativeImage();
}
const ImageFrame& ImageSource::frameAtIndexCacheIfNeeded(size_t index, ImageFrame::Caching caching, const std::optional<SubsamplingLevel>& subsamplingLevel, const DecodingOptions& decodingOptions)
{
...
ImageFrame& frame = m_frames[index];
if (!isDecoderAvailable() || frameIsBeingDecodedAndIsCompatibleWithOptionsAtIndex(index, DecodingOptions(DecodingMode::Asynchronous)))
return frame;
...
}
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Said Abou-Hallawa
rdar://129791964
Radar WebKit Bug Importer
<rdar://problem/130764389>
Said Abou-Hallawa
rdar://129791964
Said Abou-Hallawa
Pull request: https://github.com/WebKit/WebKit/pull/30275
EWS
Committed 280540@main (da23487493c6): <https://commits.webkit.org/280540@main>
Reviewed commits have been landed. Closing PR #30275 and removing active labels.