Devirtualize WebGLContextGroup, WebGLGetInfo and WebGLUniformLocation's destructors

Devirtualize WebGLContextGroup, WebGLGetInfo and WebGLUniformLocation's destructors

Devirtualize WebGLContextGroup, WebGLGetInfo and WebGLUniformLocation's destructors
as these are never subclassed nor do they have any virtual function. This avoids
creating unnecessary vtables.

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

[Ken Russell (switch to Gerrit), 2013-10-24 21:05:33]

What is the motivation for making this change? Even though these classes don't
have any superclasses with virtuals, and aren't subclassed, I think it is the
wrong direction to take the code in terms of maintainability.

There is no BUG= line in the CL pointing to a larger description. Please add
one.

[Inactive, 2013-10-24 21:32:18]

On 2013/10/24 21:05:33, Ken Russell wrote:
> What is the motivation for making this change? Even though these classes don't
> have any superclasses with virtuals, and aren't subclassed, I think it is the
> wrong direction to take the code in terms of maintainability.
> 
> There is no BUG= line in the CL pointing to a larger description. Please add
> one.

The motivation is that I believe we should avoid creating vtables for classes
that are clearly not meant to be subclassed.
These vtables have a cost in terms of memory:
- The memory used by the vtable for each of these classes
- An extra pointer for each instance object of these classes

They also have a runtime cost due to the destructor call indirection.

The thing is that we have a *lot* of virtual destructors in Blink that are not
strictly needed. In this end, the cost adds up.

I understand your argument is about maintainability and this is a valid point.
My proposal is to not blindly make the destructors virtual and keep them when it
is not technically needed only if the class is likely to get subclassed at some
point. I don't know these classes enough to argue one way or another, which is
why I added you as reviewer. If you believe those classes may be subclassed at
some point, I am happy to keep those destructors virtual for the sake of
maintainability.

I am also happy to file a bug for this if you agree to the change.

[Ken Russell (switch to Gerrit), 2013-10-25 20:59:40]

Please see my comment on http://crbug.com/311561 . I'd like that answered before
reviewing this CL.

[Inactive, 2013-10-25 21:56:24]

On 2013/10/25 20:59:40, Ken Russell wrote:
> Please see my comment on http://crbug.com/311561 . I'd like that answered
before
> reviewing this CL.

Marked the classes as FINAL as suggested on the bug.

[Ken Russell (switch to Gerrit), 2013-10-25 22:17:40]

Thank you. Very nice. LGTM

[commit-bot: I haz the power, 2013-10-25 22:25:09]

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

[commit-bot: I haz the power, 2013-10-25 23:33:09]

Change committed as 160638