Changeset 213833 in webkit


Ignore:
Timestamp:
Mar 13, 2017 5:01:02 AM (7 years ago)
Author:
magomez@igalia.com
Message:

ImageDecoder can be deleted while the async decoder thread is still using it
​https://bugs.webkit.org/show_bug.cgi?id=169199

Reviewed by Carlos Garcia Campos.

Make the image decoder used by ImageSource and ImageFrameCache into a RefPtr instead of
and unique_ptr, and pass a reference to the decoder thread. This ensures that the decoder
will stay alive as long as the decoding thread is processing frames. Also, stop the async
decoding queue if a new decoder is set to ImageFrameCache.

No new tests.

  • platform/graphics/ImageFrameCache.cpp:

(WebCore::ImageFrameCache::setDecoder):
(WebCore::ImageFrameCache::decoder):
(WebCore::ImageFrameCache::startAsyncDecodingQueue):
(WebCore::ImageFrameCache::metadata):

  • platform/graphics/ImageFrameCache.h:

(WebCore::ImageFrameCache::setDecoder): Deleted.
Moved to source file so we can keep the ImageDecoder forward declaration.
(WebCore::ImageFrameCache::decoder): Deleted.
Moved to source file so we can keep the ImageDecoder forward declaration.

  • platform/graphics/ImageSource.h:
  • platform/graphics/cg/ImageDecoderCG.h:

(WebCore::ImageDecoder::create):

  • platform/graphics/win/ImageDecoderDirect2D.h:

(WebCore::ImageDecoder::create):

  • platform/image-decoders/ImageDecoder.cpp:

