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

Changeset 294694 in webkit


Ignore:
Timestamp:
May 23, 2022, 4:28:36 PM (4 years ago)
Author:
Alan Coon
Message:

Revert r294280. rdar://problem/87980543

This reverts r294672.

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

    r294672 r294694  
    8585            (WebCore::AccessibilityObject::accessibilityIsIgnored const):
    8686            Don't call ignoredFromModalPresence if we're in the midst of computing the current modal.
    87 
    88 2022-05-23  Alan Coon  <alancoon@apple.com>
    89 
    90         Cherry-pick r294280. rdar://problem/87980543
    91 
    92     REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
    93     https://bugs.webkit.org/show_bug.cgi?id=239113
    94     rdar://87980543
    95    
    96     Reviewed by Simon Fraser.
    97    
    98     Source/WebCore:
    99    
    100     CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
    101     of the animated image can be decoded correctly before creating the temporary
    102     static image. If the first frame can't be decoded, this function should return
    103     immediately. This matches the behavior of this function before r249162.
    104    
    105     The animated image decodes its frames asynchronously in a work queue. But
    106     the first frame has to be decoded synchronously in the main run loop. So
    107     to avoid running the image decoder in two different threads we are going
    108     to keep the first and the current frame cached when we receive a memory
    109     pressure warning. This should not increase the memory allocation of the
    110     animated image because the numbers of cached frames increases quickly and
    111     we keep all of them till a memory warning is received. But the memory
    112     pressure warning will be received a little bit more often. This depends
    113     on the memory size of the first frame.
    114    
    115     To make the code more robust, make ImageSource take a Ref<NativeImage>
    116     instead of taking a RefPtr<NativeImage>.
    117    
    118     * html/canvas/CanvasRenderingContext2DBase.cpp:
    119     (WebCore::CanvasRenderingContext2DBase::drawImage):
    120     * platform/graphics/BitmapImage.cpp:
    121     (WebCore::BitmapImage::BitmapImage):
    122     (WebCore::BitmapImage::destroyDecodedData):
    123     * platform/graphics/BitmapImage.h:
    124     * platform/graphics/ImageSource.cpp:
    125     (WebCore::ImageSource::ImageSource):
    126     (WebCore::ImageSource::destroyDecodedData):
    127     (WebCore::ImageSource::setNativeImage):
    128     * platform/graphics/ImageSource.h:
    129     (WebCore::ImageSource::create):
    130     (WebCore::ImageSource::isDecoderAvailable const):
    131     (WebCore::ImageSource::destroyAllDecodedData): Deleted.
    132     (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
    133     (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
    134    
    135     Source/WebKit:
    136    
    137     * GPUProcess/graphics/RemoteDisplayListRecorder.cpp:
    138     (WebKit::RemoteDisplayListRecorder::drawSystemImage):
    139    
    140     Canonical link: https://commits.webkit.org/250624@main
    141     git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294280 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    142 
    143     2022-05-16  Said Abou-Hallawa  <said@apple.com>
    144 
    145             REGRESSION(r249162): CanvasRenderingContext2DBase::drawImage() crashes if the image is animated and the first frame cannot be decoded
    146             https://bugs.webkit.org/show_bug.cgi?id=239113
    147             rdar://87980543
    148 
    149             Reviewed by Simon Fraser.
    150 
    151             CanvasRenderingContext2DBase::drawImage() needs to ensure the first frame
    152             of the animated image can be decoded correctly before creating the temporary
    153             static image. If the first frame can't be decoded, this function should return
    154             immediately. This matches the behavior of this function before r249162.
    155 
    156             The animated image decodes its frames asynchronously in a work queue. But
    157             the first frame has to be decoded synchronously in the main run loop. So
    158             to avoid running the image decoder in two different threads we are going
    159             to keep the first and the current frame cached when we receive a memory
    160             pressure warning. This should not increase the memory allocation of the
    161             animated image because the numbers of cached frames increases quickly and
    162             we keep all of them till a memory warning is received. But the memory
    163             pressure warning will be received a little bit more often. This depends
    164             on the memory size of the first frame.
    165 
    166             To make the code more robust, make ImageSource take a Ref<NativeImage>
    167             instead of taking a RefPtr<NativeImage>.
    168 
    169             * html/canvas/CanvasRenderingContext2DBase.cpp:
    170             (WebCore::CanvasRenderingContext2DBase::drawImage):
    171             * platform/graphics/BitmapImage.cpp:
    172             (WebCore::BitmapImage::BitmapImage):
    173             (WebCore::BitmapImage::destroyDecodedData):
    174             * platform/graphics/BitmapImage.h:
    175             * platform/graphics/ImageSource.cpp:
    176             (WebCore::ImageSource::ImageSource):
    177             (WebCore::ImageSource::destroyDecodedData):
    178             (WebCore::ImageSource::setNativeImage):
    179             * platform/graphics/ImageSource.h:
    180             (WebCore::ImageSource::create):
    181             (WebCore::ImageSource::isDecoderAvailable const):
    182             (WebCore::ImageSource::destroyAllDecodedData): Deleted.
    183             (WebCore::ImageSource::destroyAllDecodedDataExcludeFrame): Deleted.
    184             (WebCore::ImageSource::destroyDecodedDataBeforeFrame): Deleted.
    18587
    186882022-05-23  Alan Coon  <alancoon@apple.com>

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

    r294669 r294694  
    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         }
    15531550        downcast<BitmapImage>(*image).updateFromSettings(document.settings());
    15541551    }

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

    r294669 r294694  
    5353}
    5454
    55 BitmapImage::BitmapImage(Ref<NativeImage>&& image)
    56     : m_source(ImageSource::create(WTFMove(image)))
     55BitmapImage::BitmapImage(RefPtr<NativeImage>&& image, ImageObserver* observer)
     56    : Image(observer)
     57    , m_source(ImageSource::create(WTFMove(image)))
    5758{
    5859}
    … …  
    7778    LOG(Images, "BitmapImage::%s - %p - url: %s", __FUNCTION__, this, sourceURL().string().utf8().data());
    7879
    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());
     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();
    8886        m_currentFrameDecodingStatus = DecodingStatus::Invalid;
    8987    }
    … …  
    231229        return ImageDrawResult::DidNothing;
    232230
     231   
    233232    auto srcRect = requestedSrcRect;
    234233    auto preferredSize = size();

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

    r294669 r294694  
    5454class BitmapImage final : public Image {
    5555public:
    56     static RefPtr<BitmapImage> create(PlatformImagePtr&& platformImage)
     56    static Ref<BitmapImage> create(PlatformImagePtr&& platformImage, ImageObserver* observer = nullptr)
    5757    {
    58         return create(NativeImage::create(WTFMove(platformImage)));
     58        return adoptRef(*new BitmapImage(NativeImage::create(WTFMove(platformImage)), observer));
    5959    }
    60     static RefPtr<BitmapImage> create(RefPtr<NativeImage>&& nativeImage)
     60    static Ref<BitmapImage> create(RefPtr<NativeImage>&& nativeImage, ImageObserver* observer = nullptr)
    6161    {
    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)));
     62        return adoptRef(*new BitmapImage(WTFMove(nativeImage), observer));
    6963    }
    7064    static Ref<BitmapImage> create(ImageObserver* observer = nullptr)
    … …  
    161155
    162156private:
    163     WEBCORE_EXPORT BitmapImage(Ref<NativeImage>&&);
     157    WEBCORE_EXPORT BitmapImage(RefPtr<NativeImage>&&, ImageObserver* = nullptr);
    164158    WEBCORE_EXPORT BitmapImage(ImageObserver* = nullptr);
    165159

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

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

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

    r294669 r294694  
    5252    }
    5353
    54     static Ref<ImageSource> create(Ref<NativeImage>&& nativeImage)
     54    static Ref<ImageSource> create(RefPtr<NativeImage>&& nativeImage)
    5555    {
    5656        return adoptRef(*new ImageSource(WTFMove(nativeImage)));
    … …  
    6363
    6464    unsigned decodedSize() const { return m_decodedSize; }
    65     void destroyDecodedData(size_t begin, size_t end);
     65    void destroyAllDecodedData() { destroyDecodedData(frameCount(), frameCount()); }
     66    void destroyAllDecodedDataExcludeFrame(size_t excludeFrame) { destroyDecodedData(frameCount(), excludeFrame); }
     67    void destroyDecodedDataBeforeFrame(size_t beforeFrame) { destroyDecodedData(beforeFrame, beforeFrame); }
    6668    void destroyIncompleteDecodedData();
    6769    void clearFrameBufferCache(size_t beforeFrame);
    … …  
    127129private:
    128130    ImageSource(BitmapImage*, AlphaOption = AlphaOption::Premultiplied, GammaAndColorProfileOption = GammaAndColorProfileOption::Applied);
    129     ImageSource(Ref<NativeImage>&&);
     131    ImageSource(RefPtr<NativeImage>&&);
    130132
    131133    enum class MetadataType {
    … …  
    152154    bool ensureDecoderAvailable(FragmentedSharedBuffer* data);
    153155    bool isDecoderAvailable() const { return m_decoder; }
     156    void destroyDecodedData(size_t frameCount, size_t excludeFrame);
    154157    void decodedSizeChanged(long long decodedSize);
    155158    void didDecodeProperties(unsigned decodedPropertiesSize);
    … …  
    159162    void encodedDataStatusChanged(EncodedDataStatus);
    160163
    161     void setNativeImage(Ref<NativeImage>&&);
     164    void setNativeImage(RefPtr<NativeImage>&&);
    162165    void cacheMetadataAtIndex(size_t, SubsamplingLevel, DecodingStatus = DecodingStatus::Invalid);
    163166    void cachePlatformImageAtIndex(PlatformImagePtr&&, size_t, SubsamplingLevel, const DecodingOptions&, DecodingStatus = DecodingStatus::Invalid);

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

    r294669 r294694  
    1 2022-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 
    6712022-05-02  Alan Coon  <alancoon@apple.com>
    682
Note: See TracChangeset for help on using the changeset viewer.