StylePendingImage needs to correctly manage the CSSValue pointer lifetime

https://bugs.webkit.org/show_bug.cgi?id=125468

Reviewed by Andreas Kling.

Source/WebCore:

Test: fast/css/pending-image-crash.xhtml

Disconnect the reference counted StylePendingImage from the CSSValue that owns
it when it's not needed any more, otherwise we could end up using a pointer
that might no longer be valid.

* css/CSSCursorImageValue.cpp:
(WebCore::CSSCursorImageValue::detachPendingImage): Added. Calls detachFromCSSValue
on the current image if it is a StylePendingImage.
(WebCore::CSSCursorImageValue::~CSSCursorImageValue): Call detachPendingImage.
(WebCore::CSSCursorImageValue::cachedImage): Call detachPendingImage before changing
m_image to a new value.
(WebCore::CSSCursorImageValue::clearCachedImage): Ditto.
* css/CSSCursorImageValue.h: Added detachPendingImage.

* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::detachPendingImage): Added. Calls detachFromCSSValue
on the current image set if it is a StylePendingImage.
(WebCore::CSSImageSetValue::~CSSImageSetValue): Call detachPendingImage.
(WebCore::CSSImageSetValue::cachedImageSet): Call detachPendingImage before changing
m_imageSet to a new value.
* css/CSSImageSetValue.h: Added detachPendingImage.

* css/CSSImageValue.cpp:
(WebCore::CSSImageValue::detachPendingImage): Added. Calls detachFromCSSValue on the
current image if it is a StylePendingImage.
(WebCore::CSSImageValue::~CSSImageValue): Call detachPendingImage.
(WebCore::CSSImageValue::cachedImage): Call detachPendingImage before changing m_image
to a new value.
* css/CSSImageValue.h: Added detachPendingImage.

* rendering/style/StylePendingImage.h:
(WebCore::StylePendingImage::cssImageValue): Added a null check.
(WebCore::StylePendingImage::cssImageGeneratorValue): Added a null check.
(WebCore::StylePendingImage::cssCursorImageValue): Added a null check.
(WebCore::StylePendingImage::cssImageSetValue): Added a null check.
(WebCore::StylePendingImage::detachFromCSSValue): Added. Sets m_value to null since
the style is no longer using this StylePendingImage.
(WebCore::StylePendingImage::data): Changed to use the "this" pointer since all we
need is some arbitrary pointer uniquely identifying the image. Before loading the image,
we have no suitable weak identifier, so it suffices to use the unique pointer to each
StylePendingImage object. This function is used only in a limited way; it would be nice
to find a way to make the code less strange long term.

Change-Id: Iab8a5d033c5cdf9a488ac18d8f233dee48c5f3e7
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@160479 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Reviewed-by: Michael Bruning <michael.bruning@digia.com>

7 files changed, 48 insertions, 8 deletions

diff --git a/Source/WebCore/css/CSSCursorImageValue.cpp b/Source/WebCore/css/CSSCursorImageValue.cpp
index b7c747e8a..f37273ccd 100644
--- a/Source/WebCore/css/CSSCursorImageValue.cpp
+++ b/Source/WebCore/css/CSSCursorImageValue.cpp
@@ -67,8 +67,16 @@ CSSCursorImageValue::CSSCursorImageValue(PassRefPtr<CSSValue> imageValue, bool h
 {
 }
 
+inline void CSSCursorImageValue::detachPendingImage()
+{
+    if (m_image && m_image->isPendingImage())
+        static_cast<StylePendingImage&>(*m_image).detachFromCSSValue();
+}
+
 CSSCursorImageValue::~CSSCursorImageValue()
 {
+    detachPendingImage();
+
 #if ENABLE(SVG)
     if (!isSVGCursor())
         return;
@@ -153,6 +161,7 @@ StyleImage* CSSCursorImageValue::cachedImage(CachedResourceLoader* loader)
             RefPtr<CSSImageValue> imageValue = static_cast<CSSImageValue*>(m_imageValue.get());
             // FIXME: This will fail if the <cursor> element is in a shadow DOM (bug 59827)
             if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(imageValue->url(), loader->document())) {
+                detachPendingImage();
                 RefPtr<CSSImageValue> svgImageValue = CSSImageValue::create(cursorElement->href());
                 StyleCachedImage* cachedImage = svgImageValue->cachedImage(loader);
                 m_image = cachedImage;
@@ -161,8 +170,10 @@ StyleImage* CSSCursorImageValue::cachedImage(CachedResourceLoader* loader)
         }
 #endif
 
-        if (m_imageValue->isImageValue())
+        if (m_imageValue->isImageValue()) {
+            detachPendingImage();
             m_image = static_cast<CSSImageValue*>(m_imageValue.get())->cachedImage(loader);
+        }
     }
 
     if (m_image && m_image->isCachedImage())
