Bug 138230 - Clean up virtual functions in css/
Summary: Clean up virtual functions in css/
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-30 14:20 PDT by Chris Dumez
Modified: 2014-10-30 16:08 PDT (History)
3 users (show)

See Also:


Attachments
Patch (26.46 KB, patch)
2014-10-30 14:22 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (26.50 KB, patch)
2014-10-30 15:24 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.