Reverse ownership relation of StyleCachedImage and CSSImageValue
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Sep 2016 14:27:58 +0000 (14:27 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Sep 2016 14:27:58 +0000 (14:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161447

Reviewed by Andreas Kling.

Currently StyleCachedImage (which represents an image in RenderStyle) has a weak ref to the
underlying CSSImageValue/CSSImageSetValue which actually owns it. This is awkwards especially since
StyleGeneratedImage, the other StyleImage subclass has reversed relationship where it refs
the underlying CSSImageGeneratorValue.

This patch makes StyleCachedImage similar to StyleGeneratedImage. StyleCachedImage now refs the
underlying CSSImageValue/CSSImageSetValue. CSSImageValues no longer need to know about StyleCachedImage.
Instead they reference CachedImages (memory cache objects) directly. StyleCachedImage instances are now
conceptually unique to RenderStyle instances. Actual resources are shared as before by sharing CachedImages.

* css/CSSCursorImageValue.cpp:
(WebCore::CSSCursorImageValue::loadImage):
(WebCore::CSSCursorImageValue::cachedImage):
(WebCore::CSSCursorImageValue::styleImage): Deleted.
* css/CSSCursorImageValue.h:
* css/CSSImageGeneratorValue.cpp:
(WebCore::CSSImageGeneratorValue::cachedImageForCSSValue):
* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::~CSSImageSetValue):
(WebCore::CSSImageSetValue::loadBestFitImage):
(WebCore::CSSImageSetValue::traverseSubresources):
(WebCore::CSSImageSetValue::styleImage): Deleted.
* css/CSSImageSetValue.h:
* css/CSSImageValue.cpp:
(WebCore::CSSImageValue::CSSImageValue):
(WebCore::CSSImageValue::~CSSImageValue):
(WebCore::CSSImageValue::isPending):
(WebCore::CSSImageValue::loadImage):
(WebCore::CSSImageValue::traverseSubresources):
(WebCore::CSSImageValue::knownToBeOpaque):
(WebCore::CSSImageValue::styleImage): Deleted.
* css/CSSImageValue.h:
* css/StyleBuilderCustom.h:
(WebCore::StyleBuilderCustom::applyValueContent):
* css/StyleResolver.cpp:
(WebCore::StyleResolver::styleImage):
(WebCore::StyleResolver::styleCachedImageFromValue):
(WebCore::StyleResolver::styleGeneratedImageFromValue):
(WebCore::StyleResolver::cachedOrPendingFromValue): Deleted.
(WebCore::StyleResolver::generatedOrPendingFromValue): Deleted.
(WebCore::StyleResolver::setOrPendingFromValue): Deleted.
(WebCore::StyleResolver::cursorOrPendingFromValue): Deleted.
* css/StyleResolver.h:
* editing/TextIterator.cpp:
(WebCore::fullyClipsContents):
* page/PageSerializer.cpp:
(WebCore::PageSerializer::retrieveResourcesForProperties):
* rendering/style/FillLayer.cpp:
(WebCore::FillLayer::imagesIdentical):

    Compare data equality instead of pointer equality for StyleImages (since StyleImages are no longer shared).

(WebCore::layerImagesIdentical): Deleted.
* rendering/style/StyleCachedImage.cpp:
(WebCore::StyleCachedImage::StyleCachedImage):
(WebCore::StyleCachedImage::~StyleCachedImage):
(WebCore::StyleCachedImage::cachedImage):
(WebCore::StyleCachedImage::cssValue):
(WebCore::StyleCachedImage::canRender):
(WebCore::StyleCachedImage::isPending):
(WebCore::StyleCachedImage::isLoaded):
(WebCore::StyleCachedImage::errorOccurred):
(WebCore::StyleCachedImage::imageSize):
(WebCore::StyleCachedImage::imageHasRelativeWidth):
(WebCore::StyleCachedImage::imageHasRelativeHeight):
(WebCore::StyleCachedImage::computeIntrinsicDimensions):
(WebCore::StyleCachedImage::usesImageContainerSize):
(WebCore::StyleCachedImage::setContainerSizeForRenderer):
(WebCore::StyleCachedImage::addClient):
(WebCore::StyleCachedImage::removeClient):
(WebCore::StyleCachedImage::image):
(WebCore::StyleCachedImage::knownToBeOpaque):
(WebCore::StyleCachedImage::setCachedImage): Deleted.
* rendering/style/StyleCachedImage.h:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@205344 268f45cc-cd09-0410-ab3c-d52691b4dbfc

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/css/CSSCursorImageValue.cpp
Source/WebCore/css/CSSCursorImageValue.h
Source/WebCore/css/CSSImageGeneratorValue.cpp
Source/WebCore/css/CSSImageSetValue.cpp
Source/WebCore/css/CSSImageSetValue.h
Source/WebCore/css/CSSImageValue.cpp
Source/WebCore/css/CSSImageValue.h
Source/WebCore/css/StyleBuilderCustom.h
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h
Source/WebCore/page/PageSerializer.cpp
Source/WebCore/rendering/style/FillLayer.cpp
Source/WebCore/rendering/style/StyleCachedImage.cpp
Source/WebCore/rendering/style/StyleCachedImage.h
Source/WebCore/rendering/style/StyleGeneratedImage.h
Source/WebCore/rendering/style/StyleImage.h

index 4b96a2f..b856870 100644 (file)
@@ -1,3 +1,85 @@
+2016-09-02  Antti Koivisto  <antti@apple.com>
+
+        Reverse ownership relation of StyleCachedImage and CSSImageValue
+        https://bugs.webkit.org/show_bug.cgi?id=161447
+
+        Reviewed by Andreas Kling.
+
+        Currently StyleCachedImage (which represents an image in RenderStyle) has a weak ref to the
+        underlying CSSImageValue/CSSImageSetValue which actually owns it. This is awkwards especially since
+        StyleGeneratedImage, the other StyleImage subclass has reversed relationship where it refs
+        the underlying CSSImageGeneratorValue.
+
+        This patch makes StyleCachedImage similar to StyleGeneratedImage. StyleCachedImage now refs the
+        underlying CSSImageValue/CSSImageSetValue. CSSImageValues no longer need to know about StyleCachedImage.
+        Instead they reference CachedImages (memory cache objects) directly. StyleCachedImage instances are now
+        conceptually unique to RenderStyle instances. Actual resources are shared as before by sharing CachedImages.
+
+        * css/CSSCursorImageValue.cpp:
+        (WebCore::CSSCursorImageValue::loadImage):
+        (WebCore::CSSCursorImageValue::cachedImage):
+        (WebCore::CSSCursorImageValue::styleImage): Deleted.
+        * css/CSSCursorImageValue.h:
+        * css/CSSImageGeneratorValue.cpp:
+        (WebCore::CSSImageGeneratorValue::cachedImageForCSSValue):
+        * css/CSSImageSetValue.cpp:
+        (WebCore::CSSImageSetValue::~CSSImageSetValue):
+        (WebCore::CSSImageSetValue::loadBestFitImage):
+        (WebCore::CSSImageSetValue::traverseSubresources):
+        (WebCore::CSSImageSetValue::styleImage): Deleted.
+        * css/CSSImageSetValue.h:
+        * css/CSSImageValue.cpp:
+        (WebCore::CSSImageValue::CSSImageValue):
+        (WebCore::CSSImageValue::~CSSImageValue):
+        (WebCore::CSSImageValue::isPending):
+        (WebCore::CSSImageValue::loadImage):
+        (WebCore::CSSImageValue::traverseSubresources):
+        (WebCore::CSSImageValue::knownToBeOpaque):
+        (WebCore::CSSImageValue::styleImage): Deleted.
+        * css/CSSImageValue.h:
+        * css/StyleBuilderCustom.h:
+        (WebCore::StyleBuilderCustom::applyValueContent):
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::styleImage):
+        (WebCore::StyleResolver::styleCachedImageFromValue):
+        (WebCore::StyleResolver::styleGeneratedImageFromValue):
+        (WebCore::StyleResolver::cachedOrPendingFromValue): Deleted.
+        (WebCore::StyleResolver::generatedOrPendingFromValue): Deleted.
+        (WebCore::StyleResolver::setOrPendingFromValue): Deleted.
+        (WebCore::StyleResolver::cursorOrPendingFromValue): Deleted.
+        * css/StyleResolver.h:
+        * editing/TextIterator.cpp:
+        (WebCore::fullyClipsContents):
+        * page/PageSerializer.cpp:
+        (WebCore::PageSerializer::retrieveResourcesForProperties):
+        * rendering/style/FillLayer.cpp:
+        (WebCore::FillLayer::imagesIdentical):
+
+            Compare data equality instead of pointer equality for StyleImages (since StyleImages are no longer shared).
+
+        (WebCore::layerImagesIdentical): Deleted.
+        * rendering/style/StyleCachedImage.cpp:
+        (WebCore::StyleCachedImage::StyleCachedImage):
+        (WebCore::StyleCachedImage::~StyleCachedImage):
+        (WebCore::StyleCachedImage::cachedImage):
+        (WebCore::StyleCachedImage::cssValue):
+        (WebCore::StyleCachedImage::canRender):
+        (WebCore::StyleCachedImage::isPending):
+        (WebCore::StyleCachedImage::isLoaded):
+        (WebCore::StyleCachedImage::errorOccurred):
+        (WebCore::StyleCachedImage::imageSize):
+        (WebCore::StyleCachedImage::imageHasRelativeWidth):
+        (WebCore::StyleCachedImage::imageHasRelativeHeight):
+        (WebCore::StyleCachedImage::computeIntrinsicDimensions):
+        (WebCore::StyleCachedImage::usesImageContainerSize):
+        (WebCore::StyleCachedImage::setContainerSizeForRenderer):
+        (WebCore::StyleCachedImage::addClient):
+        (WebCore::StyleCachedImage::removeClient):
+        (WebCore::StyleCachedImage::image):
+        (WebCore::StyleCachedImage::knownToBeOpaque):
+        (WebCore::StyleCachedImage::setCachedImage): Deleted.
+        * rendering/style/StyleCachedImage.h:
+
 2016-09-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Only update connected custom elements