(WebCore::ImageDecoder::create):

  • platform/image-decoders/ImageDecoder.h:
Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r213831 r213833  
     12017-03-13  Miguel Gomez  <magomez@igalia.com>
     2
     3        ImageDecoder can be deleted while the async decoder thread is still using it
     4        https://bugs.webkit.org/show_bug.cgi?id=169199
     5
     6        Reviewed by Carlos Garcia Campos.
     7
     8        Make the image decoder used by ImageSource and ImageFrameCache into a RefPtr instead of
     9        and unique_ptr, and pass a reference to the decoder thread. This ensures that the decoder
     10        will stay alive as long as the decoding thread is processing frames. Also, stop the async
     11        decoding queue if a new decoder is set to ImageFrameCache.
     12
     13        No new tests.
     14
     15        * platform/graphics/ImageFrameCache.cpp:
     16        (WebCore::ImageFrameCache::setDecoder):
     17        (WebCore::ImageFrameCache::decoder):
     18        (WebCore::ImageFrameCache::startAsyncDecodingQueue):
     19        (WebCore::ImageFrameCache::metadata):
     20        * platform/graphics/ImageFrameCache.h:
     21        (WebCore::ImageFrameCache::setDecoder): Deleted.
     22        Moved to source file so we can keep the ImageDecoder forward declaration.
     23        (WebCore::ImageFrameCache::decoder): Deleted.
     24        Moved to source file so we can keep the ImageDecoder forward declaration.
     25        * platform/graphics/ImageSource.h:
     26        * platform/graphics/cg/ImageDecoderCG.h:
     27        (WebCore::ImageDecoder::create):
     28        * platform/graphics/win/ImageDecoderDirect2D.h:
     29        (WebCore::ImageDecoder::create):
     30        * platform/image-decoders/ImageDecoder.cpp:
     31        (WebCore::ImageDecoder::create):
     32        * platform/image-decoders/ImageDecoder.h:
     33
    1342017-03-13  Manuel Rego Casasnovas  <rego@igalia.com>
    235

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

    r213764 r213833  
    7171}
    7272
     73void ImageFrameCache::setDecoder(ImageDecoder* decoder)
     74{
     75    if (m_decoder == decoder)
     76        return;
     77
     78    // Changing the decoder has to stop the decoding thread. The current frame will
     79    // continue decoding safely because the decoding thread has its own
     80    // reference of the old decoder.
     81    stopAsyncDecodingQueue();
     82    m_decoder = decoder;
     83}
     84
     85ImageDecoder* ImageFrameCache::decoder() const
     86{
     87    return m_decoder.get();
     88}
     89
    7390void ImageFrameCache::destroyDecodedData(size_t frameCount, size_t excludeFrame)
    7491{
    … …  
    271288    Ref<ImageFrameCache> protectedThis = Ref<ImageFrameCache>(*this);
    272289    Ref<WorkQueue> protectedQueue = decodingQueue();
    273 
    274     // We need to protect this and m_decodingQueue from being deleted while we are in the decoding loop.
    275     decodingQueue()->dispatch([this, protectedThis = WTFMove(protectedThis), protectedQueue = WTFMove(protectedQueue)] {
     290    Ref<ImageDecoder> protectedDecoder = Ref<ImageDecoder>(*m_decoder);
     291
     292    // We need to protect this, m_decodingQueue and m_decoder from being deleted while we are in the decoding loop.
     293    decodingQueue()->dispatch([this, protectedThis = WTFMove(protectedThis), protectedQueue = WTFMove(protectedQueue), protectedDecoder = WTFMove(protectedDecoder)] {
    276294        ImageFrameRequest frameRequest;
    277295
    278296        while (m_frameRequestQueue.dequeue(frameRequest)) {
    279297            // Get the frame NativeImage on the decoding thread.
    280             NativeImagePtr nativeImage = m_decoder->createFrameImageAtIndex(frameRequest.index, frameRequest.subsamplingLevel, frameRequest.sizeForDrawing);
     298            NativeImagePtr nativeImage = protectedDecoder->createFrameImageAtIndex(frameRequest.index, frameRequest.subsamplingLevel, frameRequest.sizeForDrawing);
    281299
    282300            // Update the cached frames on the main thread to avoid updating the MemoryCache from a different thread.
    … …  
    386404
    387405    if (!cachedValue)
    388         return (m_decoder->*functor)();
    389 
    390     *cachedValue = (m_decoder->*functor)();
     406        return (*m_decoder.*functor)();
     407
     408    *cachedValue = (*m_decoder.*functor)();
    391409    didDecodeProperties(m_decoder->bytesDecodedToDetermineProperties());
    392410    return cachedValue->value();

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

    r213764 r213833  
    5656    ~ImageFrameCache();
    5757
    58     void setDecoder(ImageDecoder* decoder) { m_decoder = decoder; }
    59     ImageDecoder* decoder() const { return m_decoder; }
     58    void setDecoder(ImageDecoder*);
     59    ImageDecoder* decoder() const;
    6060
    6161    unsigned decodedSize() const { return m_decodedSize; }
    … …  
    137137
    138138    Image* m_image { nullptr };
    139     ImageDecoder* m_decoder { nullptr };
     139    RefPtr<ImageDecoder> m_decoder;
    140140    unsigned m_decodedSize { 0 };
    141141    unsigned m_decodedPropertiesSize { 0 };

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

    r213764 r213833  
    113113
    114114    Ref<ImageFrameCache> m_frameCache;
    115     std::unique_ptr<ImageDecoder> m_decoder;
     115    RefPtr<ImageDecoder> m_decoder;
    116116
    117117    std::optional<SubsamplingLevel> m_maximumSubsamplingLevel;

  • trunk/Source/WebCore/platform/graphics/cg/ImageDecoderCG.h

    r213563 r213833  
    3636namespace WebCore {
    3737
    38 class ImageDecoder {
     38class ImageDecoder : public RefCounted<ImageDecoder> {
    3939    WTF_MAKE_FAST_ALLOCATED;
    4040public:
    4141    ImageDecoder(AlphaOption, GammaAndColorProfileOption);
    4242
    43     static std::unique_ptr<ImageDecoder> create(const SharedBuffer&, AlphaOption alphaOption, GammaAndColorProfileOption gammaAndColorProfileOption)
     43    static Ref<ImageDecoder> create(const SharedBuffer&, AlphaOption alphaOption, GammaAndColorProfileOption gammaAndColorProfileOption)
    4444    {
    45         return std::make_unique<ImageDecoder>(alphaOption, gammaAndColorProfileOption);
     45        return adoptRef(*new ImageDecoder(alphaOption, gammaAndColorProfileOption));
    4646    }
    4747   

  • trunk/Source/WebCore/platform/graphics/win/ImageDecoderDirect2D.h

    r213563 r213833  
    3737namespace WebCore {
    3838
    39 class ImageDecoder {
     39class ImageDecoder : public RefCounted<ImageDecoder> {
    4040    WTF_MAKE_FAST_ALLOCATED;
    4141public:
    4242    ImageDecoder();
    4343   
    44     static std::unique_ptr<ImageDecoder> create(const SharedBuffer&, AlphaOption, GammaAndColorProfileOption)
     44    static Ref<ImageDecoder> create(const SharedBuffer&, AlphaOption, GammaAndColorProfileOption)
    4545    {
    46         return std::make_unique<ImageDecoder>();
     46        return adoptRef(*new ImageDecoder());
    4747    }
    4848   

  • trunk/Source/WebCore/platform/image-decoders/ImageDecoder.cpp

    r213563 r213833  
    9797}
    9898
    99 std::unique_ptr<ImageDecoder> ImageDecoder::create(const SharedBuffer& data, AlphaOption alphaOption, GammaAndColorProfileOption gammaAndColorProfileOption)
     99RefPtr<ImageDecoder> ImageDecoder::create(const SharedBuffer& data, AlphaOption alphaOption, GammaAndColorProfileOption gammaAndColorProfileOption)
    100100{
    101101    static const unsigned lengthOfLongestSignature = 14; // To wit: "RIFF????WEBPVP"
    … …  
    106106
    107107    if (matchesGIFSignature(contents))
    108         return std::unique_ptr<ImageDecoder> { std::make_unique<GIFImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     108        return adoptRef(*new GIFImageDecoder(alphaOption, gammaAndColorProfileOption));
    109109
    110110    if (matchesPNGSignature(contents))
    111         return std::unique_ptr<ImageDecoder> { std::make_unique<PNGImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     111        return adoptRef(*new PNGImageDecoder(alphaOption, gammaAndColorProfileOption));
    112112
    113113    if (matchesICOSignature(contents) || matchesCURSignature(contents))
    114         return std::unique_ptr<ImageDecoder> { std::make_unique<ICOImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     114        return adoptRef(*new ICOImageDecoder(alphaOption, gammaAndColorProfileOption));
    115115
    116116    if (matchesJPEGSignature(contents))
    117         return std::unique_ptr<ImageDecoder> { std::make_unique<JPEGImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     117        return adoptRef(*new JPEGImageDecoder(alphaOption, gammaAndColorProfileOption));
    118118
    119119#if USE(WEBP)
    120120    if (matchesWebPSignature(contents))
    121         return std::unique_ptr<ImageDecoder> { std::make_unique<WEBPImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     121        return adoptRef(*new WEBPImageDecoder(alphaOption, gammaAndColorProfileOption));
    122122#endif
    123123
    124124    if (matchesBMPSignature(contents))
    125         return std::unique_ptr<ImageDecoder> { std::make_unique<BMPImageDecoder>(alphaOption, gammaAndColorProfileOption) };
     125        return adoptRef(*new BMPImageDecoder(alphaOption, gammaAndColorProfileOption));
    126126
    127127    return nullptr;

  • trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h

    r213563 r213833  
    4848    // at decode time.  Image decoders will downsample any images larger than
    4949    // |m_maxNumPixels|.  FIXME: Not yet supported by all decoders.
    50     class ImageDecoder {
     50    class ImageDecoder : public RefCounted<ImageDecoder> {
    5151        WTF_MAKE_NONCOPYABLE(ImageDecoder); WTF_MAKE_FAST_ALLOCATED;
    5252    public:
    … …  
    6161        }
    6262
    63         // Returns a caller-owned decoder of the appropriate type.  Returns 0 if
    64         // we can't sniff a supported type from the provided data (possibly
     63        // Returns nullptr if we can't sniff a supported type from the provided data (possibly
    6564        // because there isn't enough data yet).
    66         static std::unique_ptr<ImageDecoder> create(const SharedBuffer& data, AlphaOption, GammaAndColorProfileOption);
     65        static RefPtr<ImageDecoder> create(const SharedBuffer& data, AlphaOption, GammaAndColorProfileOption);
    6766
    6867        virtual String filenameExtension() const = 0;
Note: See TracChangeset for help on using the changeset viewer.