[Webkit-unassigned] [Bug 122371] Move paint() to RenderElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122371





--- Comment #5 from Antti Koivisto <koivisto at iki.fi>  2013-10-05 03:09:54 PST ---
(In reply to comment #4)
> Why is this check needed? We didn’t add a check in RenderFrameSet::paint, but we did here. I think the old code would have asserted if it ever called paint on a text renderer. And toRenderElement will assert. Is adding the check fixing some kind of bug?

It can be easily shown that RenderFrameSet can only have RenderElement children. This is definitely not true for all RenderBoxes. Almost everyone has their specialised paint but at least RenderSVGRoot calls to RenderBox::paint(). I'm not sure it can't have RenderSVGInlineText children.

The right move would probably be to get rid of RenderBox::paint completely.

> > Source/WebCore/rendering/RenderElement.h:90
> > +    virtual void paint(PaintInfo&, const LayoutPoint&) { }
> 
> Could this be pure virtual instead of empty? Are there any concrete element classes that actually paint nothing?

Just RenderLineBreak. I'll add empty paint there and make this pure virtual.

> Why is this check needed? We didn’t add a check in RenderFrameSet::paint, but we did here. I think the old code would have asserted if it ever called paint on a text renderer. And toRenderElement will assert. Is adding the check fixing some kind of bug?

Same as RenderBox. I can't easily prove that this can't happen. Old code would still be ok in release build. With new code we would have unsafe cast.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.