Skip to content
Permalink
Browse files
Cherry-pick r294280. rdar://problem/87980543
  • Loading branch information
Said Abou-Hallawa authored and alancoon committed May 24, 2022
1 parent 0d98a54 commit a173819be2432024f6c7b9f2d72ab80fb2b2a1aa
Showing 7 changed files with 201 additions and 32 deletions.
@@ -84,6 +84,103 @@
(WebCore::AccessibilityObject::accessibilityIsIgnored const):
Don't call ignoredFromModalPresence if we're in the midst of computing the current modal.

2022-05-23 Alan Coon <alancoon@apple.com>

Cherry-pick r294280. rdar://problem/87980543

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc

2022-05-16 Said Abou-Hallawa <said@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-23 Alan Coon <alancoon@apple.com>

Cherry-pick r289713. rdar://problem/93601919
@@ -1545,8 +1545,11 @@ ExceptionOr<void> CanvasRenderingContext2DBase::drawImage(Document& document, Ca

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());
}

@@ -52,9 +52,8 @@ BitmapImage::BitmapImage(ImageObserver* observer)
{
}

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 @@ void BitmapImage::destroyDecodedData(bool destroyAll)
{
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;
}

@@ -228,7 +230,6 @@ ImageDrawResult BitmapImage::draw(GraphicsContext& context, const FloatRect& des
if (destRect.isEmpty() || requestedSrcRect.isEmpty())
return ImageDrawResult::DidNothing;


auto srcRect = requestedSrcRect;
auto preferredSize = size();
auto srcSize = sourceSize();
@@ -53,13 +53,19 @@ class Timer;

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)
{
@@ -154,7 +160,7 @@ class BitmapImage final : public Image {
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); }
@@ -44,7 +44,7 @@ ImageSource::ImageSource(BitmapImage* image, AlphaOption alphaOption, GammaAndCo
{
}

ImageSource::ImageSource(RefPtr<NativeImage>&& nativeImage)
ImageSource::ImageSource(Ref<NativeImage>&& nativeImage)
: m_runLoop(RunLoop::current())
{
m_frameCount = 1;
@@ -120,17 +120,17 @@ bool ImageSource::isAllDataReceived()
return isDecoderAvailable() ? m_decoder->isAllDataReceived() : frameCount();
}

void ImageSource::destroyDecodedData(size_t frameCount, size_t excludeFrame)
void ImageSource::destroyDecodedData(size_t begin, size_t end)
{
unsigned decodedSize = 0;
if (begin >= end)
return;

ASSERT(frameCount <= m_frames.size());
ASSERT(end <= m_frames.size());

for (size_t index = 0; index < frameCount; ++index) {
if (index == excludeFrame)
continue;
unsigned decodedSize = 0;

for (size_t index = begin; index < end; ++index)
decodedSize += m_frames[index].clearImage();
}

decodedSizeReset(decodedSize);
}
@@ -232,7 +232,7 @@ void ImageSource::growFrames()
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];
@@ -51,7 +51,7 @@ class ImageSource : public ThreadSafeRefCounted<ImageSource>, public CanMakeWeak
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 @@ class ImageSource : public ThreadSafeRefCounted<ImageSource>, public CanMakeWeak
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 @@ class ImageSource : public ThreadSafeRefCounted<ImageSource>, public CanMakeWeak

private:
ImageSource(BitmapImage*, AlphaOption = AlphaOption::Premultiplied, GammaAndColorProfileOption = GammaAndColorProfileOption::Applied);
ImageSource(RefPtr<NativeImage>&&);
ImageSource(Ref<NativeImage>&&);

enum class MetadataType {
AccessibilityDescription = 1 << 0,
@@ -153,15 +151,14 @@ class ImageSource : public ThreadSafeRefCounted<ImageSource>, public CanMakeWeak

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);
void decodedSizeDecreased(unsigned decodedSize);
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);
@@ -1,3 +1,68 @@
2022-05-23 Alan Coon <alancoon@apple.com>

Cherry-pick r294280. rdar://problem/87980543

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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc

2022-05-16 Said Abou-Hallawa <said@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-02 Alan Coon <alancoon@apple.com>

Apply patch. rdar://problem/92617943

0 comments on commit a173819

Please sign in to comment.