Use FINAL macro for classes whose virtual destructor was removed

Use FINAL macro for classes whose virtual destructor was removed

Use FINAL macro for classes whose virtual destructor was removed, to make sure
no one tries to subclass them in the future.

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

[eseidel, 2013-10-27 17:16:41]

Do we expect this to change the binary clang produces (I don't think gcc
respects class-level final yet does it?) or were these all already non-vtable'd
classes?

[eseidel, 2013-10-27 17:47:01]

Regardless, lgtm.  If it builds, it works. :)

[abarth-chromium, 2013-10-27 17:57:28]

On 2013/10/27 17:16:41, eseidel wrote:
> Do we expect this to change the binary clang produces (I don't think gcc
> respects class-level final yet does it?) or were these all already
non-vtable'd
> classes?

He's been removing virtual destructors from classes that would otherwise lack a
vtable.  I'd expect this not to change the binary.

Does this mean we should mark all leaf classes as final?  I think we already
have things set up so that it's an error to have virtual functions without a
virtual destructor.

[Chris Evans, 2013-10-27 18:24:24]

On 2013/10/27 17:16:41, eseidel wrote:
> Do we expect this to change the binary clang produces (I don't think gcc
> respects class-level final yet does it?)

GCC does, but not in the older version we tend to use (which is the Ubuntu 12.04
LTS one).

> or were these all already non-vtable'd
> classes?

[Inactive, 2013-10-28 13:11:35]

On 2013/10/27 17:57:28, abarth wrote:
> On 2013/10/27 17:16:41, eseidel wrote:
> > Do we expect this to change the binary clang produces (I don't think gcc
> > respects class-level final yet does it?) or were these all already
> non-vtable'd
> > classes?
> 
> He's been removing virtual destructors from classes that would otherwise lack
a
> vtable.  I'd expect this not to change the binary.
> 
> Does this mean we should mark all leaf classes as final?  I think we already
> have things set up so that it's an error to have virtual functions without a
> virtual destructor.

Yes, this was my comment on the bug as well. Even though those classes used to
have a virtual destructor, they are not any different than the others leaf
classes we have in Blink. If we add FINAL to these then I think we should
generalize it to all leaf classes.

My current understanding is that it does not affect the generated code since
these classes are completely non-virtual. However, it offer extra safety by
preventing someone to introduce a subclass with virtual methods.

[commit-bot: I haz the power, 2013-10-29 17:32:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/47613002/1

[Ken Russell (switch to Gerrit), 2013-10-29 18:45:39]

Thanks for making this change. It'll help avoid accidents in the future. LGTM

[commit-bot: I haz the power, 2013-10-29 19:05:23]

Change committed as 160852