Changeset 209914 in webkit


Ignore:
Timestamp:
Dec 16, 2016 12:42:29 AM (7 years ago)
Author:
commit-queue@webkit.org
Message:

svg/as-image/svg-image-with-data-uri-use-data-uri.svg is flaky after r207754
​https://bugs.webkit.org/show_bug.cgi?id=163887
<rdar://problem/29266436>

Patch by Youenn Fablet <​youennf@gmail.com> on 2016-12-16
Reviewed by Alex Christensen.

Source/WebCore:

Test: http/tests/security/cross-origin-cached-images-with-memory-pressure.html

With the introduction of cached resource cloning, an Image may be referenced by several CachedImage.
This did not work well with Image observer system as it mandates a one-to-one relationship.

Introducing CachedImageObserver to restore the one-to-one relationship between Image and its observer.
CachedImageObserver can keep references for more than one CachedImage.

In the future, it might be better to split more clearly CachedImageObserver and its API from CachedImage.
Or remove the concept of CachedResource cloning and find new ways to provide CachedResource origin information to clients.

  • loader/cache/CachedImage.cpp:

(WebCore::CachedImage::load): Moved boolean image observer fields to CachedImageObserver.
(WebCore::CachedImage::setBodyDataFrom): Keeping a reference of the image observer when cloning the resource.
(WebCore::CachedImage::createImage): Creating the observer when creating the image.
(WebCore::CachedImage::CachedImageObserver::CachedImageObserver):
(WebCore::CachedImage::CachedImageObserver::decodedSizeChanged):
(WebCore::CachedImage::CachedImageObserver::didDraw):
(WebCore::CachedImage::CachedImageObserver::animationAdvanced):
(WebCore::CachedImage::CachedImageObserver::changedInRect):
(WebCore::CachedImage::clearImage):

  • loader/cache/CachedImage.h:
  • loader/cache/CachedResource.cpp:

(WebCore::CachedResource::setBodyDataFrom): Now that each cached image receives decodedSizeChanged callback, we need to set its size correctly.

