Bug 112773 - Move repaint methods from RenderObject to RenderBox
Summary: Move repaint methods from RenderObject to RenderBox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-19 20:19 PDT by Ojan Vafai
Modified: 2013-03-20 12:24 PDT (History)
12 users (show)

See Also:


Attachments
Patch (3.58 KB, patch)
2013-03-19 20:21 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff
fix mac build (4.34 KB, patch)
2013-03-19 20:55 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch for landing (5.76 KB, patch)
2013-03-20 11:55 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2013-03-19 20:19:30 PDT
Move repaint methods from RenderObject to RenderBox
Comment 1 Ojan Vafai 2013-03-19 20:21:12 PDT
Created attachment 193973 [details]
Patch
Comment 2 Eric Seidel (no email) 2013-03-19 20:32:42 PDT
Comment on attachment 193973 [details]
Patch

OK.  What about SVG?  IIRC repaintDuringLayoutIfMoved is part of the layoutDelta stuff?
Comment 3 Build Bot 2013-03-19 20:35:04 PDT
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
Comment 4 Ojan Vafai 2013-03-19 20:37:56 PDT
(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.
Comment 5 Ojan Vafai 2013-03-19 20:55:04 PDT
Created attachment 193978 [details]
fix mac build
Comment 6 Julien Chaffraix 2013-03-20 09:16:32 PDT
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.
Comment 7 Ojan Vafai 2013-03-20 10:34:39 PDT
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.
Comment 8 Tony Chang 2013-03-20 10:44:28 PDT
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.
Comment 9 Ojan Vafai 2013-03-20 11:55:18 PDT
Created attachment 194093 [details]
Patch for landing
Comment 10 WebKit Review Bot 2013-03-20 12:24:41 PDT
Comment on attachment 194093 [details]
Patch for landing

Clearing flags on attachment: 194093

Committed r146377: <http://trac.webkit.org/changeset/146377>
Comment 11 WebKit Review Bot 2013-03-20 12:24:45 PDT
All reviewed patches have been landed.  Closing bug.