Bug 20400 - Infinite recursion crash in WebCore::RenderSVGRoot::absoluteClippedOverflowRect on a <stop> element outside of a gradient block
Summary: Infinite recursion crash in WebCore::RenderSVGRoot::absoluteClippedOverflowRe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2008-08-15 07:26 PDT by Filipe Almeida
Modified: 2008-10-27 14:36 PDT (History)
2 users (show)

See Also:


Attachments
test case (crashes safari) (236 bytes, image/svg+xml)
2008-08-15 07:26 PDT, Filipe Almeida
no flags Details
Fix the crash (3.06 KB, patch)
2008-10-21 14:23 PDT, Eric Seidel (no email)
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filipe Almeida 2008-08-15 07:26:03 PDT
Webkit runs out of stack if it encounters a <stop> element outside a gradient block.
Comment 1 Filipe Almeida 2008-08-15 07:26:48 PDT
Created attachment 22816 [details]
test case (crashes safari)
Comment 2 Mark Rowe (bdash) 2008-08-15 08:28:19 PDT
[... snip ...]
#149625 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149626 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149627 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149628 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149629 0x02765064 in WebCore::RenderObject::absoluteClippedOverflowRect (this=0x19b0f7dc) at WebCore/rendering/RenderObject.cpp:1991
#149630 0x0277f83f in WebCore::RenderSVGRoot::absoluteClippedOverflowRect (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:232
#149631 0x02780417 in WebCore::RenderSVGRoot::layout (this=0x19b0f69c) at WebCore/rendering/RenderSVGRoot.cpp:96
#149632 0x02715897 in WebCore::RenderBlock::layoutBlockChildren (this=0x19b0f0bc, relayoutChildren=true, maxFloatBottom=@0xbfffde1c) at WebCore/rendering/RenderBlock.cpp:1281
#149633 0x02717625 in WebCore::RenderBlock::layoutBlock (this=0x19b0f0bc, relayoutChildren=true) at WebCore/rendering/RenderBlock.cpp:627
#149634 0x02705a3c in WebCore::RenderBlock::layout (this=0x19b0f0bc) at WebCore/rendering/RenderBlock.cpp:536
#149635 0x027b1db8 in WebCore::RenderView::layout (this=0x19b0f0bc) at WebCore/rendering/RenderView.cpp:118
#149636 0x024ea52b in WebCore::FrameView::layout (this=0x19b455b0, allowSubtree=true) at WebCore/page/FrameView.cpp:486
#149637 0x02439c1e in WebCore::Document::implicitClose (this=0x4978400) at WebCore/dom/Document.cpp:1591


Confirmed with TOT.
Comment 3 Mark Rowe (bdash) 2008-08-15 08:29:07 PDT
<rdar://problem/6152395>
Comment 4 Eric Seidel (no email) 2008-10-21 13:55:14 PDT
This seems like a fundamental misunderstanding in SVG's absoluteClippedOverflowRect implementation:

IntRect RenderSVGRoot::absoluteClippedOverflowRect()
{
    IntRect repaintRect;

    for (RenderObject* current = firstChild(); current != 0; current = current->nextSibling())
        repaintRect.unite(current->absoluteClippedOverflowRect());

#if ENABLE(SVG_FILTERS)
    // Filters can expand the bounding box
    SVGResourceFilter* filter = getFilterById(document(), style()->svgStyle()->filter());
    if (filter)
        repaintRect.unite(enclosingIntRect(filter->filterBBoxForItemBBox(repaintRect)));
#endif

    return repaintRect;
}

IntRect RenderObject::absoluteClippedOverflowRect()
{
    if (parent())
        return parent()->absoluteClippedOverflowRect();
    return IntRect();
}

These two recursively call each other. :(

One fix would be to add a absoluteClippedOverflowRect() implementation to RenderSVGGradientStop.  I'll do that  for now, but I think we may need more fixes to SVG here.
Comment 5 Eric Seidel (no email) 2008-10-21 14:23:07 PDT
Created attachment 24545 [details]
Fix the crash

 LayoutTests/ChangeLog                          |   10 ++++++++++
 LayoutTests/svg/custom/stop-crash-expected.txt |    1 +
 LayoutTests/svg/custom/stop-crash.svg          |    8 ++++++++
 WebCore/ChangeLog                              |   13 +++++++++++++
 WebCore/rendering/RenderSVGGradientStop.h      |   10 ++++++++--
 5 files changed, 40 insertions(+), 2 deletions(-)
Comment 6 Eric Seidel (no email) 2008-10-27 13:01:00 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/svg/custom/stop-crash-expected.txt
	A	LayoutTests/svg/custom/stop-crash.svg
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderSVGGradientStop.h
Committed r37899

Comment 7 Darin Adler 2008-10-27 13:12:30 PDT
I wish the comment explained things in a more positive way. It talks about "preventing a crash", but I'd like to understand more positively what the design is.
Comment 8 Eric Seidel (no email) 2008-10-27 14:36:49 PDT
(In reply to comment #7)
> I wish the comment explained things in a more positive way. It talks about
> "preventing a crash", but I'd like to understand more positively what the
> design is.

Me too.  I'm not sure the design is right.  I don't understand how absoluteClippedOverflowRect is supposed to work.  Hyatt may be the only one who does.