LayoutTests:

  • http/tests/security/cross-origin-cached-images-with-memory-pressure-expected.txt: Added.
  • http/tests/security/cross-origin-cached-images-with-memory-pressure.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r209910 r209914  
     12016-12-16  Youenn Fablet  <youennf@gmail.com>
     2
     3        svg/as-image/svg-image-with-data-uri-use-data-uri.svg is flaky after r207754
     4        https://bugs.webkit.org/show_bug.cgi?id=163887
     5        <rdar://problem/29266436>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * http/tests/security/cross-origin-cached-images-with-memory-pressure-expected.txt: Added.
     10        * http/tests/security/cross-origin-cached-images-with-memory-pressure.html: Added.
     11
    1122016-12-15  Zalan Bujtas  <zalan@apple.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r209912 r209914  
     12016-12-16  Youenn Fablet  <youennf@gmail.com>
     2
     3        svg/as-image/svg-image-with-data-uri-use-data-uri.svg is flaky after r207754
     4        https://bugs.webkit.org/show_bug.cgi?id=163887
     5        <rdar://problem/29266436>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Test: http/tests/security/cross-origin-cached-images-with-memory-pressure.html
     10
     11        With the introduction of cached resource cloning, an Image may be referenced by several CachedImage.
     12        This did not work well with Image observer system as it mandates a one-to-one relationship.
     13
     14        Introducing CachedImageObserver to restore the one-to-one relationship between Image and its observer.
     15        CachedImageObserver can keep references for more than one CachedImage.
     16
     17        In the future, it might be better to split more clearly CachedImageObserver and its API from CachedImage.
     18        Or remove the concept of CachedResource cloning and find new ways to provide CachedResource origin information to clients.
     19
     20        * loader/cache/CachedImage.cpp:
     21        (WebCore::CachedImage::load): Moved boolean image observer fields to CachedImageObserver.
     22        (WebCore::CachedImage::setBodyDataFrom): Keeping a reference of the image observer when cloning the resource.
     23        (WebCore::CachedImage::createImage): Creating the observer when creating the image.
     24        (WebCore::CachedImage::CachedImageObserver::CachedImageObserver):
     25        (WebCore::CachedImage::CachedImageObserver::decodedSizeChanged):
     26        (WebCore::CachedImage::CachedImageObserver::didDraw):
     27        (WebCore::CachedImage::CachedImageObserver::animationAdvanced):
     28        (WebCore::CachedImage::CachedImageObserver::changedInRect):
     29        (WebCore::CachedImage::clearImage):
     30        * loader/cache/CachedImage.h:
     31        * loader/cache/CachedResource.cpp:
     32        (WebCore::CachedResource::setBodyDataFrom): Now that each cached image receives decodedSizeChanged callback, we need to set its size correctly.
     33
    1342016-12-15  Joonghun Park  <jh718.park@samsung.com>
    235

  • trunk/Source/WebCore/loader/cache/CachedImage.cpp

    r209159 r209914  
    9090    else
    9191        setLoading(false);
    92 
    93     if (m_loader) {
    94         m_allowSubsampling = m_loader->frameLoader()->frame().settings().imageSubsamplingEnabled();
    95         m_allowLargeImageAsyncDecoding = m_loader->frameLoader()->frame().settings().largeImageAsyncDecodingEnabled();
    96         m_allowAnimatedImageAsyncDecoding = m_loader->frameLoader()->frame().settings().animatedImageAsyncDecodingEnabled();
    97         m_showDebugBackground = m_loader->frameLoader()->frame().settings().showDebugBorders();
    98     }
    9992}
    10093
    … …  
    107100
    108101    m_image = image.m_image;
     102    m_imageObserver = image.m_imageObserver;
     103    if (m_imageObserver)
     104        m_imageObserver->add(*this);
    109105
    110106    if (m_image && is<SVGImage>(*m_image))
    … …  
    325321        return;
    326322
    327 #if USE(CG) && !USE(WEBKIT_IMAGE_DECODERS)
    328     else if (m_response.mimeType() == "application/pdf")
    329         m_image = PDFDocumentImage::create(this);
    330 #endif
    331     else if (m_response.mimeType() == "image/svg+xml") {
    332         auto svgImage = SVGImage::create(*this, url());
     323    m_imageObserver = CachedImageObserver::create(*this);
     324
     325    if (m_response.mimeType() == "image/svg+xml") {
     326        auto svgImage = SVGImage::create(*m_imageObserver, url());
    333327        m_svgImageCache = std::make_unique<SVGImageCache>(svgImage.ptr());
    334328        m_image = WTFMove(svgImage);
     329#if USE(CG) && !USE(WEBKIT_IMAGE_DECODERS)
     330    } else if (m_response.mimeType() == "application/pdf") {
     331        m_image = PDFDocumentImage::create(m_imageObserver.get());
     332#endif
    335333    } else
    336         m_image = BitmapImage::create(this);
     334        m_image = BitmapImage::create(m_imageObserver.get());
    337335
    338336    if (m_image) {
    … …  
    346344}
    347345
     346CachedImage::CachedImageObserver::CachedImageObserver(CachedImage& image)
     347{
     348    m_cachedImages.reserveInitialCapacity(1);
     349    m_cachedImages.append(&image);
     350    if (auto* loader = image.loader()) {
     351        m_allowSubsampling = loader->frameLoader()->frame().settings().imageSubsamplingEnabled();
     352        m_allowLargeImageAsyncDecoding = loader->frameLoader()->frame().settings().largeImageAsyncDecodingEnabled();
     353        m_allowAnimatedImageAsyncDecoding = loader->frameLoader()->frame().settings().animatedImageAsyncDecodingEnabled();
     354        m_showDebugBackground = loader->frameLoader()->frame().settings().showDebugBorders();
     355    }
     356}
     357
     358void CachedImage::CachedImageObserver::decodedSizeChanged(const Image* image, long long delta)
     359{
     360    for (auto cachedImage : m_cachedImages)
     361        cachedImage->decodedSizeChanged(image, delta);
     362}
     363
     364void CachedImage::CachedImageObserver::didDraw(const Image* image)
     365{
     366    for (auto cachedImage : m_cachedImages)
     367        cachedImage->didDraw(image);
     368}
     369
     370void CachedImage::CachedImageObserver::animationAdvanced(const Image* image)
     371{
     372    for (auto cachedImage : m_cachedImages)
     373        cachedImage->animationAdvanced(image);
     374}
     375
     376void CachedImage::CachedImageObserver::changedInRect(const Image* image, const IntRect* rect)
     377{
     378    for (auto cachedImage : m_cachedImages)
     379        cachedImage->changedInRect(image, rect);
     380}
     381
    348382inline void CachedImage::clearImage()
    349383{
    350     // If our Image has an observer, it's always us so we need to clear the back pointer
    351     // before dropping our reference.
    352     if (m_image)
    353         m_image->setImageObserver(nullptr);
     384    if (m_imageObserver) {
     385        m_imageObserver->remove(*this);
     386        m_imageObserver = nullptr;
     387    }
    354388    m_image = nullptr;
    355389}

  • trunk/Source/WebCore/loader/cache/CachedImage.h

    r209159 r209914  
    4444struct Length;
    4545
    46 class CachedImage final : public CachedResource, public ImageObserver {
     46class CachedImage final : public CachedResource {
    4747    friend class MemoryCache;
    4848
    … …  
    117117    bool stillNeedsLoad() const override { return !errorOccurred() && status() == Unknown && !isLoading(); }
    118118
    119     // ImageObserver
    120     bool allowSubsampling() const override { return m_allowSubsampling; }
    121     bool allowLargeImageAsyncDecoding() const override { return m_allowLargeImageAsyncDecoding; }
    122     bool allowAnimatedImageAsyncDecoding() const override { return m_allowAnimatedImageAsyncDecoding; }
    123     bool showDebugBackground() const override { return m_showDebugBackground; }
    124     void decodedSizeChanged(const Image*, long long delta) override;
    125     void didDraw(const Image*) override;
     119    class CachedImageObserver final : public RefCounted<CachedImageObserver>, public ImageObserver {
     120    public:
     121        static Ref<CachedImageObserver> create(CachedImage& image) { return adoptRef(*new CachedImageObserver(image)); }
     122        void add(CachedImage& image) { m_cachedImages.append(&image); }
     123        void remove(CachedImage& image) { m_cachedImages.removeFirst(&image); }
    126124
    127     void animationAdvanced(const Image*) override;
    128     void changedInRect(const Image*, const IntRect* changeRect = nullptr) override;
     125    private:
     126        explicit CachedImageObserver(CachedImage&);
     127
     128        // ImageObserver API
     129        bool allowSubsampling() const final { return m_allowSubsampling; }
     130        bool allowLargeImageAsyncDecoding() const override { return m_allowLargeImageAsyncDecoding; }
     131        bool allowAnimatedImageAsyncDecoding() const override { return m_allowAnimatedImageAsyncDecoding; }
     132        bool showDebugBackground() const final { return m_showDebugBackground; }
     133        void decodedSizeChanged(const Image*, long long delta) final;
     134        void didDraw(const Image*) final;
     135
     136        void animationAdvanced(const Image*) final;
     137        void changedInRect(const Image*, const IntRect*) final;
     138
     139        Vector<CachedImage*> m_cachedImages;
     140        // The default value of m_allowSubsampling should be the same as defaultImageSubsamplingEnabled in Settings.cpp
     141#if PLATFORM(IOS)
     142        bool m_allowSubsampling { true };
     143#else
     144        bool m_allowSubsampling { false };
     145#endif
     146        bool m_allowLargeImageAsyncDecoding { true };
     147        bool m_allowAnimatedImageAsyncDecoding { true };
     148        bool m_showDebugBackground { false };
     149    };
     150
     151    void decodedSizeChanged(const Image*, long long delta);
     152    void didDraw(const Image*);
     153    void animationAdvanced(const Image*);
     154    void changedInRect(const Image*, const IntRect*);
    129155
    130156    void addIncrementalDataBuffer(SharedBuffer&);
    … …  
    136162    ContainerSizeRequests m_pendingContainerSizeRequests;
    137163
     164    RefPtr<CachedImageObserver> m_imageObserver;
    138165    RefPtr<Image> m_image;
    139166    std::unique_ptr<SVGImageCache> m_svgImageCache;
    140167    bool m_isManuallyCached { false };
    141168    bool m_shouldPaintBrokenImage { true };
    142 
    143     // The default value of m_allowSubsampling should be the same as defaultImageSubsamplingEnabled in Settings.cpp
    144 #if PLATFORM(IOS)
    145     bool m_allowSubsampling { true };
    146 #else
    147     bool m_allowSubsampling { false };
    148 #endif
    149     bool m_allowLargeImageAsyncDecoding { true };
    150     bool m_allowAnimatedImageAsyncDecoding { true };
    151     bool m_showDebugBackground { false };
    152169};
    153170

  • trunk/Source/WebCore/loader/cache/CachedResource.cpp

    r209235 r209914  
    298298{
    299299    m_data = resource.m_data;
     300    setDecodedSize(resource.decodedSize());
    300301}
    301302
Note: See TracChangeset for help on using the changeset viewer.