Bug 208279 - Hit test with clipPath referencing parent element causes infinite recursion
Summary: Hit test with clipPath referencing parent element causes infinite recursion
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Local Build
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-26 17:22 PST by Doug Kelly
Modified: 2020-06-17 12:00 PDT (History)
15 users (show)

See Also:


Attachments
Patch (4.41 KB, patch)
2020-02-26 17:28 PST, Doug Kelly
no flags Details | Formatted Diff | Diff
test case (will crash) (633 bytes, text/html)
2020-03-02 12:41 PST, Said Abou-Hallawa
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Doug Kelly 2020-02-26 17:22:11 PST
When using a hit test that checks an element nested inside a clipPath (but the child element refers to the clipPath), this can result in an infinite recursion.  An extra check is needed to break the cycle when performing the hit test.

<rdar://problem/58381090>
Comment 1 Doug Kelly 2020-02-26 17:28:30 PST
Created attachment 391817 [details]
Patch
Comment 2 Ryosuke Niwa 2020-02-27 19:53:55 PST
Comment on attachment 391817 [details]
Patch

Seems ok.
Comment 3 WebKit Commit Bot 2020-02-27 20:15:43 PST
Comment on attachment 391817 [details]
Patch

Clearing flags on attachment: 391817

Committed r257616: <https://trac.webkit.org/changeset/257616>
Comment 4 WebKit Commit Bot 2020-02-27 20:15:45 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Said Abou-Hallawa 2020-03-02 12:37:41 PST
Comment on attachment 391817 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391817&action=review

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:290
> +        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;
> +        }

I do not think this is the right solution. Detecting SVG resources cyclic referencing is more complicated than going one level up in the resources tree. For example if you delete the last two lines of your test case below:

    <clipPath id="clippath" clipPathUnits="objectBoundingBox">
    <text clip-path="url(#clippath)" to="currentColor">Text</text>

And you add these lines instead:

    <g id="group">
        <text clip-path="url(#clippath)" to="currentColor">Text</text>
    </g>
    <clipPath id="clippath" clipPathUnits="objectBoundingBox">
        <use href="#group"/>
    </clipPath>

The new test case will hit the following infinite recursive call stack:

#1	0x000000011c854e44 in WebCore::RenderBlock::nodeAtPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::HitTestLocation const&, WebCore::LayoutPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:1994
#2	0x000000011cc241e1 in WebCore::RenderSVGText::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGText.cpp:444
#3	0x000000011cbcc501 in WebCore::RenderSVGContainer::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGContainer.cpp:170
#4	0x000000011cbcc501 in WebCore::RenderSVGContainer::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGContainer.cpp:170
#5	0x000000011cbe0606 in WebCore::RenderSVGResourceClipper::hitTestClipContent(WebCore::FloatRect const&, WebCore::FloatPoint const&) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:293
#6	0x000000011c8553c8 in WebCore::RenderBlock::nodeAtPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::HitTestLocation const&, WebCore::LayoutPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:2048
#7	0x000000011cc241e1 in WebCore::RenderSVGText::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGText.cpp:444

Please see SVGResourcesCycleSolver::resolveCycles().
Comment 6 Said Abou-Hallawa 2020-03-02 12:41:19 PST
Created attachment 392173 [details]
test case (will crash)
Comment 7 Ryosuke Niwa 2020-03-02 22:49:01 PST
Let's track that in a separate bug.