index ae4ac05..1901785 100644 (file)
@@ -26,8 +26,6 @@
 #include "CSSImageValue.h"
 #include "CachedImage.h"
 #include "CachedResourceLoader.h"
-#include "StyleCachedImage.h"
-#include "StyleImage.h"
 #include "SVGCursorElement.h"
 #include "SVGLengthContext.h"
 #include "SVGNames.h"
@@ -109,25 +107,28 @@ void CSSCursorImageValue::loadImage(CachedResourceLoader& loader, const Resource
         return;
     }
 
+    if (auto* cursorElement = updateCursorElement(*loader.document())) {
+        if (cursorElement->href() != downcast<CSSImageValue>(m_imageValue.get()).url())
+            m_imageValue = CSSImageValue::create(cursorElement->href());
+    }
+
     downcast<CSSImageValue>(m_imageValue.get()).loadImage(loader, options);
 }
 
-StyleCachedImage& CSSCursorImageValue::styleImage(const Document& document)
+CachedImage* CSSCursorImageValue::cachedImage() const
 {
-    // Need to delegate completely so that changes in device scale factor can be handled appropriately.
-    StyleCachedImage* styleImage;
     if (is<CSSImageSetValue>(m_imageValue.get()))
-        styleImage = &downcast<CSSImageSetValue>(m_imageValue.get()).styleImage(document);
-    else {
-        if (auto* cursorElement = updateCursorElement(document)) {
-            if (cursorElement->href() != downcast<CSSImageValue>(m_imageValue.get()).url())
-                m_imageValue = CSSImageValue::create(cursorElement->href());
-        }
-        styleImage = &downcast<CSSImageValue>(m_imageValue.get()).styleImage();
-    }
-    styleImage->setCSSValue(*this);
+        return downcast<CSSImageSetValue>(m_imageValue.get()).cachedImage();
+
+    return downcast<CSSImageValue>(m_imageValue.get()).cachedImage();
+}
+
+float CSSCursorImageValue::scaleFactor() const
+{
+    if (is<CSSImageSetValue>(m_imageValue.get()))
+        return downcast<CSSImageSetValue>(m_imageValue.get()).bestFitScaleFactor();
 
-    return *styleImage;
+    return 1;
 }
 
 bool CSSCursorImageValue::equals(const CSSCursorImageValue& other) const
