Bug 138230

Summary: Clean up virtual functions in css/
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: CSSAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 2014-10-30 14:20:44 PDT
Clean up virtual functions in css/ by:
- Making virtual functions final when possible
- Making classes final when possible
- Using 'override' when appropriate
- Explicitly marking functions / destructors as virtual when they are
  inherently virtual
- Making isXXX() virtual functions private on XXX classes to avoid
  unnecessary type checks
Comment 1 Chris Dumez 2014-10-30 14:22:46 PDT
Created attachment 240696 [details]
Patch
Comment 2 Benjamin Poulain 2014-10-30 15:20:28 PDT
Comment on attachment 240696 [details]
Patch

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

> Source/WebCore/css/CSSRuleList.h:62
> +    virtual CSSStyleSheet* styleSheet() const override { return 0; }

0 -> nullptr

> Source/WebCore/css/CSSRuleList.h:69
> +    virtual CSSRule* item(unsigned index) const override { return index < m_rules.size() ? m_rules[index].get() : 0; }

ditto

I wonder why it is valid to ask for a rule index > length...
Comment 3 Chris Dumez 2014-10-30 15:23:39 PDT
Comment on attachment 240696 [details]
Patch

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

>> Source/WebCore/css/CSSRuleList.h:69
>> +    virtual CSSRule* item(unsigned index) const override { return index < m_rules.size() ? m_rules[index].get() : 0; }
> 
> ditto
> 
> I wonder why it is valid to ask for a rule index > length...

The spec says so: http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSRuleList
Comment 4 Chris Dumez 2014-10-30 15:24:22 PDT
Created attachment 240707 [details]
Patch
Comment 5 WebKit Commit Bot 2014-10-30 16:08:16 PDT
Comment on attachment 240707 [details]
Patch

Clearing flags on attachment: 240707

Committed r175391: <http://trac.webkit.org/changeset/175391>
Comment 6 WebKit Commit Bot 2014-10-30 16:08:20 PDT
All reviewed patches have been landed.  Closing bug.