Title: [148150] branches/safari-536.30-branch

Diff

Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (148149 => 148150)


--- branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-04-10 23:25:53 UTC (rev 148149)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-04-10 23:34:10 UTC (rev 148150)
@@ -1,3 +1,17 @@
+2013-04-10  Lucas Forschler  <[email protected]>
+
+        Merge r136250
+
+    2012-11-30  Florin Malita  <[email protected]>
+
+            SVG pattern data deleted while in use
+            https://bugs.webkit.org/show_bug.cgi?id=103415
+
+            Reviewed by Dirk Schulze.
+
+            * svg/custom/large-image-pattern-crash-expected.txt: Added.
+            * svg/custom/large-image-pattern-crash.html: Added.
+
 2013-03-25  Eric Carlson  <[email protected]>
 
         <rdar://problem/13465764> Many merged tests are failing

Added: branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash-expected.txt (0 => 148150)


--- branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash-expected.txt	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash-expected.txt	2013-04-10 23:34:10 UTC (rev 148150)
@@ -0,0 +1,2 @@
+PASS: did not crash.
+

Added: branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash.html (0 => 148150)


--- branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash.html	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/custom/large-image-pattern-crash.html	2013-04-10 23:34:10 UTC (rev 148150)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <svg xmlns="http://www.w3.org/2000/svg">
+      <pattern id="pattern" width="100" height="100">
+        <image height="40000" width="40000" xlink:href=""
+	    <svg xmlns='http://www.w3.org/2000/svg'></svg>
+        "/>
+      </pattern>
+      <rect fill="url(#pattern)" height="300" width="400"/>
+      <text>PASS: did not crash.</text>
+
+      <script>
+        if (window.testRunner)
+          testRunner.dumpAsText();
+      </script>
+    </svg>
+  </body>
+</html>

Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (148149 => 148150)


--- branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-04-10 23:25:53 UTC (rev 148149)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-04-10 23:34:10 UTC (rev 148150)
@@ -1,3 +1,27 @@
+2013-04-10  Lucas Forschler  <[email protected]>
+
+        Merge r136250
+
+    2012-11-30  Florin Malita  <[email protected]>
+
+            SVG pattern data deleted while in use
+            https://bugs.webkit.org/show_bug.cgi?id=103415
+
+            Reviewed by Dirk Schulze.
+
+            Various calls in RenderSVGResourcePattern::applyResource() can trigger invalidations,
+            which may end up deleting our current pattern data (via removeAllClientsFromCache).
+            To avoid this, we should add the pattern data to the cache only after it is fully built.
+            For clarity, the patch also refactors the pattern setup code into a separate method.
+
+            Test: svg/custom/large-image-pattern-crash.html
+
+            * rendering/svg/RenderSVGResourcePattern.cpp:
+            (WebCore::RenderSVGResourcePattern::buildPattern):
+            (WebCore::RenderSVGResourcePattern::applyResource):
+            * rendering/svg/RenderSVGResourcePattern.h:
+            (RenderSVGResourcePattern):
+
 2013-03-15  Lucas Forschler  <[email protected]>
 
         Merge r136062

Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp (148149 => 148150)


--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp	2013-04-10 23:25:53 UTC (rev 148149)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp	2013-04-10 23:34:10 UTC (rev 148150)
@@ -54,20 +54,15 @@
     markClientForInvalidation(client, markForInvalidation ? RepaintInvalidation : ParentOnlyInvalidation);
 }
 
