Modified: trunk/Source/WebCore/ChangeLog (136249 => 136250)
--- trunk/Source/WebCore/ChangeLog 2012-11-30 17:20:42 UTC (rev 136249)
+++ trunk/Source/WebCore/ChangeLog 2012-11-30 17:49:11 UTC (rev 136250)
@@ -1,3 +1,23 @@
+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):
+
2012-11-30 Alexander Pavlov <[email protected]>
Web Inspector: It's Sass, not SASS
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp (136249 => 136250)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp 2012-11-30 17:20:42 UTC (rev 136249)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp 2012-11-30 17:49:11 UTC (rev 136250)
@@ -55,20 +55,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());
@@ -78,71 +73,81 @@
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->value;
- 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.
- const IntSize tileImageSize = tileImage->logicalSize();
- patternData->transform.translate(tileBoundaries.x(), tileBoundaries.y());
- patternData->transform.scale(tileBoundaries.width() / tileImageSize.width(), tileBoundaries.height() / tileImageSize.height());
+ // Account for text drawing resetting the context to non-scaled, see SVGInlineTextBox::paintTextWithShadows.
+ if (resourceMode & ApplyToTextMode) {
+ AffineTransform additionalTextTransformation;
+ if (shouldTransformOnTextPainting(object, additionalTextTransformation))
+ patternData->transform *= 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->value.get();
+}
- // Account for text drawing resetting the context to non-scaled, see SVGInlineTextBox::paintTextWithShadows.
- if (resourceMode & ApplyToTextMode) {
- AffineTransform additionalTextTransformation;
- if (shouldTransformOnTextPainting(object, additionalTextTransformation))
- patternData->transform *= additionalTextTransformation;
- }
- 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: trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h (136249 => 136250)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h 2012-11-30 17:20:42 UTC (rev 136249)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourcePattern.h 2012-11-30 17:49:11 UTC (rev 136250)
@@ -66,6 +66,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;
