Title: [294669] branches/safari-7613.3.1.0-branch/Source
Revision
294669
Author
alanc...@apple.com
Date
2022-05-23 12:44:15 -0700 (Mon, 23 May 2022)

Log Message

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

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

Modified Paths

Diff

Modified: branches/safari-7613.3.1.0-branch/Source/WebCore/ChangeLog (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/ChangeLog	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/ChangeLog	2022-05-23 19:44:15 UTC (rev 294669)
@@ -1,5 +1,103 @@
 2022-05-23  Alan Coon  <alanc...@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):
+    
+    Canonical link: https://commits.webkit.org/250624@main
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-23  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r289713. rdar://problem/93601919
 
     Expose the correct role, subrole and role description properties for the <dialog> element.

Modified: branches/safari-7613.3.1.0-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp	2022-05-23 19:44:15 UTC (rev 294669)
@@ -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: branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.cpp (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.cpp	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.cpp	2022-05-23 19:44:15 UTC (rev 294669)
@@ -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;
     }
 
@@ -228,7 +230,6 @@
     if (destRect.isEmpty() || requestedSrcRect.isEmpty())
         return ImageDrawResult::DidNothing;
 
-    
     auto srcRect = requestedSrcRect;
     auto preferredSize = size();
     auto srcSize = sourceSize();

Modified: branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.h (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.h	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.h	2022-05-23 19:44:15 UTC (rev 294669)
@@ -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: branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.cpp (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.cpp	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.cpp	2022-05-23 19:44:15 UTC (rev 294669)
@@ -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: branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.h (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.h	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.h	2022-05-23 19:44:15 UTC (rev 294669)
@@ -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: branches/safari-7613.3.1.0-branch/Source/WebKit/ChangeLog (294668 => 294669)


--- branches/safari-7613.3.1.0-branch/Source/WebKit/ChangeLog	2022-05-23 19:44:11 UTC (rev 294668)
+++ branches/safari-7613.3.1.0-branch/Source/WebKit/ChangeLog	2022-05-23 19:44:15 UTC (rev 294669)
@@ -1,3 +1,69 @@
+2022-05-23  Alan Coon  <alanc...@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):
+    
+    Canonical link: https://commits.webkit.org/250624@main
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-02  Alan Coon  <alanc...@apple.com>
 
         Apply patch. rdar://problem/92617943
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to