@@ -205,7 +216,8 @@ String CSSCursorImageValue::cachedImageURL()
 
 void CSSCursorImageValue::clearCachedImage()
 {
-    m_image = 0;
+    detachPendingImage();
+    m_image.clear();
     m_accessedImage = false;
 }
 
diff --git a/Source/WebCore/css/CSSCursorImageValue.h b/Source/WebCore/css/CSSCursorImageValue.h
index 89391c33d..8e1ae35d1 100644
--- a/Source/WebCore/css/CSSCursorImageValue.h
+++ b/Source/WebCore/css/CSSCursorImageValue.h
@@ -64,6 +64,8 @@ public:
 private:
     CSSCursorImageValue(PassRefPtr<CSSValue> imageValue, bool hasHotSpot, const IntPoint& hotSpot);
 
+    void detachPendingImage();
+
 #if ENABLE(SVG)
     bool isSVGCursor() const;
     String cachedImageURL();
diff --git a/Source/WebCore/css/CSSImageSetValue.cpp b/Source/WebCore/css/CSSImageSetValue.cpp
index cbd8a4333..96cacf163 100644
--- a/Source/WebCore/css/CSSImageSetValue.cpp
+++ b/Source/WebCore/css/CSSImageSetValue.cpp
@@ -49,8 +49,16 @@ CSSImageSetValue::CSSImageSetValue()
 {
 }
 
+inline void CSSImageSetValue::detachPendingImage()
+{
+    if (m_imageSet && m_imageSet->isPendingImage())
+        static_cast<StylePendingImage&>(*m_imageSet).detachFromCSSValue();
+}
+
 CSSImageSetValue::~CSSImageSetValue()
 {
+    detachPendingImage();
+
     if (m_imageSet && m_imageSet->isCachedImageSet())
         static_cast<StyleCachedImageSet*>(m_imageSet.get())->clearImageSetValue();
 }