index f8e1eda..d6fbdee 100644 (file)
@@ -31,7 +31,6 @@ class Document;
 class Element;
 class SVGCursorElement;
 class SVGElement;
-class StyleCachedImage;
 
 class CSSCursorImageValue final : public CSSValue {
 public:
@@ -54,7 +53,9 @@ public:
     String customCSSText() const;
 
     void loadImage(CachedResourceLoader&, const ResourceLoaderOptions&);
-    StyleCachedImage& styleImage(const Document&);
+    CachedImage* cachedImage() const;
+
+    float scaleFactor() const;
 
     void removeReferencedElement(SVGElement*);
 
index 3d54b6a..4186321 100644 (file)
@@ -253,7 +253,7 @@ CachedImage* CSSImageGeneratorValue::cachedImageForCSSValue(CSSValue& value, Cac
     if (is<CSSImageValue>(value)) {
         auto& imageValue = downcast<CSSImageValue>(value);
         imageValue.loadImage(cachedResourceLoader, options);
-        return imageValue.styleImage().cachedImage();
+        return imageValue.cachedImage();
     }
     
     if (is<CSSImageGeneratorValue>(value)) {
index ba4af1e..cbe85cc 100644 (file)
 #include "CrossOriginAccessControl.h"
 #include "Document.h"
 #include "Page.h"
-#include "StyleCachedImage.h"
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
 CSSImageSetValue::CSSImageSetValue()
     : CSSValueList(ImageSetClass, CommaSeparator)
-    , m_accessedBestFitImage(false)
-    , m_scaleFactor(1)
 {
 }
 
 CSSImageSetValue::~CSSImageSetValue()
 {
-    if (m_image)
-        m_image->detachFromCSSValue();
 }
 
 void CSSImageSetValue::fillImageSet()
@@ -79,11 +74,14 @@ void CSSImageSetValue::fillImageSet()
 
 CSSImageSetValue::ImageWithScale CSSImageSetValue::bestImageForScaleFactor()
 {
+    if (!m_imagesInSet.size())
+        fillImageSet();
+
     ImageWithScale image;
     size_t numberOfImages = m_imagesInSet.size();
     for (size_t i = 0; i < numberOfImages; ++i) {
         image = m_imagesInSet.at(i);
-        if (image.scaleFactor >= m_scaleFactor)
+        if (image.scaleFactor >= m_deviceScaleFactor)
             return image;
     }
     return image;
@@ -92,16 +90,11 @@ CSSImageSetValue::ImageWithScale CSSImageSetValue::bestImageForScaleFactor()
 void CSSImageSetValue::loadBestFitImage(CachedResourceLoader& loader, const ResourceLoaderOptions& options)
 {
     Document* document = loader.document();
-    if (Page* page = document->page())
-        m_scaleFactor = page->deviceScaleFactor();
-    else
-        m_scaleFactor = 1;
-
-    if (!m_imagesInSet.size())
-        fillImageSet();
-
+    updateDeviceScaleFactor(*document);
+    
     if (m_accessedBestFitImage)
         return;
+
     // FIXME: In the future, we want to take much more than deviceScaleFactor into acount here.
     // All forms of scale should be included: Page::pageScaleFactor(), Frame::pageZoomFactor(),
     // and any CSS transforms. https://bugs.webkit.org/show_bug.cgi?id=81698
@@ -112,30 +105,19 @@ void CSSImageSetValue::loadBestFitImage(CachedResourceLoader& loader, const Reso
         ASSERT(document->securityOrigin());
         updateRequestForAccessControl(request.mutableResourceRequest(), *document->securityOrigin(), options.allowCredentials);
     }
-    if (CachedResourceHandle<CachedImage> cachedImage = loader.requestImage(request)) {
-        styleImage(*document).setCachedImage(*cachedImage, image.scaleFactor);
-        m_accessedBestFitImage = true;
-    }
+    m_cachedImage = loader.requestImage(request);
+    m_bestFitImageScaleFactor = image.scaleFactor;
+    m_accessedBestFitImage = true;
 }
 
-StyleCachedImage& CSSImageSetValue::styleImage(const Document& document)
+void CSSImageSetValue::updateDeviceScaleFactor(const Document& document)
 {
-    if (!m_image)
-        m_image = StyleCachedImage::create(*this);
-    else if (!m_image->isPending()) {
-        float deviceScaleFactor = 1;
-        if (Page* page = document.page())
-            deviceScaleFactor = page->deviceScaleFactor();
-
-        // If the deviceScaleFactor has changed, we may not have the best image loaded, so we have to re-assess.
-        if (deviceScaleFactor != m_scaleFactor) {
-            m_accessedBestFitImage = false;
-            m_image->detachFromCSSValue();
-            m_image = StyleCachedImage::create(*this);
-        }
-    }
-
-    return *m_image;
+    float deviceScaleFactor = document.page() ? document.page()->deviceScaleFactor() : 1;
+    if (deviceScaleFactor == m_deviceScaleFactor)
+        return;
+    m_deviceScaleFactor = deviceScaleFactor;
+    m_accessedBestFitImage = false;
+    m_cachedImage = nullptr;
 }
 
 String CSSImageSetValue::customCSSText() const
@@ -170,18 +152,13 @@ String CSSImageSetValue::customCSSText() const
 
 bool CSSImageSetValue::traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const
 {
-    if (!m_image)
-        return false;
-    CachedImage* cachedResource = m_image->cachedImage();
-    if (!cachedResource)
+    if (!m_cachedImage)
         return false;
-    return handler(*cachedResource);
+    return handler(*m_cachedImage);
 }
 
 CSSImageSetValue::CSSImageSetValue(const CSSImageSetValue& cloneFrom)
     : CSSValueList(cloneFrom)
-    , m_accessedBestFitImage(false)
-    , m_scaleFactor(1)
 {
     // Non-CSSValueList data is not accessible through CSS OM, no need to clone.
 }
index 27e8465..d34afbb 100644 (file)
 #define CSSImageSetValue_h
 
 #include "CSSValueList.h"
+#include "CachedImageClient.h"
+#include "CachedResourceHandle.h"
 
 namespace WebCore {
 
 class CachedResourceLoader;
 class Document;
-class StyleCachedImage;
-class StyleImage;
 struct ResourceLoaderOptions;
 
 class CSSImageSetValue final : public CSSValueList {
 public:
-
     static Ref<CSSImageSetValue> create()
     {
         return adoptRef(*new CSSImageSetValue());
@@ -46,11 +45,11 @@ public:
     ~CSSImageSetValue();
 
     void loadBestFitImage(CachedResourceLoader&, const ResourceLoaderOptions&);
-    StyleCachedImage& styleImage(const Document&);
+    CachedImage* cachedImage() const { return m_cachedImage.get(); }
 
     String customCSSText() const;
 
-    bool isPending() const { return !m_accessedBestFitImage; }
+    float bestFitScaleFactor() const { return m_bestFitImageScaleFactor; };
 
     struct ImageWithScale {
         String imageURL;
@@ -61,6 +60,8 @@ public:
 
     Ref<CSSImageSetValue> cloneForCSSOM() const;
 
+    void updateDeviceScaleFactor(const Document&);
+
 protected:
     ImageWithScale bestImageForScaleFactor();
 
@@ -71,12 +72,10 @@ private:
     void fillImageSet();
     static inline bool compareByScaleFactor(ImageWithScale first, ImageWithScale second) { return first.scaleFactor < second.scaleFactor; }
 
-    RefPtr<StyleCachedImage> m_image;
-    bool m_accessedBestFitImage;
-
-    // This represents the scale factor that we used to find the best fit image. It does not necessarily
-    // correspond to the scale factor of the best fit image.
-    float m_scaleFactor;
+    CachedResourceHandle<CachedImage> m_cachedImage;
+    bool m_accessedBestFitImage { false };
+    float m_bestFitImageScaleFactor { 1 };
+    float m_deviceScaleFactor { 1 };
 
     Vector<ImageWithScale> m_imagesInSet;
 };
index c8aee0d..bc9e3b6 100644 (file)
@@ -32,7 +32,6 @@
 #include "Document.h"
 #include "Element.h"
 #include "MemoryCache.h"
-#include "StyleCachedImage.h"
 
 namespace WebCore {
 
@@ -46,30 +45,19 @@ CSSImageValue::CSSImageValue(const String& url)
 CSSImageValue::CSSImageValue(CachedImage& image)
     : CSSValue(ImageClass)
     , m_url(image.url())
-    , m_image(StyleCachedImage::create(*this))
+    , m_cachedImage(&image)
     , m_accessedImage(true)
 {
-    m_image->setCachedImage(image);
 }
 
 
 CSSImageValue::~CSSImageValue()
 {
-    if (m_image)
-        m_image->detachFromCSSValue();
 }
 
 bool CSSImageValue::isPending() const
 {
-    return !m_image || !m_image->cachedImage();
-}
-
-StyleCachedImage& CSSImageValue::styleImage()
-{
-    if (!m_image)
-        m_image = StyleCachedImage::create(*this);
-
-    return *m_image;
+    return !m_accessedImage;
 }
 
 void CSSImageValue::loadImage(CachedResourceLoader& loader, const ResourceLoaderOptions& options)
@@ -88,18 +76,14 @@ void CSSImageValue::loadImage(CachedResourceLoader& loader, const ResourceLoader
         ASSERT(loader.document()->securityOrigin());
         updateRequestForAccessControl(request.mutableResourceRequest(), *loader.document()->securityOrigin(), options.allowCredentials);
     }
-    if (CachedResourceHandle<CachedImage> cachedImage = loader.requestImage(request))
-        styleImage().setCachedImage(*cachedImage);
+    m_cachedImage = loader.requestImage(request);
 }
 
 bool CSSImageValue::traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const
 {
-    if (!m_image)
-        return false;
-    CachedResource* cachedResource = m_image->cachedImage();
-    if (!cachedResource)
+    if (!m_cachedImage)
         return false;
-    return handler(*cachedResource);
+    return handler(*m_cachedImage);
 }
 
 bool CSSImageValue::equals(const CSSImageValue& other) const
@@ -122,7 +106,9 @@ Ref<CSSValue> CSSImageValue::cloneForCSSOM() const
 
 bool CSSImageValue::knownToBeOpaque(const RenderElement* renderer) const
 {
-    return m_image ? m_image->knownToBeOpaque(renderer) : false;
+    if (!m_cachedImage)
+        return false;
+    return m_cachedImage->currentFrameKnownToBeOpaque(renderer);
 }
 
 } // namespace WebCore
index 0a8de95..a5df79c 100644 (file)
@@ -22,6 +22,7 @@
 #define CSSImageValue_h
 
 #include "CSSValue.h"
+#include "CachedResourceHandle.h"
 #include <wtf/RefPtr.h>
 
 namespace WebCore {
@@ -29,7 +30,6 @@ namespace WebCore {
 class CachedImage;
 class CachedResourceLoader;
 class Element;
-class StyleCachedImage;
 class RenderElement;
 struct ResourceLoaderOptions;
 
@@ -41,7 +41,7 @@ public:
 
     bool isPending() const;
     void loadImage(CachedResourceLoader&, const ResourceLoaderOptions&);
-    StyleCachedImage& styleImage();
+    CachedImage* cachedImage() const { return m_cachedImage.get(); }
 
     const String& url() const { return m_url; }
 
@@ -62,7 +62,7 @@ private:
     explicit CSSImageValue(CachedImage&);
 
     String m_url;
-    RefPtr<StyleCachedImage> m_image;
+    CachedResourceHandle<CachedImage> m_cachedImage;
     bool m_accessedImage;
     AtomicString m_initiatorName;
 };
index b1c5523..7a17290 100644 (file)
@@ -46,6 +46,7 @@
 #include "SVGElement.h"
 #include "SVGRenderStyle.h"
 #include "StyleBuilderConverter.h"
+#include "StyleCachedImage.h"
 #include "StyleFontSizeFunctions.h"
 #include "StyleGeneratedImage.h"
 #include "StyleResolver.h"
@@ -1313,12 +1314,12 @@ inline void StyleBuilderCustom::applyValueContent(StyleResolver& styleResolver,
                 styleResolver.style()->setContent(StyleGeneratedImage::create(downcast<CSSImageGeneratorValue>(item.get())), didSet);
             didSet = true;
         } else if (is<CSSImageSetValue>(item.get())) {
-            styleResolver.style()->setContent(styleResolver.setOrPendingFromValue(CSSPropertyContent, downcast<CSSImageSetValue>(item.get())), didSet);
+            styleResolver.style()->setContent(styleResolver.styleCachedImageFromValue(CSSPropertyContent, item), didSet);
             didSet = true;
         }
 
         if (is<CSSImageValue>(item.get())) {
-            styleResolver.style()->setContent(styleResolver.cachedOrPendingFromValue(CSSPropertyContent, downcast<CSSImageValue>(item.get())), didSet);
+            styleResolver.style()->setContent(styleResolver.styleCachedImageFromValue(CSSPropertyContent, item), didSet);
             didSet = true;
             continue;
         }
index 3594b2a..f4123bb 100644 (file)
@@ -1702,33 +1702,27 @@ RefPtr<CSSValue> StyleResolver::resolvedVariableValue(CSSPropertyID propID, cons
 
 RefPtr<StyleImage> StyleResolver::styleImage(CSSPropertyID property, CSSValue& value)
 {
-    if (is<CSSImageValue>(value))
-        return cachedOrPendingFromValue(property, downcast<CSSImageValue>(value));
-
     if (is<CSSImageGeneratorValue>(value)) {
         if (is<CSSGradientValue>(value))
-            return generatedOrPendingFromValue(property, *downcast<CSSGradientValue>(value).gradientWithStylesResolved(this));
-        return generatedOrPendingFromValue(property, downcast<CSSImageGeneratorValue>(value));
+            return styleGeneratedImageFromValue(property, *downcast<CSSGradientValue>(value).gradientWithStylesResolved(this));
+        return styleGeneratedImageFromValue(property, downcast<CSSImageGeneratorValue>(value));
     }
 
-    if (is<CSSImageSetValue>(value))
-        return setOrPendingFromValue(property, downcast<CSSImageSetValue>(value));
-
-    if (is<CSSCursorImageValue>(value))
-        return cursorOrPendingFromValue(property, downcast<CSSCursorImageValue>(value));
+    if (is<CSSImageValue>(value) || is<CSSImageSetValue>(value) || is<CSSCursorImageValue>(value))
+        return styleCachedImageFromValue(property, value);
 
     return nullptr;
 }
 
-Ref<StyleImage> StyleResolver::cachedOrPendingFromValue(CSSPropertyID property, CSSImageValue& value)
+Ref<StyleCachedImage> StyleResolver::styleCachedImageFromValue(CSSPropertyID property, CSSValue& value)
 {
-    Ref<StyleImage> image = value.styleImage();
+    auto image = StyleCachedImage::create(value);
     if (image->isPending())
         m_state.ensurePendingResources().pendingImages.set(property, &value);
     return image;
 }
 
-Ref<StyleImage> StyleResolver::generatedOrPendingFromValue(CSSPropertyID property, CSSImageGeneratorValue& value)
+Ref<StyleGeneratedImage> StyleResolver::styleGeneratedImageFromValue(CSSPropertyID property, CSSImageGeneratorValue& value)
 {
     if (is<CSSFilterImageValue>(value)) {
         // FilterImage needs to calculate FilterOperations.
@@ -1740,22 +1734,6 @@ Ref<StyleImage> StyleResolver::generatedOrPendingFromValue(CSSPropertyID propert
     return StyleGeneratedImage::create(value);
 }
 
-RefPtr<StyleImage> StyleResolver::setOrPendingFromValue(CSSPropertyID property, CSSImageSetValue& value)
-{
-    auto& image = value.styleImage(document());
-    if (image.isPending())
-        m_state.ensurePendingResources().pendingImages.set(property, &value);
-    return &image;
-}
-
-RefPtr<StyleImage> StyleResolver::cursorOrPendingFromValue(CSSPropertyID property, CSSCursorImageValue& value)
-{
-    auto& image = value.styleImage(document());
-    if (image.isPending())
-        m_state.ensurePendingResources().pendingImages.set(property, &value);
-    return &image;
-}
-
 #if ENABLE(IOS_TEXT_AUTOSIZING)
 void StyleResolver::checkForTextSizeAdjust(RenderStyle* style)
 {
index 3f01195..22e1be6 100644 (file)
@@ -72,6 +72,8 @@ class RuleData;
 class RuleSet;
 class SelectorFilter;
 class Settings;
+class StyleCachedImage;
+class StyleGeneratedImage;
 class StyleImage;
 class StyleKeyframe;
 class StylePendingImage;
@@ -458,10 +460,8 @@ public:
     const State& state() const { return m_state; }
 
     RefPtr<StyleImage> styleImage(CSSPropertyID, CSSValue&);
-    Ref<StyleImage> cachedOrPendingFromValue(CSSPropertyID, CSSImageValue&);
-    Ref<StyleImage> generatedOrPendingFromValue(CSSPropertyID, CSSImageGeneratorValue&);
-    RefPtr<StyleImage> setOrPendingFromValue(CSSPropertyID, CSSImageSetValue&);
-    RefPtr<StyleImage> cursorOrPendingFromValue(CSSPropertyID, CSSCursorImageValue&);
+    Ref<StyleCachedImage> styleCachedImageFromValue(CSSPropertyID, CSSValue&);
+    Ref<StyleGeneratedImage> styleGeneratedImageFromValue(CSSPropertyID, CSSImageGeneratorValue&);
 
     bool applyPropertyToRegularStyle() const { return m_state.applyPropertyToRegularStyle(); }
     bool applyPropertyToVisitedLinkStyle() const { return m_state.applyPropertyToVisitedLinkStyle(); }
index 279b206..5316d67 100644 (file)
@@ -331,9 +331,7 @@ void PageSerializer::retrieveResourcesForProperties(const StyleProperties* style
         if (!is<CSSImageValue>(*cssValue))
             continue;
 
-        auto& styleImage = downcast<CSSImageValue>(*cssValue).styleImage();
-
-        auto* image = styleImage.cachedImage();
+        auto* image = downcast<CSSImageValue>(*cssValue).cachedImage();
         if (!image)
             continue;
 
index a0f0abe..d85d9e2 100644 (file)
@@ -380,16 +380,10 @@ bool FillLayer::hasFixedImage() const
     return false;
 }
 
-static inline bool layerImagesIdentical(const FillLayer& layer1, const FillLayer& layer2)
-{
-    // We just care about pointer equivalency.
-    return layer1.image() == layer2.image();
-}
-
 bool FillLayer::imagesIdentical(const FillLayer* layer1, const FillLayer* layer2)
 {
     for (; layer1 && layer2; layer1 = layer1->next(), layer2 = layer2->next()) {
-        if (!layerImagesIdentical(*layer1, *layer2))
+        if (!arePointingToEqualData(layer1->image(), layer2->image()))
             return false;
     }
 
index fe62cc7..b1ba68c 100644 (file)
 #include "config.h"
 #include "StyleCachedImage.h"
 
+#include "CSSCursorImageValue.h"
 #include "CSSImageSetValue.h"
+#include "CSSImageValue.h"
 #include "CachedImage.h"
 #include "RenderElement.h"
 
 namespace WebCore {
 
 StyleCachedImage::StyleCachedImage(CSSValue& cssValue)
-    : m_cssValue(&cssValue)
+    : m_cssValue(cssValue)
 {
+    ASSERT(is<CSSImageValue>(m_cssValue) || is<CSSImageSetValue>(m_cssValue) || is<CSSCursorImageValue>(m_cssValue));
+
     m_isCachedImage = true;
 }
 
 StyleCachedImage::~StyleCachedImage()
 {
-    if (m_image)
-        m_image->removeClient(this);
 }
 
-void StyleCachedImage::setCachedImage(CachedImage& image, float scaleFactor)
+bool StyleCachedImage::operator==(const StyleImage& other) const
 {
-    ASSERT(!m_image);
-    m_image = &image;
-    m_image->addClient(this);
-    m_scaleFactor = scaleFactor;
+    if (!is<StyleCachedImage>(other))
+        return false;
+    auto& otherCached = downcast<StyleCachedImage>(other);
+    if (&otherCached == this)
+        return true;
+    if (m_cssValue.ptr() == otherCached.m_cssValue.ptr())
+        return true;
+    if (m_cachedImage && m_cachedImage == otherCached.m_cachedImage)
+        return true;
+    return false;
+}
+
+CachedImage* StyleCachedImage::cachedImage() const
+{
+    if (!m_cachedImage) {
+        if (is<CSSImageValue>(m_cssValue)) {
+            auto& imageValue = downcast<CSSImageValue>(m_cssValue.get());
+            m_cachedImage = imageValue.cachedImage();
+        } else if (is<CSSImageSetValue>(m_cssValue)) {
+            auto& imageSetValue = downcast<CSSImageSetValue>(m_cssValue.get());
+            m_cachedImage = imageSetValue.cachedImage();
+            m_scaleFactor = imageSetValue.bestFitScaleFactor();
+        } else if (is<CSSCursorImageValue>(m_cssValue.get())) {
+            auto& cursorValue = downcast<CSSCursorImageValue>(m_cssValue.get());
+            m_cachedImage = cursorValue.cachedImage();
+            m_scaleFactor = cursorValue.scaleFactor();
+        }
+    }
+    return m_cachedImage.get();
 }
 
 PassRefPtr<CSSValue> StyleCachedImage::cssValue() const
 {
-    if (m_cssValue)
-        return m_cssValue;
-    return CSSPrimitiveValue::create(m_image->url(), CSSPrimitiveValue::CSS_URI);
+    return const_cast<CSSValue*>(m_cssValue.ptr());
 }
 
 bool StyleCachedImage::canRender(const RenderObject* renderer, float multiplier) const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->canRender(renderer, multiplier);
+    return image->canRender(renderer, multiplier);
 }
 
 bool StyleCachedImage::isPending() const
 {
-    return !m_image;
+    return !m_cachedImage;
 }
 
 bool StyleCachedImage::isLoaded() const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->isLoaded();
+    return image->isLoaded();
 }
 
 bool StyleCachedImage::errorOccurred() const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->errorOccurred();
+    return image->errorOccurred();
 }
 
 FloatSize StyleCachedImage::imageSize(const RenderElement* renderer, float multiplier) const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return { };
-    FloatSize size = m_image->imageSizeForRenderer(renderer, multiplier);
+    FloatSize size = image->imageSizeForRenderer(renderer, multiplier);
     size.scale(1 / m_scaleFactor);
     return size;
 }
 
 bool StyleCachedImage::imageHasRelativeWidth() const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->imageHasRelativeWidth();
+    return image->imageHasRelativeWidth();
 }
 
 bool StyleCachedImage::imageHasRelativeHeight() const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->imageHasRelativeHeight();
+    return image->imageHasRelativeHeight();
 }
 
 void StyleCachedImage::computeIntrinsicDimensions(const RenderElement*, Length& intrinsicWidth, Length& intrinsicHeight, FloatSize& intrinsicRatio)
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return;
-    m_image->computeIntrinsicDimensions(intrinsicWidth, intrinsicHeight, intrinsicRatio);
+    image->computeIntrinsicDimensions(intrinsicWidth, intrinsicHeight, intrinsicRatio);
 }
 
 bool StyleCachedImage::usesImageContainerSize() const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->usesImageContainerSize();
+    return image->usesImageContainerSize();
 }
 
 void StyleCachedImage::setContainerSizeForRenderer(const RenderElement* renderer, const FloatSize& imageContainerSize, float imageContainerZoomFactor)
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return;
-    m_image->setContainerSizeForRenderer(renderer, LayoutSize(imageContainerSize), imageContainerZoomFactor);
+    image->setContainerSizeForRenderer(renderer, LayoutSize(imageContainerSize), imageContainerZoomFactor);
 }
 
 void StyleCachedImage::addClient(RenderElement* renderer)
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return;
-    m_image->addClient(renderer);
+    image->addClient(renderer);
 }
 
 void StyleCachedImage::removeClient(RenderElement* renderer)
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return;
-    m_image->removeClient(renderer);
+    image->removeClient(renderer);
 }
 
 RefPtr<Image> StyleCachedImage::image(RenderElement* renderer, const FloatSize&) const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return nullptr;
-    return m_image->imageForRenderer(renderer);
+    return image->imageForRenderer(renderer);
 }
 
 float StyleCachedImage::imageScaleFactor() const
@@ -155,9 +192,10 @@ float StyleCachedImage::imageScaleFactor() const
 
 bool StyleCachedImage::knownToBeOpaque(const RenderElement* renderer) const
 {
-    if (!m_image)
+    auto* image = cachedImage();
+    if (!image)
         return false;
-    return m_image->currentFrameKnownToBeOpaque(renderer);
+    return image->currentFrameKnownToBeOpaque(renderer);
 }
 
 }
