- Revision
- 160479
- Author
- [email protected]
- Date
- 2013-12-12 00:58:50 -0800 (Thu, 12 Dec 2013)
Log Message
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.
LayoutTests:
* fast/css/pending-image-crash-expected.txt: Added.
* fast/css/pending-image-crash.xhtml: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (160478 => 160479)
--- trunk/LayoutTests/ChangeLog 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/LayoutTests/ChangeLog 2013-12-12 08:58:50 UTC (rev 160479)
@@ -1,3 +1,13 @@
+2013-12-11 Darin Adler <[email protected]>
+
+ StylePendingImage needs to correctly manage the CSSValue pointer lifetime
+ https://bugs.webkit.org/show_bug.cgi?id=125468
+
+ Reviewed by Andreas Kling.
+
+ * fast/css/pending-image-crash-expected.txt: Added.
+ * fast/css/pending-image-crash.xhtml: Added.
+
2013-12-11 Alexey Proskuryakov <[email protected]>
WebCrypto keys should support structured clone
Added: trunk/LayoutTests/fast/css/pending-image-crash-expected.txt (0 => 160479)
--- trunk/LayoutTests/fast/css/pending-image-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/pending-image-crash-expected.txt 2013-12-12 08:58:50 UTC (rev 160479)
@@ -0,0 +1,5 @@
+PASS test did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Property changes on: trunk/LayoutTests/fast/css/pending-image-crash-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/css/pending-image-crash.xhtml (0 => 160479)
--- trunk/LayoutTests/fast/css/pending-image-crash.xhtml (rev 0)
+++ trunk/LayoutTests/fast/css/pending-image-crash.xhtml 2013-12-12 08:58:50 UTC (rev 160479)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head id="head">
+<script src=""
+<script>
+window.jsTestIsAsync = true;
+var count = 0;
+for (i = 0; i != 50; i++) {
+ setTimeout(function() {
+ var head = document.getElementsByTagName("head")[0];
+ var style = document.createElement("style");
+ style.innerHTML=":first-of-type {-webkit-border-image:-webkit-cross-fade(url(#head), url(#head), 100%);}";
+ head.appendChild(style);
+ count++;
+ if (count == 50) {
+ testPassed("test did not crash");
+ finishJSTest();
+ }
+ }, 36);
+}
+</script>
+<script src=""
+</head>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (160478 => 160479)
--- trunk/Source/WebCore/ChangeLog 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/ChangeLog 2013-12-12 08:58:50 UTC (rev 160479)
@@ -1,5 +1,56 @@
2013-12-11 Darin Adler <[email protected]>
+ StylePendingImage needs to correctly manage the CSSValue pointer lifetime
+ https://bugs.webkit.org/show_bug.cgi?id=125468
+
+ Reviewed by Andreas Kling.
+
+ 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.
+
+2013-12-11 Darin Adler <[email protected]>
+
Remove some unneeded code noticed while looking at StylePendingImage
https://bugs.webkit.org/show_bug.cgi?id=125618
Modified: trunk/Source/WebCore/css/CSSCursorImageValue.cpp (160478 => 160479)
--- trunk/Source/WebCore/css/CSSCursorImageValue.cpp 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSCursorImageValue.cpp 2013-12-12 08:58:50 UTC (rev 160479)
@@ -67,8 +67,16 @@
{
}
+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;
@@ -150,6 +158,7 @@
if (isSVGCursor() && loader && loader->document()) {
// FIXME: This will fail if the <cursor> element is in a shadow DOM (bug 59827)
if (SVGCursorElement* cursorElement = resourceReferencedByCursorElement(toCSSImageValue(m_imageValue.get()).url(), *loader->document())) {
+ detachPendingImage();
Ref<CSSImageValue> svgImageValue(CSSImageValue::create(cursorElement->href()));
StyleCachedImage* cachedImage = svgImageValue->cachedImage(loader);
m_image = cachedImage;
@@ -158,8 +167,10 @@
}
#endif
- if (m_imageValue.get().isImageValue())
+ if (m_imageValue.get().isImageValue()) {
+ detachPendingImage();
m_image = toCSSImageValue(m_imageValue.get()).cachedImage(loader);
+ }
}
if (m_image && m_image->isCachedImage())
@@ -203,7 +214,8 @@
void CSSCursorImageValue::clearCachedImage()
{
- m_image = 0;
+ detachPendingImage();
+ m_image = nullptr;
m_accessedImage = false;
}
Modified: trunk/Source/WebCore/css/CSSCursorImageValue.h (160478 => 160479)
--- trunk/Source/WebCore/css/CSSCursorImageValue.h 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSCursorImageValue.h 2013-12-12 08:58:50 UTC (rev 160479)
@@ -64,6 +64,8 @@
private:
CSSCursorImageValue(PassRef<CSSValue> imageValue, bool hasHotSpot, const IntPoint& hotSpot);
+ void detachPendingImage();
+
#if ENABLE(SVG)
bool isSVGCursor() const;
String cachedImageURL();
Modified: trunk/Source/WebCore/css/CSSImageSetValue.cpp (160478 => 160479)
--- trunk/Source/WebCore/css/CSSImageSetValue.cpp 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSImageSetValue.cpp 2013-12-12 08:58:50 UTC (rev 160479)
@@ -49,8 +49,16 @@
{
}
+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();
}
@@ -113,6 +121,7 @@
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;
}
Modified: trunk/Source/WebCore/css/CSSImageSetValue.h (160478 => 160479)
--- trunk/Source/WebCore/css/CSSImageSetValue.h 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSImageSetValue.h 2013-12-12 08:58:50 UTC (rev 160479)
@@ -71,6 +71,7 @@
CSSImageSetValue();
CSSImageSetValue(const CSSImageSetValue& cloneFrom);
+ void detachPendingImage();
void fillImageSet();
static inline bool compareByScaleFactor(ImageWithScale first, ImageWithScale second) { return first.scaleFactor < second.scaleFactor; }
Modified: trunk/Source/WebCore/css/CSSImageValue.cpp (160478 => 160479)
--- trunk/Source/WebCore/css/CSSImageValue.cpp 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSImageValue.cpp 2013-12-12 08:58:50 UTC (rev 160479)
@@ -52,8 +52,15 @@
{
}
+inline void CSSImageValue::detachPendingImage()
+{
+ if (m_image && m_image->isPendingImage())
+ static_cast<StylePendingImage&>(*m_image).detachFromCSSValue();
+}
+
CSSImageValue::~CSSImageValue()
{
+ detachPendingImage();
}
StyleImage* CSSImageValue::cachedOrPendingImage()
@@ -80,8 +87,10 @@
if (options.requestOriginPolicy == PotentiallyCrossOriginEnabled)
updateRequestForAccessControl(request.mutableResourceRequest(), loader->document()->securityOrigin(), options.allowCredentials);
- 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;
Modified: trunk/Source/WebCore/css/CSSImageValue.h (160478 => 160479)
--- trunk/Source/WebCore/css/CSSImageValue.h 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/css/CSSImageValue.h 2013-12-12 08:58:50 UTC (rev 160479)
@@ -61,6 +61,7 @@
private:
explicit CSSImageValue(const String& url);
CSSImageValue(const String& url, StyleImage*);
+ void detachPendingImage();
String m_url;
RefPtr<StyleImage> m_image;
Modified: trunk/Source/WebCore/rendering/style/StylePendingImage.h (160478 => 160479)
--- trunk/Source/WebCore/rendering/style/StylePendingImage.h 2013-12-12 08:44:32 UTC (rev 160478)
+++ trunk/Source/WebCore/rendering/style/StylePendingImage.h 2013-12-12 08:58:50 UTC (rev 160479)
@@ -29,7 +29,6 @@
#include "CSSCursorImageValue.h"
#include "CSSImageGeneratorValue.h"
#include "CSSImageValue.h"
-#include "Image.h"
#include "StyleImage.h"
#if ENABLE(CSS_IMAGE_SET)
@@ -46,15 +45,18 @@
public:
static PassRefPtr<StylePendingImage> create(CSSValue* value) { return adoptRef(new StylePendingImage(value)); }
- CSSImageValue* cssImageValue() const { return m_value->isImageValue() ? toCSSImageValue(m_value) : nullptr; }
- CSSImageGeneratorValue* cssImageGeneratorValue() const { return m_value->isImageGeneratorValue() ? static_cast<CSSImageGeneratorValue*>(m_value) : nullptr; }
- CSSCursorImageValue* cssCursorImageValue() const { return m_value->isCursorImageValue() ? toCSSCursorImageValue(m_value) : nullptr; }
+ CSSImageValue* cssImageValue() const { return m_value && m_value->isImageValue() ? toCSSImageValue(m_value) : nullptr; }
+ CSSImageGeneratorValue* cssImageGeneratorValue() const { return m_value && m_value->isImageGeneratorValue() ? static_cast<CSSImageGeneratorValue*>(m_value) : nullptr; }
+ CSSCursorImageValue* cssCursorImageValue() const { return m_value && m_value->isCursorImageValue() ? toCSSCursorImageValue(m_value) : nullptr; }
+
#if ENABLE(CSS_IMAGE_SET)
- CSSImageSetValue* cssImageSetValue() const { return m_value->isImageSetValue() ? toCSSImageSetValue(m_value) : nullptr; }
+ CSSImageSetValue* cssImageSetValue() const { return m_value && m_value->isImageSetValue() ? toCSSImageSetValue(m_value) : nullptr; }
#endif
+ void detachFromCSSValue() { m_value = nullptr; }
+
private:
- virtual WrappedImagePtr data() const OVERRIDE { return toCSSImageValue(m_value); }
+ virtual WrappedImagePtr data() const OVERRIDE { return const_cast<StylePendingImage*>(this); }
virtual PassRefPtr<CSSValue> cssValue() const OVERRIDE { return m_value; }
@@ -85,4 +87,5 @@
};
}
+
#endif
