Changeset 294669 in webkit


Ignore:
Timestamp:
May 23, 2022 12:44:15 PM (2 years ago)
Author:
Alan Coon
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

Location:
branches/safari-7613.3.1.0-branch/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-7613.3.1.0-branch/Source/WebCore/ChangeLog

    r294667 r294669  
     12022-05-23  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r294280. rdar://problem/87980543
     4
     5    REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     6    https://bugs.webkit.org/show_bug.cgi?id=239113
     7    rdar://87980543
     8   
     9    Reviewed by Simon Fraser.
     10   
     11    Source/WebCore:
     12   
     13    CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
     14    of the animated image can be decoded correctly before creating the temporary
     15    static image. If the first frame can't be decoded, this function should return
     16    immediately. This matches the behavior of this function before r249162.
     17   
     18    The animated image decodes its frames asynchronously in a work queue. But
     19    the first frame has to be decoded synchronously in the main run loop. So
     20    to avoid running the image decoder in two different threads we are going
     21    to keep the first and the current frame cached when we receive a memory
     22    pressure warning. This should not increase the memory allocation of the
     23    animated image because the numbers of cached frames increases quickly and
     24    we keep all of them till a memory warning is received. But the memory
     25    pressure warning will be received a little bit more often. This depends
     26    on the memory size of the first frame.
     27   
     28    To make the code more robust, make ImageSource take a Ref<NativeImage>
     29    instead of taking a RefPtr<NativeImage>.
     30   
     31    * html/canvas/CanvasRenderingContext2DBase.cpp:
     32    (WebCore::CanvasRenderingContext2DBase::drawImage):
     33    * platform/graphics/BitmapImage.cpp:
     34    (WebCore::BitmapImage::BitmapImage):
     35    (WebCore::BitmapImage::destroyDecodedData):
     36    * platform/graphics/BitmapImage.h:
     37    * platform/graphics/ImageSource.cpp:
     38    (WebCore::ImageSource::ImageSource):
     39    (WebCore::ImageSource::destroyDecodedData):
     40    (WebCore::ImageSource::setNativeImage):
     41    * platform/graphics/ImageSource.h:
     42    (WebCore::ImageSource::create):
     43    (WebCore::ImageSource::isDecoderAvailable const):
     44    (WebCore::ImageSource::destroyAllDecodedData): Deleted.
     45    (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
     46    (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
     47   
     48    Source/WebKit:
     49   
     50    * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
     51    (WebKit::RemoteDisplayListRecorder::drawSystemImage):
     52   
     53    Canonical link: https://commits.webkit.org/250624@main
     54    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     55
     56    2022-05-16  Said Abou-Hallawa  <said@apple.com>
     57
     58            REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     59            https://bugs.webkit.org/show_bug.cgi?id=239113
     60            rdar://87980543
     61
     62            Reviewed by Simon Fraser.
     63
     64            CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
     65            of the animated image can be decoded correctly before creating the temporary
     66            static image. If the first frame can't be decoded, this function should return
     67            immediately. This matches the behavior of this function before r249162.
     68
     69            The animated image decodes its frames asynchronously in a work queue. But
     70            the first frame has to be decoded synchronously in the main run loop. So
     71            to avoid running the image decoder in two different threads we are going
     72            to keep the first and the current frame cached when we receive a memory
     73            pressure warning. This should not increase the memory allocation of the
     74            animated image because the numbers of cached frames increases quickly and
     75            we keep all of them till a memory warning is received. But the memory
     76            pressure warning will be received a little bit more often. This depends
     77            on the memory size of the first frame.
     78
     79            To make the code more robust, make ImageSource take a Ref<NativeImage>
     80            instead of taking a RefPtr<NativeImage>.
     81
     82            * html/canvas/CanvasRenderingContext2DBase.cpp:
     83            (WebCore::CanvasRenderingContext2DBase::drawImage):
     84            * platform/graphics/BitmapImage.cpp:
     85            (WebCore::BitmapImage::BitmapImage):
     86            (WebCore::BitmapImage::destroyDecodedData):
     87            * platform/graphics/BitmapImage.h:
     88            * platform/graphics/ImageSource.cpp:
     89            (WebCore::ImageSource::ImageSource):
     90            (WebCore::ImageSource::destroyDecodedData):
     91            (WebCore::ImageSource::setNativeImage):
     92            * platform/graphics/ImageSource.h:
     93            (WebCore::ImageSource::create):
     94            (WebCore::ImageSource::isDecoderAvailable const):
     95            (WebCore::ImageSource::destroyAllDecodedData): Deleted.
     96            (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
     97            (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
     98
    1992022-05-23  Alan Coon  <alancoon@apple.com>
    2100

  • branches/safari-7613.3.1.0-branch/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp

    r287731 r294669  
    15461546    if (image->isBitmapImage()) {
    15471547        // Drawing an animated image to a canvas should draw the first frame (except for a few layout tests)
    1548         if (image->isAnimated() && !document.settings().animatedImageDebugCanvasDrawingEnabled())
     1548        if (image->isAnimated() && !document.settings().animatedImageDebugCanvasDrawingEnabled()) {
    15491549            image = BitmapImage::create(image->nativeImage());
     1550            if (!image)
     1551                return { };
     1552        }
    15501553        downcast<BitmapImage>(*image).updateFromSettings(document.settings());
    15511554    }

  • branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.cpp

    r287829 r294669  
    5353}
    5454
    55 BitmapImage::BitmapImage(RefPtr<NativeImage>&& image, ImageObserver* observer)
    56     : Image(observer)
    57     , m_source(ImageSource::create(WTFMove(image)))
     55BitmapImage::BitmapImage(Ref<NativeImage>&& image)
     56    : m_source(ImageSource::create(WTFMove(image)))
    5857{
    5958}
    … …  
    7877    LOG(Images, "BitmapImage::%s - %p - url: %s", __FUNCTION__, this, sourceURL().string().utf8().data());
    7978
    80     if (!destroyAll)
    81         m_source->destroyDecodedDataBeforeFrame(m_currentFrame);
    82     else if (!canDestroyDecodedData())
    83         m_source->destroyAllDecodedDataExcludeFrame(m_currentFrame);
    84     else {
    85         m_source->destroyAllDecodedData();
     79    if (!destroyAll) {
     80        // Destroy all the frames between frame0 and m_currentFrame.
     81        m_source->destroyDecodedData(1, m_currentFrame);
     82    } else if (!canDestroyDecodedData()) {
     83        // Destroy all the frames except frame0 and m_currentFrame.
     84        m_source->destroyDecodedData(1, m_currentFrame);
     85        m_source->destroyDecodedData(m_currentFrame + 1, frameCount());
     86    } else {
     87        m_source->destroyDecodedData(0, frameCount());
    8688        m_currentFrameDecodingStatus = DecodingStatus::Invalid;
    8789    }
    … …  
    229231        return ImageDrawResult::DidNothing;
    230232
    231    
    232233    auto srcRect = requestedSrcRect;
    233234    auto preferredSize = size();

  • branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/BitmapImage.h

    r287829 r294669  
    5454class BitmapImage final : public Image {
    5555public:
    56     static Ref<BitmapImage> create(PlatformImagePtr&& platformImage, ImageObserver* observer = nullptr)
    57     {
    58         return adoptRef(*new BitmapImage(NativeImage::create(WTFMove(platformImage)), observer));
    59     }
    60     static Ref<BitmapImage> create(RefPtr<NativeImage>&& nativeImage, ImageObserver* observer = nullptr)
    61     {
    62         return adoptRef(*new BitmapImage(WTFMove(nativeImage), observer));
     56    static RefPtr<BitmapImage> create(PlatformImagePtr&& platformImage)
     57    {
     58        return create(NativeImage::create(WTFMove(platformImage)));
     59    }
     60    static RefPtr<BitmapImage> create(RefPtr<NativeImage>&& nativeImage)
     61    {
     62        if (!nativeImage)
     63            return nullptr;
     64        return create(nativeImage.releaseNonNull());
     65    }
     66    static Ref<BitmapImage> create(Ref<NativeImage>&& nativeImage)
     67    {
     68        return adoptRef(*new BitmapImage(WTFMove(nativeImage)));
    6369    }
    6470    static Ref<BitmapImage> create(ImageObserver* observer = nullptr)
    … …  
    155161
    156162private:
    157     WEBCORE_EXPORT BitmapImage(RefPtr<NativeImage>&&, ImageObserver* = nullptr);
     163    WEBCORE_EXPORT BitmapImage(Ref<NativeImage>&&);
    158164    WEBCORE_EXPORT BitmapImage(ImageObserver* = nullptr);
    159165

  • branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.cpp

    r287829 r294669  
    4545}
    4646
    47 ImageSource::ImageSource(RefPtr<NativeImage>&& nativeImage)
     47ImageSource::ImageSource(Ref<NativeImage>&& nativeImage)
    4848    : m_runLoop(RunLoop::current())
    4949{
    … …  
    121121}
    122122
    123 void ImageSource::destroyDecodedData(size_t frameCount, size_t excludeFrame)
    124 {
     123void ImageSource::destroyDecodedData(size_t begin, size_t end)
     124{
     125    if (begin >= end)
     126        return;
     127
     128    ASSERT(end <= m_frames.size());
     129
    125130    unsigned decodedSize = 0;
    126131
    127     ASSERT(frameCount <= m_frames.size());
    128 
    129     for (size_t index = 0; index < frameCount; ++index) {
    130         if (index == excludeFrame)
    131             continue;
     132    for (size_t index = begin; index < end; ++index)
    132133        decodedSize += m_frames[index].clearImage();
    133     }
    134134
    135135    decodedSizeReset(decodedSize);
    … …  
    233233}
    234234
    235 void ImageSource::setNativeImage(RefPtr<NativeImage>&& nativeImage)
     235void ImageSource::setNativeImage(Ref<NativeImage>&& nativeImage)
    236236{
    237237    ASSERT(m_frames.size() == 1);

  • branches/safari-7613.3.1.0-branch/Source/WebCore/platform/graphics/ImageSource.h

    r287829 r294669  
    5252    }
    5353
    54     static Ref<ImageSource> create(RefPtr<NativeImage>&& nativeImage)
     54    static Ref<ImageSource> create(Ref<NativeImage>&& nativeImage)
    5555    {
    5656        return adoptRef(*new ImageSource(WTFMove(nativeImage)));
    … …  
    6363
    6464    unsigned decodedSize() const { return m_decodedSize; }
    65     void destroyAllDecodedData() { destroyDecodedData(frameCount(), frameCount()); }
    66     void destroyAllDecodedDataExcludeFrame(size_t excludeFrame) { destroyDecodedData(frameCount(), excludeFrame); }
    67     void destroyDecodedDataBeforeFrame(size_t beforeFrame) { destroyDecodedData(beforeFrame, beforeFrame); }
     65    void destroyDecodedData(size_t begin, size_t end);
    6866    void destroyIncompleteDecodedData();
    6967    void clearFrameBufferCache(size_t beforeFrame);
    … …  
    129127private:
    130128    ImageSource(BitmapImage*, AlphaOption = AlphaOption::Premultiplied, GammaAndColorProfileOption = GammaAndColorProfileOption::Applied);
    131     ImageSource(RefPtr<NativeImage>&&);
     129    ImageSource(Ref<NativeImage>&&);
    132130
    133131    enum class MetadataType {
    … …  
    154152    bool ensureDecoderAvailable(FragmentedSharedBuffer* data);
    155153    bool isDecoderAvailable() const { return m_decoder; }
    156     void destroyDecodedData(size_t frameCount, size_t excludeFrame);
    157154    void decodedSizeChanged(long long decodedSize);
    158155    void didDecodeProperties(unsigned decodedPropertiesSize);
    … …  
    162159    void encodedDataStatusChanged(EncodedDataStatus);
    163160
    164     void setNativeImage(RefPtr<NativeImage>&&);
     161    void setNativeImage(Ref<NativeImage>&&);
    165162    void cacheMetadataAtIndex(size_t, SubsamplingLevel, DecodingStatus = DecodingStatus::Invalid);
    166163    void cachePlatformImageAtIndex(PlatformImagePtr&&, size_t, SubsamplingLevel, const DecodingOptions&, DecodingStatus = DecodingStatus::Invalid);

  • branches/safari-7613.3.1.0-branch/Source/WebKit/ChangeLog

    r293866 r294669  
     12022-05-23  Alan Coon  <alancoon@apple.com>
     2
     3        Cherry-pick r294280. rdar://problem/87980543
     4
     5    REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     6    https://bugs.webkit.org/show_bug.cgi?id=239113
     7    rdar://87980543
     8   
     9    Reviewed by Simon Fraser.
     10   
     11    Source/WebCore:
     12   
     13    CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
     14    of the animated image can be decoded correctly before creating the temporary
     15    static image. If the first frame can't be decoded, this function should return
     16    immediately. This matches the behavior of this function before r249162.
     17   
     18    The animated image decodes its frames asynchronously in a work queue. But
     19    the first frame has to be decoded synchronously in the main run loop. So
     20    to avoid running the image decoder in two different threads we are going
     21    to keep the first and the current frame cached when we receive a memory
     22    pressure warning. This should not increase the memory allocation of the
     23    animated image because the numbers of cached frames increases quickly and
     24    we keep all of them till a memory warning is received. But the memory
     25    pressure warning will be received a little bit more often. This depends
     26    on the memory size of the first frame.
     27   
     28    To make the code more robust, make ImageSource take a Ref<NativeImage>
     29    instead of taking a RefPtr<NativeImage>.
     30   
     31    * html/canvas/CanvasRenderingContext2DBase.cpp:
     32    (WebCore::CanvasRenderingContext2DBase::drawImage):
     33    * platform/graphics/BitmapImage.cpp:
     34    (WebCore::BitmapImage::BitmapImage):
     35    (WebCore::BitmapImage::destroyDecodedData):
     36    * platform/graphics/BitmapImage.h:
     37    * platform/graphics/ImageSource.cpp:
     38    (WebCore::ImageSource::ImageSource):
     39    (WebCore::ImageSource::destroyDecodedData):
     40    (WebCore::ImageSource::setNativeImage):
     41    * platform/graphics/ImageSource.h:
     42    (WebCore::ImageSource::create):
     43    (WebCore::ImageSource::isDecoderAvailable const):
     44    (WebCore::ImageSource::destroyAllDecodedData): Deleted.
     45    (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
     46    (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
     47   
     48    Source/WebKit:
     49   
     50    * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
     51    (WebKit::RemoteDisplayListRecorder::drawSystemImage):
     52   
     53    Canonical link: https://commits.webkit.org/250624@main
     54    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     55
     56    2022-05-16  Said Abou-Hallawa  <said@apple.com>
     57
     58            REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     59            https://bugs.webkit.org/show_bug.cgi?id=239113
     60            rdar://87980543
     61
     62            Reviewed by Simon Fraser.
     63
     64            * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
     65            (WebKit::RemoteDisplayListRecorder::drawSystemImage):
     66
    1672022-05-02  Alan Coon  <alancoon@apple.com>
    268
Note: See TracChangeset for help on using the changeset viewer.