index f8a5d31..77e6246 100644 (file)
@@ -32,21 +32,19 @@ namespace WebCore {
 
 class CSSValue;
 class CachedImage;
+class Document;
 
-class StyleCachedImage final : public StyleImage, private CachedImageClient {
+class StyleCachedImage final : public StyleImage {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     static Ref<StyleCachedImage> create(CSSValue& cssValue) { return adoptRef(*new StyleCachedImage(cssValue)); }
     virtual ~StyleCachedImage();
 
-    CachedImage* cachedImage() const override { return m_image.get(); }
+    bool operator==(const StyleImage& other) const override;
 
-    void detachFromCSSValue() { m_cssValue = nullptr; }
-    void setCSSValue(CSSValue& value) { m_cssValue = &value; }
+    CachedImage* cachedImage() const override;
 
-    void setCachedImage(CachedImage&, float scaleFactor = 1);
-
-    WrappedImagePtr data() const override { return m_image.get(); }
+    WrappedImagePtr data() const override { return m_cachedImage.get(); }
 
     PassRefPtr<CSSValue> cssValue() const override;
     
@@ -69,9 +67,9 @@ public:
 private:
     StyleCachedImage(CSSValue&);
 
-    CSSValue* m_cssValue;
-    float m_scaleFactor { 1 };
-    CachedResourceHandle<CachedImage> m_image;
+    Ref<CSSValue> m_cssValue;
+    mutable float m_scaleFactor { 1 };
+    mutable CachedResourceHandle<CachedImage> m_cachedImage;
 };
 
 } // namespace WebCore
index 63c5922..d49f1e5 100644 (file)
@@ -41,6 +41,8 @@ public:
     CSSImageGeneratorValue& imageValue() { return m_imageGeneratorValue; }
 
 private:
+    bool operator==(const StyleImage& other) const override { return data() == other.data(); }
+
     WrappedImagePtr data() const override { return m_imageGeneratorValue.ptr(); }
 
     PassRefPtr<CSSValue> cssValue() const override;
index aa45637..1f0d315 100644 (file)
@@ -45,10 +45,7 @@ class StyleImage : public RefCounted<StyleImage> {
 public:
     virtual ~StyleImage() { }
 
-    bool operator==(const StyleImage& other) const
-    {
-        return &other == this || (data() && data() == other.data());
-    }
+    virtual bool operator==(const StyleImage& other) const = 0;
 
     virtual PassRefPtr<CSSValue> cssValue() const = 0;