-bool RenderSVGResourcePattern::applyResource(RenderObject* object, RenderStyle* style, GraphicsContext*& context, unsigned short resourceMode)
+PatternData* RenderSVGResourcePattern::buildPattern(RenderObject* object, unsigned short resourceMode)
 {
-    ASSERT(object);
-    ASSERT(style);
-    ASSERT(context);
-    ASSERT(resourceMode != ApplyToDefaultMode);
+    PatternData* currentData = m_patternMap.get(object);
+    if (currentData && currentData->pattern)
+        return currentData;
 
-    // Be sure to synchronize all SVG properties on the patternElement _before_ processing any further.
-    // Otherwhise the call to collectPatternAttributes() below, may cause the SVG DOM property
-    // synchronization to kick in, which causes removeAllClientsFromCache() to be called, which in turn deletes our
-    // PatternData object! Leaving out the line below will cause svg/dynamic-updates/SVGPatternElement-svgdom* to crash.
     SVGPatternElement* patternElement = static_cast<SVGPatternElement*>(node());
     if (!patternElement)
-        return false;
+        return 0;
 
     if (m_shouldCollectPatternAttributes) {
         patternElement->updateAnimatedSVGAttribute(anyQName());
@@ -77,64 +72,79 @@
         m_shouldCollectPatternAttributes = false;
     }
 
-    // Spec: When the geometry of the applicable element has no width or height and objectBoundingBox is specified,
-    // then the given effect (e.g. a gradient or a filter) will be ignored.
-    FloatRect objectBoundingBox = object->objectBoundingBox();
-    if (m_attributes.patternUnits() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX && objectBoundingBox.isEmpty())
-        return false;
+    // If we couldn't determine the pattern content element root, stop here.
+    if (!m_attributes.patternContentElement())
+        return 0;
 
-    OwnPtr<PatternData>& patternData = m_patternMap.add(object, nullptr).iterator->second;
-    if (!patternData)
-        patternData = adoptPtr(new PatternData);
+    // Compute all necessary transformations to build the tile image & the pattern.
+    FloatRect tileBoundaries;
+    AffineTransform tileImageTransform;
+    if (!buildTileImageTransform(object, m_attributes, patternElement, tileBoundaries, tileImageTransform))
+        return 0;
 
-    if (!patternData->pattern) {
-        // If we couldn't determine the pattern content element root, stop here.
-        if (!m_attributes.patternContentElement())
-            return false;
+    AffineTransform absoluteTransformIgnoringRotation;
+    SVGRenderingContext::calculateTransformationToOutermostSVGCoordinateSystem(object, absoluteTransformIgnoringRotation);
 
-        // Compute all necessary transformations to build the tile image & the pattern.
-        FloatRect tileBoundaries;
-        AffineTransform tileImageTransform;
-        if (!buildTileImageTransform(object, m_attributes, patternElement, tileBoundaries, tileImageTransform))
-            return false;
+    // Ignore 2D rotation, as it doesn't affect the size of the tile.
+    SVGRenderingContext::clear2DRotation(absoluteTransformIgnoringRotation);
+    FloatRect absoluteTileBoundaries = absoluteTransformIgnoringRotation.mapRect(tileBoundaries);
+    FloatRect clampedAbsoluteTileBoundaries;
 
-        AffineTransform absoluteTransformIgnoringRotation;
-        SVGRenderingContext::calculateTransformationToOutermostSVGCoordinateSystem(object, absoluteTransformIgnoringRotation);
+    // Scale the tile size to match the scale level of the patternTransform.
+    absoluteTileBoundaries.scale(static_cast<float>(m_attributes.patternTransform().xScale()),
+        static_cast<float>(m_attributes.patternTransform().yScale()));
 
-        // Ignore 2D rotation, as it doesn't affect the size of the tile.
-        SVGRenderingContext::clear2DRotation(absoluteTransformIgnoringRotation);
-        FloatRect absoluteTileBoundaries = absoluteTransformIgnoringRotation.mapRect(tileBoundaries);
-        FloatRect clampedAbsoluteTileBoundaries;
+    // Build tile image.
+    OwnPtr<ImageBuffer> tileImage = createTileImage(m_attributes, tileBoundaries, absoluteTileBoundaries, tileImageTransform, clampedAbsoluteTileBoundaries);
+    if (!tileImage)
+        return 0;
 
-        // Scale the tile size to match the scale level of the patternTransform.
-        absoluteTileBoundaries.scale(static_cast<float>(m_attributes.patternTransform().xScale()),
-            static_cast<float>(m_attributes.patternTransform().yScale()));
+    RefPtr<Image> copiedImage = tileImage->copyImage(CopyBackingStore);
+    if (!copiedImage)
+        return 0;
 
-        // Build tile image.
-        OwnPtr<ImageBuffer> tileImage = createTileImage(m_attributes, tileBoundaries, absoluteTileBoundaries, tileImageTransform, clampedAbsoluteTileBoundaries);
-        if (!tileImage)
-            return false;
+    // Build pattern.
+    OwnPtr<PatternData> patternData = adoptPtr(new PatternData);
+    patternData->pattern = Pattern::create(copiedImage, true, true);
 
-        RefPtr<Image> copiedImage = tileImage->copyImage(CopyBackingStore);
-        if (!copiedImage)
-            return false;
+    // Compute pattern space transformation.
+    const IntSize tileImageSize = tileImage->logicalSize();
+    patternData->transform.translate(tileBoundaries.x(), tileBoundaries.y());
+    patternData->transform.scale(tileBoundaries.width() / tileImageSize.width(), tileBoundaries.height() / tileImageSize.height());
 
-        // Build pattern.
-        patternData->pattern = Pattern::create(copiedImage, true, true);
-        if (!patternData->pattern)
-            return false;
+    AffineTransform patternTransform = m_attributes.patternTransform();
+    if (!patternTransform.isIdentity())
+        patternData->transform = patternTransform * patternData->transform;
 
-        // Compute pattern space transformation.
-        patternData->transform.translate(tileBoundaries.x(), tileBoundaries.y());
-        patternData->transform.scale(tileBoundaries.width() / clampedAbsoluteTileBoundaries.width(), tileBoundaries.height() / clampedAbsoluteTileBoundaries.height());
+    // Account for text drawing resetting the context to non-scaled, see SVGInlineTextBox::paintTextWithShadows.
+    if (resourceMode & ApplyToTextMode) {
+        AffineTransform additionalTextTransformation;
+    }
+    patternData->pattern->setPatternSpaceTransform(patternData->transform);
 
-        AffineTransform patternTransform = m_attributes.patternTransform();
-        if (!patternTransform.isIdentity())
-            patternData->transform = patternTransform * patternData->transform;
+    // Various calls above may trigger invalidations in some fringe cases (ImageBuffer allocation
+    // failures in the SVG image cache for example). To avoid having our PatternData deleted by
+    // removeAllClientsFromCache(), we only make it visible in the cache at the very end.
+    return m_patternMap.set(object, patternData.release()).iterator->second.get();
+}
 
-        patternData->pattern->setPatternSpaceTransform(patternData->transform);
-    }
+bool RenderSVGResourcePattern::applyResource(RenderObject* object, RenderStyle* style, GraphicsContext*& context, unsigned short resourceMode)
+{
+    ASSERT(object);
+    ASSERT(style);
+    ASSERT(context);
+    ASSERT(resourceMode != ApplyToDefaultMode);
 
+    // Spec: When the geometry of the applicable element has no width or height and objectBoundingBox is specified,
+    // then the given effect (e.g. a gradient or a filter) will be ignored.
+    FloatRect objectBoundingBox = object->objectBoundingBox();
+    if (m_attributes.patternUnits() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX && objectBoundingBox.isEmpty())
+        return false;
+
+    PatternData* patternData = buildPattern(object, resourceMode);
+    if (!patternData)
+        return false;
+        
     // Draw pattern
     context->save();
 

Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h (148149 => 148150)


--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h	2013-04-10 23:25:53 UTC (rev 148149)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h	2013-04-10 23:34:10 UTC (rev 148150)
@@ -64,6 +64,8 @@
                                             const FloatRect& absoluteTileBoundaries, const AffineTransform& tileImageTransform,
                                             FloatRect& clampedAbsoluteTileBoundaries) const;
 
+    PatternData* buildPattern(RenderObject*, unsigned short resourceMode);
+
     bool m_shouldCollectPatternAttributes : 1;
     PatternAttributes m_attributes;
     HashMap<RenderObject*, OwnPtr<PatternData> > m_patternMap;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to