Crash in WebCore::GraphicsContext::paintingDisabled
authorschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Mar 2012 15:14:57 +0000 (15:14 +0000)
committerschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Mar 2012 15:14:57 +0000 (15:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=80669

Reviewed by Nikolas Zimmermann.

Source/WebCore:

The SVGImageBufferTools::clipToImageBuffer method deletes the clip
image when it thinks it is not needed. However, there are cases when
it is in fact still needed, particularly when the clip buffer is
coming from higher up in the stack where it may be needed again.

So this patch adds a flag to only allow deletion of the image buffer
if it was created at the most recent call site.

Tests: svg/custom/circular-clip-path-references-crash-expected.svg
       svg/custom/circular-clip-path-references-crash.svg

* rendering/svg/RenderSVGResourceClipper.cpp:
(WebCore::RenderSVGResourceClipper::applyClippingToContext):
* rendering/svg/RenderSVGResourceGradient.cpp:
(WebCore::clipToTextMask):
* rendering/svg/RenderSVGResourceMasker.cpp:
(WebCore::RenderSVGResourceMasker::applyResource):
* rendering/svg/SVGImageBufferTools.cpp:
(WebCore::SVGImageBufferTools::clipToImageBuffer):
* rendering/svg/SVGImageBufferTools.h:
(SVGImageBufferTools):

LayoutTests:

* svg/custom/circular-clip-path-references-crash-expected.svg: Added.
* svg/custom/circular-clip-path-references-crash.svg: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/circular-clip-path-references-crash-expected.svg [new file with mode: 0644]
LayoutTests/svg/custom/circular-clip-path-references-crash.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp
Source/WebCore/rendering/svg/RenderSVGResourceGradient.cpp
Source/WebCore/rendering/svg/RenderSVGResourceMasker.cpp
Source/WebCore/rendering/svg/SVGImageBufferTools.cpp
Source/WebCore/rendering/svg/SVGImageBufferTools.h

index 804a0cd..0fca9f7 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-13  Stephen Chenney  <schenney@chromium.org>
+
+        Crash in WebCore::GraphicsContext::paintingDisabled
+        https://bugs.webkit.org/show_bug.cgi?id=80669
+
+        Reviewed by Nikolas Zimmermann.
+
+        * svg/custom/circular-clip-path-references-crash-expected.svg: Added.
+        * svg/custom/circular-clip-path-references-crash.svg: Added.
+
 2012-03-13  Ádám Kallai  <kadam@inf.u-szeged.hu>
 
         [Qt] Skip some tests until fix.
diff --git a/LayoutTests/svg/custom/circular-clip-path-references-crash-expected.svg b/LayoutTests/svg/custom/circular-clip-path-references-crash-expected.svg
new file mode 100644 (file)
index 0000000..a2afabd
--- /dev/null
@@ -0,0 +1,5 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+
+<text x="10" y="75">This test passes if it does not crash.</text>
+
+</svg>
diff --git a/LayoutTests/svg/custom/circular-clip-path-references-crash.svg b/LayoutTests/svg/custom/circular-clip-path-references-crash.svg
new file mode 100644 (file)
index 0000000..0432bed
--- /dev/null
@@ -0,0 +1,25 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+<defs>
+    <clipPath id="clip0">
+        <rect width="1" height="1" clip-path="url(#clip)" />
+
+    </clipPath>
+
+    <clipPath id="clip2">
+        <rect width="100" height="100" clip-path="url(#clip0)"/>
+    </clipPath>
+
+    <clipPath id="clip">
+        <rect width="1" height="1" clip-path="url(#clip2)"/>
+    </clipPath>
+
+    <mask id="mask1" x="0" y="0" width="1" height="1" maskContentUnits="objectBoundingBox">
+        <rect width="1" height="1" clip-path="url(#clip)" />
+    </mask>
+</defs>
+
+<text x="10" y="75">This test passes if it does not crash.</text>
+
+<circle r="50" mask="url(#mask1)"/>
+
+</svg>
index 105e81d..3e68306 100644 (file)
@@ -1,3 +1,32 @@
+2012-03-13  Stephen Chenney  <schenney@chromium.org>
+
+        Crash in WebCore::GraphicsContext::paintingDisabled
+        https://bugs.webkit.org/show_bug.cgi?id=80669
+
+        Reviewed by Nikolas Zimmermann.
+
+        The SVGImageBufferTools::clipToImageBuffer method deletes the clip
+        image when it thinks it is not needed. However, there are cases when
+        it is in fact still needed, particularly when the clip buffer is
+        coming from higher up in the stack where it may be needed again.
+
+        So this patch adds a flag to only allow deletion of the image buffer
+        if it was created at the most recent call site.
+
+        Tests: svg/custom/circular-clip-path-references-crash-expected.svg
+               svg/custom/circular-clip-path-references-crash.svg
+
+        * rendering/svg/RenderSVGResourceClipper.cpp:
+        (WebCore::RenderSVGResourceClipper::applyClippingToContext):
+        * rendering/svg/RenderSVGResourceGradient.cpp:
+        (WebCore::clipToTextMask):
+        * rendering/svg/RenderSVGResourceMasker.cpp:
+        (WebCore::RenderSVGResourceMasker::applyResource):
+        * rendering/svg/SVGImageBufferTools.cpp:
+        (WebCore::SVGImageBufferTools::clipToImageBuffer):
+        * rendering/svg/SVGImageBufferTools.h:
+        (SVGImageBufferTools):
+
 2012-03-13  Gavin Peters  <gavinp@chromium.org>
 
         Fix an enumeration name in ReasonsFrameCannotBeInPageCache.
index ba20502..5742061 100644 (file)
@@ -155,7 +155,8 @@ bool RenderSVGResourceClipper::pathOnlyClipping(GraphicsContext* context, const
 bool RenderSVGResourceClipper::applyClippingToContext(RenderObject* object, const FloatRect& objectBoundingBox,
                                                       const FloatRect& repaintRect, GraphicsContext* context)
 {
-    if (!m_clipper.contains(object))
+    bool missingClipperData = !m_clipper.contains(object);
+    if (missingClipperData)
         m_clipper.set(object, new ClipperData);
 
     bool shouldCreateClipData = false;
@@ -201,7 +202,7 @@ bool RenderSVGResourceClipper::applyClippingToContext(RenderObject* object, cons
     if (!clipperData->clipMaskImage)
         return false;
 
-    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage);
+    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, repaintRect, clipperData->clipMaskImage, missingClipperData);
     return true;
 }
 
index ead5510..1a1d5a0 100644 (file)
@@ -98,7 +98,7 @@ static inline AffineTransform clipToTextMask(GraphicsContext* context,
     SVGImageBufferTools::calculateTransformationToOutermostSVGCoordinateSystem(textRootBlock, absoluteTransform);
 
     targetRect = textRootBlock->repaintRectInLocalCoordinates();
-    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, targetRect, imageBuffer);
+    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, targetRect, imageBuffer, false);
 
     AffineTransform matrix;
     if (boundingBoxMode) {
index bf59549..f7a35e3 100644 (file)
@@ -86,7 +86,8 @@ bool RenderSVGResourceMasker::applyResource(RenderObject* object, RenderStyle*,
     ASSERT(context);
     ASSERT_UNUSED(resourceMode, resourceMode == ApplyToDefaultMode);
 
-    if (!m_masker.contains(object))
+    bool missingMaskerData = !m_masker.contains(object);
+    if (missingMaskerData)
         m_masker.set(object, new MaskerData);
 
     MaskerData* maskerData = m_masker.get(object);
@@ -116,7 +117,7 @@ bool RenderSVGResourceMasker::applyResource(RenderObject* object, RenderStyle*,
     if (!maskerData->maskImage)
         return false;
 
-    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, repaintRect, maskerData->maskImage);
+    SVGImageBufferTools::clipToImageBuffer(context, absoluteTransform, repaintRect, maskerData->maskImage, missingMaskerData);
     return true;
 }
 
index 8995500..1294196 100644 (file)
@@ -121,7 +121,7 @@ void SVGImageBufferTools::renderSubtreeToImageBuffer(ImageBuffer* image, RenderO
     contentTransformation = savedContentTransformation;
 }
 
-void SVGImageBufferTools::clipToImageBuffer(GraphicsContext* context, const AffineTransform& absoluteTransform, const FloatRect& targetRect, OwnPtr<ImageBuffer>& imageBuffer)
+void SVGImageBufferTools::clipToImageBuffer(GraphicsContext* context, const AffineTransform& absoluteTransform, const FloatRect& targetRect, OwnPtr<ImageBuffer>& imageBuffer, bool safeToClear)
 {
     ASSERT(context);
     ASSERT(imageBuffer);
@@ -136,7 +136,7 @@ void SVGImageBufferTools::clipToImageBuffer(GraphicsContext* context, const Affi
 
     // When nesting resources, with objectBoundingBox as content unit types, there's no use in caching the
     // resulting image buffer as the parent resource already caches the result.
-    if (!currentContentTransformation().isIdentity())
+    if (safeToClear && !currentContentTransformation().isIdentity())
         imageBuffer.clear();
 }
 
index dce80cc..b71b10d 100644 (file)
@@ -42,7 +42,7 @@ public:
     static bool createImageBufferForPattern(const FloatRect& absoluteTargetRect, const FloatRect& clampedAbsoluteTargetRect, OwnPtr<ImageBuffer>&, ColorSpace, RenderingMode);
 
     static void renderSubtreeToImageBuffer(ImageBuffer*, RenderObject*, const AffineTransform&);
-    static void clipToImageBuffer(GraphicsContext*, const AffineTransform& absoluteTransform, const FloatRect& targetRect, OwnPtr<ImageBuffer>&);
+    static void clipToImageBuffer(GraphicsContext*, const AffineTransform& absoluteTransform, const FloatRect& targetRect, OwnPtr<ImageBuffer>&, bool safeToClear);
 
     static void calculateTransformationToOutermostSVGCoordinateSystem(const RenderObject*, AffineTransform& absoluteTransform);
     static IntSize clampedAbsoluteSize(const IntSize&);