Bug 147963

Summary: Move RenderBox-specific Scroll Snap code from RenderElement to RenderBox
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebCore Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, esprehn+autocc, glenn, kondapallykalyan, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 147596    
Bug Blocks:    
Attachments:
Description Flags
Patch simon.fraser: review+

Description Brent Fulgham 2015-08-12 17:42:16 PDT 
Certain scroll snap routines were added to RenderElement before we decided that this code only applied to RenderBoxes. Rather than adding type checking in RenderElement to handle these objects, only do the RenderBox-specific logic in the RenderBox class.
Comment 1 Brent Fulgham 2015-08-12 17:49:24 PDT 
Created attachment 258863 [details]
Patch
Comment 2 Brent Fulgham 2015-08-12 18:20:44 PDT 
Committed r188370: <http://trac.webkit.org/changeset/188370>
Comment 3 zalan 2015-08-12 18:32:32 PDT 
Comment on attachment 258863 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.h:638
> +    void willBeRemovedFromTree() override;

I'd write out 'virtual' here.
Comment 4 Brent Fulgham 2015-08-12 19:20:20 PDT 
(In reply to comment #3)
> Comment on attachment 258863 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258863&action=review
> 
> > Source/WebCore/rendering/RenderBox.h:638
> > +    void willBeRemovedFromTree() override;
> 
> I'd write out 'virtual' here.

Darin says we don't do that anymore. It's redundant with the 'override' declaration.
Comment 5 zalan 2015-08-12 19:22:55 PDT 
Comment on attachment 258863 [details]
Patch

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

>>> Source/WebCore/rendering/RenderBox.h:638
>>> +    void willBeRemovedFromTree() override;
>> 
>> I'd write out 'virtual' here.
> 
> Darin says we don't do that anymore. It's redundant with the 'override' declaration.

Ok, thanks. Good to know.