@@ -114,6 +122,7 @@ StyleCachedImageSet* CSSImageSetValue::cachedImageSet(CachedResourceLoader* load
         CachedResourceRequest request(ResourceRequest(document->completeURL(image.imageURL)));
         request.setInitiator(cachedResourceRequestInitiators().css);
         if (CachedResourceHandle<CachedImage> cachedImage = loader->requestImage(request)) {
+            detachPendingImage();
             m_imageSet = StyleCachedImageSet::create(cachedImage.get(), image.scaleFactor, this);
             m_accessedBestFitImage = true;
         }
diff --git a/Source/WebCore/css/CSSImageSetValue.h b/Source/WebCore/css/CSSImageSetValue.h
index 95270a67d..bb950734c 100644
--- a/Source/WebCore/css/CSSImageSetValue.h
+++ b/Source/WebCore/css/CSSImageSetValue.h
@@ -71,6 +71,7 @@ private:
     CSSImageSetValue();
     CSSImageSetValue(const CSSImageSetValue& cloneFrom);
 
+    void detachPendingImage();
     void fillImageSet();
     static inline bool compareByScaleFactor(ImageWithScale first, ImageWithScale second) { return first.scaleFactor < second.scaleFactor; }
 
diff --git a/Source/WebCore/css/CSSImageValue.cpp b/Source/WebCore/css/CSSImageValue.cpp
index a040ac3b5..bdb8d9e5a 100644
--- a/Source/WebCore/css/CSSImageValue.cpp
+++ b/Source/WebCore/css/CSSImageValue.cpp
@@ -51,8 +51,15 @@ CSSImageValue::CSSImageValue(const String& url, StyleImage* image)
 {
 }
 
+inline void CSSImageValue::detachPendingImage()
+{
+    if (m_image && m_image->isPendingImage())
+        static_cast<StylePendingImage&>(*m_image).detachFromCSSValue();
+}
+
 CSSImageValue::~CSSImageValue()
 {
+    detachPendingImage();
 }
 
 StyleImage* CSSImageValue::cachedOrPendingImage()
@@ -75,8 +82,10 @@ StyleCachedImage* CSSImageValue::cachedImage(CachedResourceLoader* loader, const
             request.setInitiator(cachedResourceRequestInitiators().css);
         else
             request.setInitiator(m_initiatorName);
-        if (CachedResourceHandle<CachedImage> cachedImage = loader->requestImage(request))
+        if (CachedResourceHandle<CachedImage> cachedImage = loader->requestImage(request)) {
+            detachPendingImage();
             m_image = StyleCachedImage::create(cachedImage.get());
+        }
     }
 
     return (m_image && m_image->isCachedImage()) ? static_cast<StyleCachedImage*>(m_image.get()) : 0;
diff --git a/Source/WebCore/css/CSSImageValue.h b/Source/WebCore/css/CSSImageValue.h
index 8b73cee0c..66b596e4e 100644
--- a/Source/WebCore/css/CSSImageValue.h
+++ b/Source/WebCore/css/CSSImageValue.h
@@ -61,6 +61,7 @@ public:
 private:
     explicit CSSImageValue(const String& url);
     CSSImageValue(const String& url, StyleImage*);
+    void detachPendingImage();
 
     String m_url;
     RefPtr<StyleImage> m_image;
diff --git a/Source/WebCore/rendering/style/StylePendingImage.h b/Source/WebCore/rendering/style/StylePendingImage.h
index b689ee779..96e825c17 100644
--- a/Source/WebCore/rendering/style/StylePendingImage.h
+++ b/Source/WebCore/rendering/style/StylePendingImage.h
@@ -28,6 +28,9 @@
 
 #include "CSSCursorImageValue.h"
 #include "CSSImageGeneratorValue.h"
+#include "CSSImageValue.h"
+#include "StyleImage.h"
+
 #if ENABLE(CSS_IMAGE_SET)
 #include "CSSImageSetValue.h"
 #endif
@@ -48,13 +51,15 @@ public:
     virtual WrappedImagePtr data() const { return static_cast<CSSImageValue*>(m_value); }
 
     virtual PassRefPtr<CSSValue> cssValue() const { return m_value; }
-    CSSImageValue* cssImageValue() const { return m_value->isImageValue() ? static_cast<CSSImageValue*>(m_value) : 0; }
-    CSSImageGeneratorValue* cssImageGeneratorValue() const { return m_value->isImageGeneratorValue() ? static_cast<CSSImageGeneratorValue*>(m_value) : 0; }
-    CSSCursorImageValue* cssCursorImageValue() const { return m_value->isCursorImageValue() ? static_cast<CSSCursorImageValue*>(m_value) : 0; }
+    CSSImageValue* cssImageValue() const { return m_value && m_value->isImageValue() ? static_cast<CSSImageValue*>(m_value) : 0; }
+    CSSImageGeneratorValue* cssImageGeneratorValue() const { return m_value && m_value->isImageGeneratorValue() ? static_cast<CSSImageGeneratorValue*>(m_value) : 0; }
+    CSSCursorImageValue* cssCursorImageValue() const { return m_value && m_value->isCursorImageValue() ? static_cast<CSSCursorImageValue*>(m_value) : 0; }
 #if ENABLE(CSS_IMAGE_SET)
-    CSSImageSetValue* cssImageSetValue() const { return m_value->isImageSetValue() ? static_cast<CSSImageSetValue*>(m_value) : 0; }
+    CSSImageSetValue* cssImageSetValue() const { return m_value && m_value->isImageSetValue() ? static_cast<CSSImageSetValue*>(m_value) : 0; }
 #endif
-    
+
+    void detachFromCSSValue() { m_value = 0; }
+
     virtual LayoutSize imageSize(const RenderObject*, float /*multiplier*/) const OVERRIDE { return LayoutSize(); }
     virtual bool imageHasRelativeWidth() const { return false; }
     virtual bool imageHasRelativeHeight() const { return false; }
@@ -81,4 +86,5 @@ private:
 };
 
 }
+
 #endif