Clean up virtual functions in css/

Clean up virtual functions in css/

Clean up virtual functions in css/ by:
- Making classes final when possible
- Using 'override' when appropriate
- Making isXXX() virtual functions private on XXX classes

I also added a few nullptr usages.

Based on:
http://trac.webkit.org/changeset/175391

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185243

[rwlbuis, 2014-11-12 17:03:06]

rob.buis@samsung.com changed reviewers:
+ timloh@chromium.org

[rwlbuis, 2014-11-12 17:03:06]

PTAL

[Timothy Loh, 2014-11-12 18:07:05]

On 2014/11/12 17:03:06, rwlbuis wrote:
> PTAL

I don't understand the point of moving type() functions to be private (or why
they're different than the other virtual functions like cssText()), could you
explain more?

[rwlbuis, 2014-11-12 18:20:29]

On 2014/11/12 18:07:05, Timothy Loh wrote:
> On 2014/11/12 17:03:06, rwlbuis wrote:
> > PTAL
> 
> I don't understand the point of moving type() functions to be private (or why
> they're different than the other virtual functions like cssText()), could you
> explain more?

This is from the referenced patchset. I think the point is that calling type()
directly
on the subclasses is a bit pointless since that means you already have a
reference/pointer
to the subclass, so you'd know the type.
That is not the case if you have just a pointer to the base class. Also cssText
can still do some complex formatting, so I don't thing you can compare it to
the simple type getter.
I am pretty neutral on this and mainly took it because it is in the original
patchset. In general I like hiding API that should not be used though. I can
remove the type() changes from the patch if you prefer.

[Timothy Loh, 2014-11-12 18:31:08]

On 2014/11/12 18:20:29, rwlbuis wrote:
> On 2014/11/12 18:07:05, Timothy Loh wrote:
> > On 2014/11/12 17:03:06, rwlbuis wrote:
> > > PTAL
> > 
> > I don't understand the point of moving type() functions to be private (or
why
> > they're different than the other virtual functions like cssText()), could
you
> > explain more?
> 
> This is from the referenced patchset. I think the point is that calling type()
> directly
> on the subclasses is a bit pointless since that means you already have a
> reference/pointer
> to the subclass, so you'd know the type.
> That is not the case if you have just a pointer to the base class. Also
cssText
> can still do some complex formatting, so I don't thing you can compare it to
> the simple type getter.
> I am pretty neutral on this and mainly took it because it is in the original
> patchset. In general I like hiding API that should not be used though. I can
> remove the type() changes from the patch if you prefer.

If you already have the type, the compiler will optimise the call anyway (since
the subclasses are final).

lgtm anyway, I don't think we should spend too much time thinking about this
(but update the description since we aren't really avoiding any type checks)

[rwlbuis, 2014-11-12 19:26:07]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-11-12 19:26:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/719833002/20001

[commit-bot: I haz the power, 2014-11-12 20:35:22]

Committed patchset #2 (id:20001) as 185243