Changeset 285769 in webkit

Timestamp:

Nov 12, 2021, 6:12:05 PM (5 years ago)

Author:

Said Abou-Hallawa

Message:

REGRESSION(r285481): Infinite recursion with cyclic filter reference
​https://bugs.webkit.org/show_bug.cgi?id=232972
rdar://85264240

Reviewed by Wenson Hsieh.

Source/WebCore:

Before r285481, we were creating the ImageBuffer of the referenced SVGElement
for the FEImage through RenderSVGResourceFilter::postApplyResource(). Now
we create this ImageBuffer through RenderSVGResourceFilter::applyResource().
The difference is at the end of RenderSVGResourceFilter::applyResource()
we add an entry to m_rendererFilterDataMap. This entry was preventing
trying to rebuild the SVGFilter for the same renderer if there is a
cyclic reference.

The fix is to add the entry in m_rendererFilterDataMap before creating the
SVGFilter. If an error happens, this entry will be removed before returning.

Test: svg/filters/feImage-cyclic-reference.svg

• rendering/svg/RenderSVGResourceFilter.cpp:

(WebCore::RenderSVGResourceFilter::applyResource):

LayoutTests:

• svg/filters/feImage-cyclic-reference-expected.txt: Added.
• svg/filters/feImage-cyclic-reference.svg: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/filters/feImage-cyclic-reference-expected.txt (added)
• LayoutTests/svg/filters/feImage-cyclic-reference.svg (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-12  Said Abou-Hallawa  <said@apple.com>
+
+        REGRESSION(r285481): Infinite recursion with cyclic filter reference
+        https://bugs.webkit.org/show_bug.cgi?id=232972
+        rdar://85264240
+
+        Reviewed by Wenson Hsieh.
+
+        * svg/filters/feImage-cyclic-reference-expected.txt: Added.
+        * svg/filters/feImage-cyclic-reference.svg: Added.
+
 2021-11-12  Rob Buis  <rbuis@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-12  Said Abou-Hallawa  <said@apple.com>
+
+        REGRESSION(r285481): Infinite recursion with cyclic filter reference
+        https://bugs.webkit.org/show_bug.cgi?id=232972
+        rdar://85264240
+
+        Reviewed by Wenson Hsieh.
+
+        Before r285481, we were creating the ImageBuffer of the referenced SVGElement
+        for the FEImage through RenderSVGResourceFilter::postApplyResource(). Now
+        we create this ImageBuffer through RenderSVGResourceFilter::applyResource(). 
+        The difference is at the end of RenderSVGResourceFilter::applyResource()
+        we add an entry to m_rendererFilterDataMap. This entry was preventing 
+        trying to rebuild the SVGFilter for the same renderer if there is a
+        cyclic reference.
+
+        The fix is to add the entry in m_rendererFilterDataMap before creating the
+        SVGFilter. If an error happens, this entry will be removed before returning.
+
+        Test: svg/filters/feImage-cyclic-reference.svg
+
+        * rendering/svg/RenderSVGResourceFilter.cpp:
+        (WebCore::RenderSVGResourceFilter::applyResource):
+
 2021-11-12  Rob Buis  <rbuis@igalia.com>
 

trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp

@@ -97 +97 @@
     }
 
-    auto filterData = makeUnique<FilterData>();
+    auto addResult = m_rendererFilterDataMap.set(&renderer, makeUnique<FilterData>());
+    auto filterData = addResult.iterator->value.get();
+    
     FloatRect targetBoundingBox = renderer.objectBoundingBox();
 
     filterData->boundaries = SVGLengthContext::resolveRectangle<SVGFilterElement>(&filterElement(), filterElement().filterUnits(), targetBoundingBox);
-    if (filterData->boundaries.isEmpty())
-        return false;
+    if (filterData->boundaries.isEmpty()) {
+        m_rendererFilterDataMap.remove(&renderer);
+        return false;
+    }
 
     // Determine absolute transformation matrix for filter. 
     AffineTransform absoluteTransform = SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer);
-    if (!absoluteTransform.isInvertible())
-        return false;
+    if (!absoluteTransform.isInvertible()) {
+        m_rendererFilterDataMap.remove(&renderer);
+        return false;
+    }
 
     // Eliminate shear of the absolute transformation matrix, to be able to produce unsheared tile images for feTile.
@@ -124 +130 @@
     filterData->builder = makeUnique<SVGFilterBuilder>();
     filterData->filter = SVGFilter::create(filterElement(), *filterData->builder, filterScale, absoluteDrawingRegion, filterData->boundaries, targetBoundingBox);
-    if (!filterData->filter)
-        return false;
+    if (!filterData->filter) {
+        m_rendererFilterDataMap.remove(&renderer);
+        return false;
+    }
 
     auto lastEffect = filterData->builder->lastEffect();
@@ -145 +153 @@
     // Even if the target objectBoundingBox() is empty, we still have to draw the last effect result image in postApplyResource.
     if (filterData->drawingRegion.isEmpty()) {
-        ASSERT(!m_rendererFilterDataMap.contains(&renderer));
+        ASSERT(m_rendererFilterDataMap.contains(&renderer));
         filterData->savedContext = context;
-        m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
         return false;
     }
@@ -162 +169 @@
     auto sourceGraphic = SVGRenderingContext::createImageBuffer(filterData->drawingRegion, effectiveTransform, colorSpace, renderingMode, context);
     if (!sourceGraphic) {
-        ASSERT(!m_rendererFilterDataMap.contains(&renderer));
+        ASSERT(m_rendererFilterDataMap.contains(&renderer));
         filterData->savedContext = context;
-        m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
         return false;
     }
@@ -178 +184 @@
     context = &sourceGraphicContext;
 
-    ASSERT(!m_rendererFilterDataMap.contains(&renderer));
-    m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
-
+    ASSERT(m_rendererFilterDataMap.contains(&renderer));
     return true;
 }