Title: [294850] branches/safari-613-branch/Source
Revision
294850
Author
[email protected]
Date
2022-05-25 16:50:16 -0700 (Wed, 25 May 2022)

Log Message

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

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

    This reverts r294684.

    Canonical link: https://commits.webkit.org/[email protected]
    git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-7613.3.1.1-branch@294731 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-613-branch/Source/WebCore/ChangeLog (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-05-25 23:50:16 UTC (rev 294850)
@@ -1,5 +1,113 @@
 2022-05-25  Alan Coon  <[email protected]>
 
+        Cherry-pick a173819be243. rdar://problem/87980543
+
+    Cherry-pick r294280. rdar://problem/87980543
+    
+    This reverts r294684.
+    
+    Canonical link: https://commits.webkit.org/[email protected]
+    git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-7613.3.1.1-branch@294731 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-05-23  Alan Coon  <[email protected]>
+
+            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  <[email protected]>
+
+                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-25  Alan Coon  <[email protected]>
+
         Cherry-pick 1a810ee6f98a. rdar://problem/87980543
 
     Expose the correct role, subrole and role description properties for the <dialog> element.

Modified: branches/safari-613-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp	2022-05-25 23:50:16 UTC (rev 294850)
@@ -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-613-branch/Source/WebCore/platform/graphics/BitmapImage.cpp (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/platform/graphics/BitmapImage.cpp	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/platform/graphics/BitmapImage.cpp	2022-05-25 23:50:16 UTC (rev 294850)
@@ -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-613-branch/Source/WebCore/platform/graphics/BitmapImage.h (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/platform/graphics/BitmapImage.h	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/platform/graphics/BitmapImage.h	2022-05-25 23:50:16 UTC (rev 294850)
@@ -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-613-branch/Source/WebCore/platform/graphics/ImageSource.cpp (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/platform/graphics/ImageSource.cpp	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/platform/graphics/ImageSource.cpp	2022-05-25 23:50:16 UTC (rev 294850)
@@ -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-613-branch/Source/WebCore/platform/graphics/ImageSource.h (294849 => 294850)


--- branches/safari-613-branch/Source/WebCore/platform/graphics/ImageSource.h	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebCore/platform/graphics/ImageSource.h	2022-05-25 23:50:16 UTC (rev 294850)
@@ -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-613-branch/Source/WebKit/ChangeLog (294849 => 294850)


--- branches/safari-613-branch/Source/WebKit/ChangeLog	2022-05-25 23:50:11 UTC (rev 294849)
+++ branches/safari-613-branch/Source/WebKit/ChangeLog	2022-05-25 23:50:16 UTC (rev 294850)
@@ -1,3 +1,79 @@
+2022-05-25  Alan Coon  <[email protected]>
+
+        Cherry-pick a173819be243. rdar://problem/87980543
+
+    Cherry-pick r294280. rdar://problem/87980543
+    
+    This reverts r294684.
+    
+    Canonical link: https://commits.webkit.org/[email protected]
+    git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-7613.3.1.1-branch@294731 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-05-23  Alan Coon  <[email protected]>
+
+            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  <[email protected]>
+
+                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  Alan Coon  <[email protected]>
 
         Cherry-pick r293894. rdar://problem/74201964
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to