- Revision
- 294280
- Author
- s...@apple.com
- Date
- 2022-05-16 16:44:54 -0700 (Mon, 16 May 2022)
Log Message
REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
https://bugs.webkit.org/show_bug.cgi?id=239113
rdar://87980543
Reviewed by Simon Fraser.
Source/WebCore:
CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
of the animated image can be decoded correctly before creating the temporary
static image. If the first frame can't be decoded, this function should return
immediately. This matches the behavior of this function before r249162.
The animated image decodes its frames asynchronously in a work queue. But
the first frame has to be decoded synchronously in the main run loop. So
to avoid running the image decoder in two different threads we are going
to keep the first and the current frame cached when we receive a memory
pressure warning. This should not increase the memory allocation of the
animated image because the numbers of cached frames increases quickly and
we keep all of them till a memory warning is received. But the memory
pressure warning will be received a little bit more often. This depends
on the memory size of the first frame.
To make the code more robust, make ImageSource take a Ref<NativeImage>
instead of taking a RefPtr<NativeImage>.
* html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::drawImage):
* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::BitmapImage):
(WebCore::BitmapImage::destroyDecodedData):
* platform/graphics/BitmapImage.h:
* platform/graphics/ImageSource.cpp:
(WebCore::ImageSource::ImageSource):
(WebCore::ImageSource::destroyDecodedData):
(WebCore::ImageSource::setNativeImage):
* platform/graphics/ImageSource.h:
(WebCore::ImageSource::create):
(WebCore::ImageSource::isDecoderAvailable const):
(WebCore::ImageSource::destroyAllDecodedData): Deleted.
(WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
(WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
Source/WebKit:
* GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
(WebKit::RemoteDisplayListRecorder::drawSystemImage):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (294279 => 294280)
--- trunk/Source/WebCore/ChangeLog 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/ChangeLog 2022-05-16 23:44:54 UTC (rev 294280)
@@ -1,3 +1,46 @@
+2022-05-16 Said Abou-Hallawa <s...@apple.com>
+
+ REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
+ https://bugs.webkit.org/show_bug.cgi?id=239113
+ rdar://87980543
+
+ Reviewed by Simon Fraser.
+
+ CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
+ of the animated image can be decoded correctly before creating the temporary
+ static image. If the first frame can't be decoded, this function should return
+ immediately. This matches the behavior of this function before r249162.
+
+ The animated image decodes its frames asynchronously in a work queue. But
+ the first frame has to be decoded synchronously in the main run loop. So
+ to avoid running the image decoder in two different threads we are going
+ to keep the first and the current frame cached when we receive a memory
+ pressure warning. This should not increase the memory allocation of the
+ animated image because the numbers of cached frames increases quickly and
+ we keep all of them till a memory warning is received. But the memory
+ pressure warning will be received a little bit more often. This depends
+ on the memory size of the first frame.
+
+ To make the code more robust, make ImageSource take a Ref<NativeImage>
+ instead of taking a RefPtr<NativeImage>.
+
+ * html/canvas/CanvasRenderingContext2DBase.cpp:
+ (WebCore::CanvasRenderingContext2DBase::drawImage):
+ * platform/graphics/BitmapImage.cpp:
+ (WebCore::BitmapImage::BitmapImage):
+ (WebCore::BitmapImage::destroyDecodedData):
+ * platform/graphics/BitmapImage.h:
+ * platform/graphics/ImageSource.cpp:
+ (WebCore::ImageSource::ImageSource):
+ (WebCore::ImageSource::destroyDecodedData):
+ (WebCore::ImageSource::setNativeImage):
+ * platform/graphics/ImageSource.h:
+ (WebCore::ImageSource::create):
+ (WebCore::ImageSource::isDecoderAvailable const):
+ (WebCore::ImageSource::destroyAllDecodedData): Deleted.
+ (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
+ (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
+
2022-05-16 Oriol Brufau <obru...@igalia.com>
Take intrinsicBorderForFieldset() into account in intrinsically sized fieldset
Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp (294279 => 294280)
--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp 2022-05-16 23:44:54 UTC (rev 294280)
@@ -1545,8 +1545,11 @@
if (image->isBitmapImage()) {
// Drawing an animated image to a canvas should draw the first frame (except for a few layout tests)
- if (image->isAnimated() && !document.settings().animatedImageDebugCanvasDrawingEnabled())
+ if (image->isAnimated() && !document.settings().animatedImageDebugCanvasDrawingEnabled()) {
image = BitmapImage::create(image->nativeImage());
+ if (!image)
+ return { };
+ }
downcast<BitmapImage>(*image).updateFromSettings(document.settings());
}
Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.cpp (294279 => 294280)
--- trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.cpp 2022-05-16 23:44:54 UTC (rev 294280)
@@ -52,9 +52,8 @@
{
}
-BitmapImage::BitmapImage(RefPtr<NativeImage>&& image, ImageObserver* observer)
- : Image(observer)
- , m_source(ImageSource::create(WTFMove(image)))
+BitmapImage::BitmapImage(Ref<NativeImage>&& image)
+ : m_source(ImageSource::create(WTFMove(image)))
{
}
@@ -77,12 +76,15 @@
{
LOG(Images, "BitmapImage::%s - %p - url: %s", __FUNCTION__, this, sourceURL().string().utf8().data());
- if (!destroyAll)
- m_source->destroyDecodedDataBeforeFrame(m_currentFrame);
- else if (!canDestroyDecodedData())
- m_source->destroyAllDecodedDataExcludeFrame(m_currentFrame);
- else {
- m_source->destroyAllDecodedData();
+ if (!destroyAll) {
+ // Destroy all the frames between frame0 and m_currentFrame.
+ m_source->destroyDecodedData(1, m_currentFrame);
+ } else if (!canDestroyDecodedData()) {
+ // Destroy all the frames except frame0 and m_currentFrame.
+ m_source->destroyDecodedData(1, m_currentFrame);
+ m_source->destroyDecodedData(m_currentFrame + 1, frameCount());
+ } else {
+ m_source->destroyDecodedData(0, frameCount());
m_currentFrameDecodingStatus = DecodingStatus::Invalid;
}
@@ -229,7 +231,6 @@
if (destRect.isEmpty() || requestedSrcRect.isEmpty())
return ImageDrawResult::DidNothing;
-
auto srcRect = requestedSrcRect;
auto preferredSize = size();
auto srcSize = sourceSize();
Modified: trunk/Source/WebCore/platform/graphics/BitmapImage.h (294279 => 294280)
--- trunk/Source/WebCore/platform/graphics/BitmapImage.h 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/platform/graphics/BitmapImage.h 2022-05-16 23:44:54 UTC (rev 294280)
@@ -53,14 +53,20 @@
class BitmapImage final : public Image {
public:
- static Ref<BitmapImage> create(PlatformImagePtr&& platformImage, ImageObserver* observer = nullptr)
+ static RefPtr<BitmapImage> create(PlatformImagePtr&& platformImage)
{
- return adoptRef(*new BitmapImage(NativeImage::create(WTFMove(platformImage)), observer));
+ return create(NativeImage::create(WTFMove(platformImage)));
}
- static Ref<BitmapImage> create(RefPtr<NativeImage>&& nativeImage, ImageObserver* observer = nullptr)
+ static RefPtr<BitmapImage> create(RefPtr<NativeImage>&& nativeImage)
{
- return adoptRef(*new BitmapImage(WTFMove(nativeImage), observer));
+ if (!nativeImage)
+ return nullptr;
+ return create(nativeImage.releaseNonNull());
}
+ static Ref<BitmapImage> create(Ref<NativeImage>&& nativeImage)
+ {
+ return adoptRef(*new BitmapImage(WTFMove(nativeImage)));
+ }
static Ref<BitmapImage> create(ImageObserver* observer = nullptr)
{
return adoptRef(*new BitmapImage(observer));
@@ -154,7 +160,7 @@
void decode(Function<void()>&&);
private:
- WEBCORE_EXPORT BitmapImage(RefPtr<NativeImage>&&, ImageObserver* = nullptr);
+ WEBCORE_EXPORT BitmapImage(Ref<NativeImage>&&);
WEBCORE_EXPORT BitmapImage(ImageObserver* = nullptr);
RefPtr<NativeImage> frameImageAtIndex(size_t index) { return m_source->frameImageAtIndex(index); }
Modified: trunk/Source/WebCore/platform/graphics/ImageSource.cpp (294279 => 294280)
--- trunk/Source/WebCore/platform/graphics/ImageSource.cpp 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/platform/graphics/ImageSource.cpp 2022-05-16 23:44:54 UTC (rev 294280)
@@ -44,7 +44,7 @@
{
}
-ImageSource::ImageSource(RefPtr<NativeImage>&& nativeImage)
+ImageSource::ImageSource(Ref<NativeImage>&& nativeImage)
: m_runLoop(RunLoop::current())
{
m_frameCount = 1;
@@ -120,17 +120,17 @@
return isDecoderAvailable() ? m_decoder->isAllDataReceived() : frameCount();
}
-void ImageSource::destroyDecodedData(size_t frameCount, size_t excludeFrame)
+void ImageSource::destroyDecodedData(size_t begin, size_t end)
{
+ if (begin >= end)
+ return;
+
+ ASSERT(end <= m_frames.size());
+
unsigned decodedSize = 0;
- ASSERT(frameCount <= m_frames.size());
-
- for (size_t index = 0; index < frameCount; ++index) {
- if (index == excludeFrame)
- continue;
+ for (size_t index = begin; index < end; ++index)
decodedSize += m_frames[index].clearImage();
- }
decodedSizeReset(decodedSize);
}
@@ -232,7 +232,7 @@
m_frames.grow(newSize);
}
-void ImageSource::setNativeImage(RefPtr<NativeImage>&& nativeImage)
+void ImageSource::setNativeImage(Ref<NativeImage>&& nativeImage)
{
ASSERT(m_frames.size() == 1);
ImageFrame& frame = m_frames[0];
Modified: trunk/Source/WebCore/platform/graphics/ImageSource.h (294279 => 294280)
--- trunk/Source/WebCore/platform/graphics/ImageSource.h 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebCore/platform/graphics/ImageSource.h 2022-05-16 23:44:54 UTC (rev 294280)
@@ -51,7 +51,7 @@
return adoptRef(*new ImageSource(image, alphaOption, gammaAndColorProfileOption));
}
- static Ref<ImageSource> create(RefPtr<NativeImage>&& nativeImage)
+ static Ref<ImageSource> create(Ref<NativeImage>&& nativeImage)
{
return adoptRef(*new ImageSource(WTFMove(nativeImage)));
}
@@ -62,9 +62,7 @@
bool isAllDataReceived();
unsigned decodedSize() const { return m_decodedSize; }
- void destroyAllDecodedData() { destroyDecodedData(frameCount(), frameCount()); }
- void destroyAllDecodedDataExcludeFrame(size_t excludeFrame) { destroyDecodedData(frameCount(), excludeFrame); }
- void destroyDecodedDataBeforeFrame(size_t beforeFrame) { destroyDecodedData(beforeFrame, beforeFrame); }
+ void destroyDecodedData(size_t begin, size_t end);
void destroyIncompleteDecodedData();
void clearFrameBufferCache(size_t beforeFrame);
@@ -128,7 +126,7 @@
private:
ImageSource(BitmapImage*, AlphaOption = AlphaOption::Premultiplied, GammaAndColorProfileOption = GammaAndColorProfileOption::Applied);
- ImageSource(RefPtr<NativeImage>&&);
+ ImageSource(Ref<NativeImage>&&);
enum class MetadataType {
AccessibilityDescription = 1 << 0,
@@ -153,7 +151,6 @@
bool ensureDecoderAvailable(FragmentedSharedBuffer* data);
bool isDecoderAvailable() const { return m_decoder; }
- void destroyDecodedData(size_t frameCount, size_t excludeFrame);
void decodedSizeChanged(long long decodedSize);
void didDecodeProperties(unsigned decodedPropertiesSize);
void decodedSizeIncreased(unsigned decodedSize);
@@ -161,7 +158,7 @@
void decodedSizeReset(unsigned decodedSize);
void encodedDataStatusChanged(EncodedDataStatus);
- void setNativeImage(RefPtr<NativeImage>&&);
+ void setNativeImage(Ref<NativeImage>&&);
void cacheMetadataAtIndex(size_t, SubsamplingLevel, DecodingStatus = DecodingStatus::Invalid);
void cachePlatformImageAtIndex(PlatformImagePtr&&, size_t, SubsamplingLevel, const DecodingOptions&, DecodingStatus = DecodingStatus::Invalid);
void cachePlatformImageAtIndexAsync(PlatformImagePtr&&, size_t, SubsamplingLevel, const DecodingOptions&, DecodingStatus);
Modified: trunk/Source/WebKit/ChangeLog (294279 => 294280)
--- trunk/Source/WebKit/ChangeLog 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebKit/ChangeLog 2022-05-16 23:44:54 UTC (rev 294280)
@@ -1,3 +1,14 @@
+2022-05-16 Said Abou-Hallawa <s...@apple.com>
+
+ REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
+ https://bugs.webkit.org/show_bug.cgi?id=239113
+ rdar://87980543
+
+ Reviewed by Simon Fraser.
+
+ * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
+ (WebKit::RemoteDisplayListRecorder::drawSystemImage):
+
2022-05-16 Tim Horton <timothy_hor...@apple.com>
Fix the build after 250556@main
Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp (294279 => 294280)
--- trunk/Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp 2022-05-16 23:40:16 UTC (rev 294279)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp 2022-05-16 23:44:54 UTC (rev 294280)
@@ -302,7 +302,7 @@
ASSERT_NOT_REACHED();
return;
}
- badge.setImage(BitmapImage::create(WTFMove(nativeImage)));
+ badge.setImage(BitmapImage::create(nativeImage.releaseNonNull()));
}
#endif
handleItem(DisplayList::DrawSystemImage(systemImage, destinationRect));
