Bug 112773 – Move repaint methods from RenderObject to RenderBox

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

Move repaint methods from RenderObject to RenderBox

[Ojan Vafai]

Created attachment 193973 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 193973 [details]
Patch

OK.  What about SVG?  IIRC repaintDuringLayoutIfMoved is part of the layoutDelta stuff?

[Build Bot]

Comment on attachment 193973 [details]
Patch

Attachment 193973 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17193407

[Ojan Vafai]

(In reply to comment #2)
> (From update of attachment 193973 [details])
> OK.  What about SVG?  IIRC repaintDuringLayoutIfMoved is part of the layoutDelta stuff?

Nope, that's the very similarly named, but different repaintAfterLayoutIfNeeded, which is used by LayoutRepainter. There's also checkForRepaintDuringLayout, which is used by SVG as well as other RenderObject subclasses.

[Ojan Vafai]

Created attachment 193978 [details]
fix mac build

[Julien Chaffraix]

Comment on attachment 193978 [details]
fix mac build

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

> Source/WebCore/rendering/RenderBox.h:381
> +    virtual void repaintOverhangingFloats(bool) { };

We should switch that function to using an enum or at least keep the parameter name. Not providing it makes it hard to see what this bool is supposed to be.

[Ojan Vafai]

Comment on attachment 193978 [details]
fix mac build

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

>> Source/WebCore/rendering/RenderBox.h:381
>> +    virtual void repaintOverhangingFloats(bool) { };
> 
> We should switch that function to using an enum or at least keep the parameter name. Not providing it makes it hard to see what this bool is supposed to be.

Yeah, I should switch it to an enum...I guess I'll just do that. Keeping the parameter name doesn't work because then the Mac build complains about an unused variable name.

[Tony Chang]

Comment on attachment 193978 [details]
fix mac build

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

Can you add OVERRIDE to RenderBlock::repaintOverhangingFloats?

>>> Source/WebCore/rendering/RenderBox.h:381
>>> +    virtual void repaintOverhangingFloats(bool) { };
>> 
>> We should switch that function to using an enum or at least keep the parameter name. Not providing it makes it hard to see what this bool is supposed to be.
> 
> Yeah, I should switch it to an enum...I guess I'll just do that. Keeping the parameter name doesn't work because then the Mac build complains about an unused variable name.

You could use the UNUSED_PARAM macro and switch to an enum in a separate patch.  You could also not inline it since it's already virtual.

[Ojan Vafai]

Created attachment 194093 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 194093 [details]
Patch for landing

Clearing flags on attachment: 194093

Committed r146377: <http://trac.webkit.org/changeset/146377>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.