- Revision
- 259722
- Author
- [email protected]
- Date
- 2020-04-08 10:00:34 -0700 (Wed, 08 Apr 2020)
Log Message
Hit test with clipPath referencing parent element causes infinite recursion
https://bugs.webkit.org/show_bug.cgi?id=209773
<rdar://problem/60002347>
Reviewed by Geoffrey Garen.
Source/WebCore:
Upon further investigation, the original fix for the hit test in RenderSVGResourceClipper to prevent
infinite recursion was incomplete, as something such as a use element could easily cause another cycle which
would not be detected by the initial fix. Instead, by maintaining a set of visited elements, we can prevent
visiting the same element twice, and thus breaking any cycles which might occur in the SVG document. We
track these elements within the SVGHitTestCycleDetectionScope class, where the set of visited elements are
maintained statically, and instances of the class will manage the scope, as an RAII-style object.
This is covered by an existing test, but includes additional test cases which illustrate the more complex
document structure.
Tests: svg/hittest/svg-clip-path-child-element-with-use-root.html
svg/hittest/svg-clip-path-child-element-with-use.html
* rendering/svg/RenderSVGContainer.cpp:
(WebCore::RenderSVGContainer::nodeAtFloatPoint):
* rendering/svg/RenderSVGImage.cpp:
(WebCore::RenderSVGImage::nodeAtFloatPoint):
* rendering/svg/RenderSVGResourceClipper.cpp:
(WebCore::RenderSVGResourceClipper::hitTestClipContent):
* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::nodeAtPoint):
* rendering/svg/RenderSVGShape.cpp:
(WebCore::RenderSVGShape::nodeAtFloatPoint):
* rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::nodeAtFloatPoint):
* rendering/svg/SVGRenderSupport.cpp:
(WebCore::SVGRenderSupport::pointInClippingArea):
(WebCore::SVGHitTestCycleDetectionScope::SVGHitTestCycleDetectionScope):
(WebCore::SVGHitTestCycleDetectionScope::~SVGHitTestCycleDetectionScope):
(WebCore::SVGHitTestCycleDetectionScope::visitedElements):
(WebCore::SVGHitTestCycleDetectionScope::isEmpty):
(WebCore::SVGHitTestCycleDetectionScope::isVisiting):
* rendering/svg/SVGRenderSupport.h:
LayoutTests:
* svg/hittest/svg-clip-path-child-element-with-use-expected.txt: Added.
* svg/hittest/svg-clip-path-child-element-with-use-root-expected.txt: Added.
* svg/hittest/svg-clip-path-child-element-with-use-root.html: Added.
* svg/hittest/svg-clip-path-child-element-with-use.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (259721 => 259722)
--- trunk/LayoutTests/ChangeLog 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/LayoutTests/ChangeLog 2020-04-08 17:00:34 UTC (rev 259722)
@@ -1,3 +1,16 @@
+2020-04-08 Doug Kelly <[email protected]>
+
+ Hit test with clipPath referencing parent element causes infinite recursion
+ https://bugs.webkit.org/show_bug.cgi?id=209773
+ <rdar://problem/60002347>
+
+ Reviewed by Geoffrey Garen.
+
+ * svg/hittest/svg-clip-path-child-element-with-use-expected.txt: Added.
+ * svg/hittest/svg-clip-path-child-element-with-use-root-expected.txt: Added.
+ * svg/hittest/svg-clip-path-child-element-with-use-root.html: Added.
+ * svg/hittest/svg-clip-path-child-element-with-use.html: Added.
+
2020-04-08 Antoine Quint <[email protected]>
transition-property is not computed correctly when transition-duration is set to "inherit"
Added: trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-expected.txt (0 => 259722)
--- trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-expected.txt 2020-04-08 17:00:34 UTC (rev 259722)
@@ -0,0 +1 @@
+Tests SVG hit test with a recursive clipPath. Test passes if WebKit does not crash. PASS
Added: trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root-expected.txt (0 => 259722)
--- trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root-expected.txt 2020-04-08 17:00:34 UTC (rev 259722)
@@ -0,0 +1 @@
+Tests SVG hit test with a recursive clipPath. Test passes if WebKit does not crash. PASS
Added: trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root.html (0 => 259722)
--- trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root.html (rev 0)
+++ trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use-root.html 2020-04-08 17:00:34 UTC (rev 259722)
@@ -0,0 +1,21 @@
+<style>
+.path { -webkit-clip-path: url(#clippath); }
+</style>
+<script>
+function loadevent() {
+ document.caretRangeFromPoint(37, 70);
+ if (window.testRunner) {
+ document.body.innerText = "Tests SVG hit test with a recursive clipPath. Test passes if WebKit does not crash. PASS";
+ testRunner.dumpAsText();
+ }
+}
+</script>
+<body _onload_=loadevent()>
+<svg contentScriptType="text/ecmascript" id="svg">
+<path d="M16 8 L32 56" class="path" />
+<g id="group">
+ <text clip-path="url(#clippath)" to="currentColor">Text</text>
+</g>
+<clipPath id="clippath" clipPathUnits="objectBoundingBox">
+ <use href=""
+</clipPath>
Added: trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use.html (0 => 259722)
--- trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use.html (rev 0)
+++ trunk/LayoutTests/svg/hittest/svg-clip-path-child-element-with-use.html 2020-04-08 17:00:34 UTC (rev 259722)
@@ -0,0 +1,21 @@
+<style>
+.path { -webkit-clip-path: url(#clippath); }
+</style>
+<script>
+function loadevent() {
+ document.caretRangeFromPoint(37, 70);
+ if (window.testRunner) {
+ document.body.innerText = "Tests SVG hit test with a recursive clipPath. Test passes if WebKit does not crash. PASS";
+ testRunner.dumpAsText();
+ }
+}
+</script>
+<body _onload_=loadevent()>
+<svg contentScriptType="text/ecmascript">
+<path d="M16 8 L32 56" class="path" />
+<g id="group">
+ <text clip-path="url(#clippath)" to="currentColor">Text</text>
+</g>
+<clipPath id="clippath" clipPathUnits="objectBoundingBox">
+ <use href=""
+</clipPath>
Modified: trunk/Source/WebCore/ChangeLog (259721 => 259722)
--- trunk/Source/WebCore/ChangeLog 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/ChangeLog 2020-04-08 17:00:34 UTC (rev 259722)
@@ -1,3 +1,45 @@
+2020-04-08 Doug Kelly <[email protected]>
+
+ Hit test with clipPath referencing parent element causes infinite recursion
+ https://bugs.webkit.org/show_bug.cgi?id=209773
+ <rdar://problem/60002347>
+
+ Reviewed by Geoffrey Garen.
+
+ Upon further investigation, the original fix for the hit test in RenderSVGResourceClipper to prevent
+ infinite recursion was incomplete, as something such as a use element could easily cause another cycle which
+ would not be detected by the initial fix. Instead, by maintaining a set of visited elements, we can prevent
+ visiting the same element twice, and thus breaking any cycles which might occur in the SVG document. We
+ track these elements within the SVGHitTestCycleDetectionScope class, where the set of visited elements are
+ maintained statically, and instances of the class will manage the scope, as an RAII-style object.
+
+ This is covered by an existing test, but includes additional test cases which illustrate the more complex
+ document structure.
+
+ Tests: svg/hittest/svg-clip-path-child-element-with-use-root.html
+ svg/hittest/svg-clip-path-child-element-with-use.html
+
+ * rendering/svg/RenderSVGContainer.cpp:
+ (WebCore::RenderSVGContainer::nodeAtFloatPoint):
+ * rendering/svg/RenderSVGImage.cpp:
+ (WebCore::RenderSVGImage::nodeAtFloatPoint):
+ * rendering/svg/RenderSVGResourceClipper.cpp:
+ (WebCore::RenderSVGResourceClipper::hitTestClipContent):
+ * rendering/svg/RenderSVGRoot.cpp:
+ (WebCore::RenderSVGRoot::nodeAtPoint):
+ * rendering/svg/RenderSVGShape.cpp:
+ (WebCore::RenderSVGShape::nodeAtFloatPoint):
+ * rendering/svg/RenderSVGText.cpp:
+ (WebCore::RenderSVGText::nodeAtFloatPoint):
+ * rendering/svg/SVGRenderSupport.cpp:
+ (WebCore::SVGRenderSupport::pointInClippingArea):
+ (WebCore::SVGHitTestCycleDetectionScope::SVGHitTestCycleDetectionScope):
+ (WebCore::SVGHitTestCycleDetectionScope::~SVGHitTestCycleDetectionScope):
+ (WebCore::SVGHitTestCycleDetectionScope::visitedElements):
+ (WebCore::SVGHitTestCycleDetectionScope::isEmpty):
+ (WebCore::SVGHitTestCycleDetectionScope::isVisiting):
+ * rendering/svg/SVGRenderSupport.h:
+
2020-04-08 Antoine Quint <[email protected]>
transition-property is not computed correctly when transition-duration is set to "inherit"
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGContainer.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGContainer.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGContainer.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -165,7 +165,9 @@
if (!SVGRenderSupport::pointInClippingArea(*this, localPoint))
return false;
-
+
+ SVGHitTestCycleDetectionScope hitTestScope(*this);
+
for (RenderObject* child = lastChild(); child; child = child->previousSibling()) {
if (child->nodeAtFloatPoint(request, result, localPoint, hitTestAction)) {
updateHitTestResult(result, LayoutPoint(localPoint));
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGImage.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGImage.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGImage.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -226,6 +226,8 @@
if (!SVGRenderSupport::pointInClippingArea(*this, localPoint))
return false;
+ SVGHitTestCycleDetectionScope hitTestScope(*this);
+
if (hitRules.canHitFill) {
if (m_objectBoundingBox.contains(localPoint)) {
updateHitTestResult(result, LayoutPoint(localPoint));
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -268,6 +268,8 @@
if (!SVGRenderSupport::pointInClippingArea(*this, point))
return false;
+ SVGHitTestCycleDetectionScope hitTestScope(*this);
+
if (clipPathElement().clipPathUnits() == SVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
AffineTransform transform;
transform.translate(objectBoundingBox.location());
@@ -284,15 +286,6 @@
if (!renderer->isSVGShape() && !renderer->isSVGText() && !childNode->hasTagName(SVGNames::useTag))
continue;
- const RenderStyle& style = renderer->style();
- if (is<ReferenceClipPathOperation>(style.clipPath())) {
- auto& clipPath = downcast<ReferenceClipPathOperation>(*style.clipPath());
- AtomString id(clipPath.fragment());
- RenderSVGResourceClipper* clipper = getRenderSVGResourceById<RenderSVGResourceClipper>(document(), id);
- if (clipper == this)
- continue;
- }
-
IntPoint hitPoint;
HitTestResult result(hitPoint);
constexpr OptionSet<HitTestRequest::RequestType> hitType { HitTestRequest::SVGClipContent, HitTestRequest::DisallowUserAgentShadowContent };
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGRoot.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -402,6 +402,8 @@
LayoutPoint pointInParent = locationInContainer.point() - toLayoutSize(accumulatedOffset);
LayoutPoint pointInBorderBox = pointInParent - toLayoutSize(location());
+ ASSERT(SVGHitTestCycleDetectionScope::isEmpty());
+
// Test SVG content if the point is in our content box or it is inside the visualOverflowRect and the overflow is visible.
// FIXME: This should be an intersection when rect-based hit tests are supported by nodeAtFloatPoint.
if (contentBoxRect().contains(pointInBorderBox) || (!shouldApplyViewportClip() && visualOverflowRect().contains(pointInParent))) {
@@ -411,8 +413,10 @@
// FIXME: nodeAtFloatPoint() doesn't handle rect-based hit tests yet.
if (child->nodeAtFloatPoint(request, result, localPoint, hitTestAction)) {
updateHitTestResult(result, pointInBorderBox);
- if (result.addNodeToListBasedTestResult(child->node(), request, locationInContainer) == HitTestProgress::Stop)
+ if (result.addNodeToListBasedTestResult(child->node(), request, locationInContainer) == HitTestProgress::Stop) {
+ ASSERT(SVGHitTestCycleDetectionScope::isEmpty());
return true;
+ }
}
}
}
@@ -431,6 +435,8 @@
}
}
+ ASSERT(SVGHitTestCycleDetectionScope::isEmpty());
+
return false;
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGShape.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -366,6 +366,8 @@
if (!SVGRenderSupport::pointInClippingArea(*this, localPoint))
return false;
+ SVGHitTestCycleDetectionScope hitTestScope(*this);
+
PointerEventsHitRules hitRules(PointerEventsHitRules::SVG_PATH_HITTESTING, request, style().pointerEvents());
bool isVisible = (style().visibility() == Visibility::Visible);
if (isVisible || !hitRules.requireVisible) {
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -440,6 +440,8 @@
if (!SVGRenderSupport::pointInClippingArea(*this, localPoint))
return false;
+ SVGHitTestCycleDetectionScope hitTestScope(*this);
+
HitTestLocation hitTestLocation(LayoutPoint(flooredIntPoint(localPoint)));
return RenderBlock::nodeAtPoint(request, result, hitTestLocation, LayoutPoint(), hitTestAction);
}
Modified: trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/SVGRenderSupport.cpp 2020-04-08 17:00:34 UTC (rev 259722)
@@ -407,6 +407,9 @@
bool SVGRenderSupport::pointInClippingArea(const RenderElement& renderer, const FloatPoint& point)
{
+ if (SVGHitTestCycleDetectionScope::isVisiting(renderer))
+ return false;
+
ClipPathOperation* clipPathOperation = renderer.style().clipPath();
if (is<ShapeClipPathOperation>(clipPathOperation) || is<BoxClipPathOperation>(clipPathOperation))
return isPointInCSSClippingArea(renderer, point);
@@ -506,4 +509,33 @@
#endif
+SVGHitTestCycleDetectionScope::SVGHitTestCycleDetectionScope(const RenderElement& element)
+{
+ m_element = makeWeakPtr(&element);
+ auto result = visitedElements().add(m_element.get());
+ ASSERT_UNUSED(result, result.isNewEntry);
}
+
+SVGHitTestCycleDetectionScope::~SVGHitTestCycleDetectionScope()
+{
+ bool result = visitedElements().remove(*m_element.get());
+ ASSERT_UNUSED(result, result);
+}
+
+WeakHashSet<RenderElement>& SVGHitTestCycleDetectionScope::visitedElements()
+{
+ static NeverDestroyed<WeakHashSet<RenderElement>> s_visitedElements;
+ return s_visitedElements;
+}
+
+bool SVGHitTestCycleDetectionScope::isEmpty()
+{
+ return visitedElements().computesEmpty();
+}
+
+bool SVGHitTestCycleDetectionScope::isVisiting(const RenderElement& element)
+{
+ return visitedElements().contains(element);
+}
+
+}
Modified: trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h (259721 => 259722)
--- trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h 2020-04-08 17:00:19 UTC (rev 259721)
+++ trunk/Source/WebCore/rendering/svg/SVGRenderSupport.h 2020-04-08 17:00:34 UTC (rev 259722)
@@ -95,4 +95,17 @@
~SVGRenderSupport();
};
+class SVGHitTestCycleDetectionScope {
+ WTF_MAKE_NONCOPYABLE(SVGHitTestCycleDetectionScope);
+public:
+ SVGHitTestCycleDetectionScope(const RenderElement&);
+ ~SVGHitTestCycleDetectionScope();
+ static bool isEmpty();
+ static bool isVisiting(const RenderElement&);
+
+private:
+ static WeakHashSet<RenderElement>& visitedElements();
+ WeakPtr<RenderElement> m_element;
+};
+
} // namespace WebCore
