Diff
Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (148325 => 148326)
--- branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 00:46:21 UTC (rev 148325)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 00:49:23 UTC (rev 148326)
@@ -1,3 +1,21 @@
+2013-04-12 Tim Horton <[email protected]>
+
+ Merge r131488
+
+ 2012-10-16 Stephen Chenney <[email protected]>
+
+ An feImage that tries to render itself should be stopped
+ https://bugs.webkit.org/show_bug.cgi?id=94652
+
+ Reviewed by Eric Seidel.
+
+ Ref-test to verify that an feImage that tries to draw itself will not
+ crash. This test must be render in order to verify the result. Do not
+ convert to dumpAstext.
+
+ * svg/filters/feImage-self-referencing-expected.html: Added.
+ * svg/filters/feImage-self-referencing.html: Added.
+
2013-04-12 Ryosuke Niwa <[email protected]>
Merge 130717.
Added: branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing-expected.html (0 => 148326)
--- branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing-expected.html (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing-expected.html 2013-04-13 00:49:23 UTC (rev 148326)
@@ -0,0 +1,6 @@
+<html>
+ <body>
+ <p>PASS if no crash</p>
+ </body>
+</html>
+
Added: branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing.html (0 => 148326)
--- branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing.html (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/svg/filters/feImage-self-referencing.html 2013-04-13 00:49:23 UTC (rev 148326)
@@ -0,0 +1,23 @@
+<html>
+ <head>
+ <script>
+ _onload_ = function() {
+ document.getElementById("feImage").setAttributeNS('http://www.w3.org/1999/xlink', 'xlink:href', '#svgRoot')
+ }
+ </script>
+ </head>
+ <body>
+ <p>PASS if no crash</p>
+ <svg width="10" height="10" id="svgRoot" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <g>
+ <defs>
+ <filter id="imageFilter">
+ <feImage id="feImage" />
+ <feOffset dx="50" dy="50" />
+ </filter>
+ </defs>
+ <rect filter="url(#imageFilter)" x="0" y="0" width="50" height="50" fill="green" />
+ </g>
+ </svg>
+ </body>
+</html>
Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (148325 => 148326)
--- branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 00:46:21 UTC (rev 148325)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 00:49:23 UTC (rev 148326)
@@ -1,3 +1,65 @@
+2013-04-12 Tim Horton <[email protected]>
+
+ Merge r131488
+
+ 2012-10-16 Stephen Chenney <[email protected]>
+ An feImage that tries to render itself should be stopped
+ https://bugs.webkit.org/show_bug.cgi?id=94652
+
+ Reviewed by Eric Seidel.
+
+ An SVG feImage filter element will accept, as the src to render, an
+ SVG document that makes use of the feImage itself. This causes the
+ feImage to try to draw itself while already in the process of drawing
+ itself. Various problems arise from this. The invariant we wish to
+ maintain is that no element in the src tree of an feImage element
+ refers to that feImage.
+
+ This patch adds a flag to all FilterData objects that tracks whether or
+ not the filter is currently applying itself, and avoids applying the
+ filter recursively.
+
+ While it may seem better to catch this problem when the src is set, or
+ when the filter is built, that turns out to be challenging and
+ inefficient. Say we choose to test when the src atttribute is set. To
+ do so would require looking through all of the DOM nodes that will be
+ rendered for the src, finding all resources used, and checking if any
+ of them make use fo the feImage element that we are setting the source
+ for. The infrastructure is not in place to do that, and it would
+ involve walking a potentially very large portion of the DOM in order
+ to detect a very rare situation. Note that it is not enough just to
+ walk the DOM directly under the src; we also need to recursively follow any
+ resource links to see if they use the feImage (e.g. patterns or
+ masks or use or ...).
+
+ If we instead try to use the renderer node to find self referencing,
+ we need to recursively walk a potentially very large render tree,
+ tracing all resources in search of the feImage. This would need to be
+ done every time the filter is built, which is again a significant
+ overhead for a situation that is very unlikely to occur. And we do not
+ have methods that make it easy to find feImage filter effect nodes; they are
+ hidden behind filter resource nodes.
+
+ Hence the runtime check to catch the problem. The check must be in
+ FilterData and RenderSVGResourceFilter code because we must prevent
+ the destruction of the feImage when we encounter it recursively.
+
+ This patch also renames FilterData::builded to FilterData::isBuilt.
+
+ Test: svg/filters/feImage-self-referencing.html
+
+ * rendering/svg/RenderSVGResourceFilter.cpp:
+ (WebCore::ApplyingFilterEffectGuard): Guard to ensure that, in the future, we always
+ clear the isApplying flag even if the postApplyResource method returns early.
+ (WebCore::RenderSVGResourceFilter::applyResource): Do not apply a resource that is already applying and
+ rename builded to isBuilt.
+ (WebCore::RenderSVGResourceFilter::postApplyResource): Mark a resource as applying and clear after
+ it is done. Abort if a resource is already applying when the method begins. Rename builded to isBuilt.
+ (WebCore::RenderSVGResourceFilter::primitiveAttributeChanged): Rename builded to isBuilt.
+ * rendering/svg/RenderSVGResourceFilter.h:
+ (WebCore::FilterData::FilterData):
+ (FilterData): Add isApplying and rename builded to isBuilt.
+
2013-04-12 Ryosuke Niwa <[email protected]>
Merge 130717.
Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (148325 => 148326)
--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2013-04-13 00:46:21 UTC (rev 148325)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp 2013-04-13 00:49:23 UTC (rev 148326)
@@ -55,6 +55,25 @@
namespace WebCore {
+class ApplyingFilterEffectGuard {
+public:
+ ApplyingFilterEffectGuard(FilterData* data)
+ : m_filterData(data)
+ {
+ // The guard must be constructed when the filter is not applying.
+ ASSERT(!m_filterData->isApplying);
+ m_filterData->isApplying = true;
+ }
+
+ ~ApplyingFilterEffectGuard()
+ {
+ ASSERT(m_filterData->isApplying);
+ m_filterData->isApplying = false;
+ }
+
+ FilterData* m_filterData;
+};
+
RenderSVGResourceType RenderSVGResourceFilter::s_resourceType = FilterResourceType;
RenderSVGResourceFilter::RenderSVGResourceFilter(SVGFilterElement* node)
@@ -149,7 +168,7 @@
// draw the stored filter output, not the unfiltered object as well.
if (m_filter.contains(object)) {
FilterData* filterData = m_filter.get(object);
- if (filterData->builded)
+ if (filterData->isBuilt || filterData->isApplying)
return false;
delete m_filter.take(object); // Oops, have to rebuild, go through normal code path
@@ -274,7 +293,12 @@
return;
}
- if (!filterData->builded) {
+ // We have a cycle if we are already applying the data.
+ // This can occur due to FeImage referencing a source that makes use of the FEImage itself.
+ if (filterData->isApplying)
+ return;
+
+ if (!filterData->isBuilt) {
if (!filterData->savedContext) {
removeClientFromCache(object);
return;
@@ -288,16 +312,18 @@
#endif
}
+ ApplyingFilterEffectGuard isApplyingGuard(filterData);
+
FilterEffect* lastEffect = filterData->builder->lastEffect();
if (lastEffect && !filterData->boundaries.isEmpty() && !lastEffect->filterPrimitiveSubregion().isEmpty()) {
// This is the real filtering of the object. It just needs to be called on the
// initial filtering process. We just take the stored filter result on a
// second drawing.
- if (!filterData->builded)
+ if (!filterData->isBuilt)
filterData->filter->setSourceImage(filterData->sourceGraphicBuffer.release());
- // Always true if filterData is just built (filterData->builded is false).
+ // Always true if filterData is just built (filterData->isBuilt is false).
if (!lastEffect->hasResult()) {
lastEffect->apply();
lastEffect->correctFilterResultIfNeeded();
@@ -307,7 +333,7 @@
resultImage->transformColorSpace(ColorSpaceLinearRGB, ColorSpaceDeviceRGB);
#endif
}
- filterData->builded = true;
+ filterData->isBuilt = true;
ImageBuffer* resultImage = lastEffect->asImageBuffer();
if (resultImage) {
@@ -340,7 +366,7 @@
for (; it != end; ++it) {
FilterData* filterData = it->second;
- if (!filterData->builded)
+ if (!filterData->isBuilt)
continue;
SVGFilterBuilder* builder = filterData->builder.get();
Modified: branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h (148325 => 148326)
--- branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2013-04-13 00:46:21 UTC (rev 148325)
+++ branches/safari-536.30-branch/Source/WebCore/rendering/svg/RenderSVGResourceFilter.h 2013-04-13 00:49:23 UTC (rev 148326)
@@ -42,7 +42,8 @@
struct FilterData {
FilterData()
: savedContext(0)
- , builded(false)
+ , isBuilt(false)
+ , isApplying(false)
, markedForRemoval(false)
{
}
@@ -54,7 +55,8 @@
AffineTransform shearFreeAbsoluteTransform;
FloatRect boundaries;
FloatSize scale;
- bool builded : 1;
+ bool isBuilt : 1;
+ bool isApplying : 1;
bool markedForRemoval : 1;
};
