⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 294280 in webkit


Ignore:
Timestamp:
May 16, 2022, 4:44:54 PM (4 years ago)
Author:
Said Abou-Hallawa
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):

Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r294275 r294280  
     12022-05-16  Said Abou-Hallawa  <said@apple.com>
     2
     3        REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     4        https://bugs.webkit.org/show_bug.cgi?id=239113
     5        rdar://87980543
     6
     7        Reviewed by Simon Fraser.
     8
     9        CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
     10        of the animated image can be decoded correctly before creating the temporary
     11        static image. If the first frame can't be decoded, this function should return
     12        immediately. This matches the behavior of this function before r249162.
     13
     14        The animated image decodes its frames asynchronously in a work queue. But
     15        the first frame has to be decoded synchronously in the main run loop. So
     16        to avoid running the image decoder in two different threads we are going
     17        to keep the first and the current frame cached when we receive a memory
     18        pressure warning. This should not increase the memory allocation of the
     19        animated image because the numbers of cached frames increases quickly and
     20        we keep all of them till a memory warning is received. But the memory
     21        pressure warning will be received a little bit more often. This depends
     22        on the memory size of the first frame.
     23
     24        To make the code more robust, make ImageSource take a Ref<NativeImage>
     25        instead of taking a RefPtr<NativeImage>.
     26
     27        * html/canvas/CanvasRenderingContext2DBase.cpp:
     28        (WebCore::CanvasRenderingContext2DBase::drawImage):
     29        * platform/graphics/BitmapImage.cpp:
     30        (WebCore::BitmapImage::BitmapImage):
     31        (WebCore::BitmapImage::destroyDecodedData):
     32        * platform/graphics/BitmapImage.h:
     33        * platform/graphics/ImageSource.cpp:
     34        (WebCore::ImageSource::ImageSource):
     35        (WebCore::ImageSource::destroyDecodedData):
     36        (WebCore::ImageSource::setNativeImage):
     37        * platform/graphics/ImageSource.h:
     38        (WebCore::ImageSource::create):
     39        (WebCore::ImageSource::isDecoderAvailable const):
     40        (WebCore::ImageSource::destroyAllDecodedData): Deleted.
     41        (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
     42        (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
     43
    1442022-05-16  Oriol Brufau  <obrufau@igalia.com>
    245

  • trunk/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp

    r294057 r294280  
    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    }

  • trunk/Source/WebCore/platform/graphics/BitmapImage.cpp

    r292469 r294280  
    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    }
    … …  
    230232        return ImageDrawResult::DidNothing;
    231233
    232    
    233234    auto srcRect = requestedSrcRect;
    234235    auto preferredSize = size();

  • trunk/Source/WebCore/platform/graphics/BitmapImage.h

    r289981 r294280  
    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

  • trunk/Source/WebCore/platform/graphics/ImageSource.cpp

    r290901 r294280  
    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);

  • trunk/Source/WebCore/platform/graphics/ImageSource.h

    r287829 r294280  
    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);

  • trunk/Source/WebKit/ChangeLog

    r294274 r294280  
     12022-05-16  Said Abou-Hallawa  <said@apple.com>
     2
     3        REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
     4        https://bugs.webkit.org/show_bug.cgi?id=239113
     5        rdar://87980543
     6
     7        Reviewed by Simon Fraser.
     8
     9        * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
     10        (WebKit::RemoteDisplayListRecorder::drawSystemImage):
     11
    1122022-05-16  Tim Horton  <timothy_horton@apple.com>
    213

  • trunk/Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.cpp

    r292319 r294280  
    303303            return;
    304304        }
    305         badge.setImage(BitmapImage::create(WTFMove(nativeImage)));
     305        badge.setImage(BitmapImage::create(nativeImage.releaseNonNull()));
    306306    }
    307307#endif
Note: See TracChangeset for help on using the changeset viewer.