Share Gamepad and WebKitGamepad implementation

Share Gamepad and WebKitGamepad implementation

Decided to just have C++ inheritance:
Factor out GamepadCommon base class from Gamepad and WebKitGamepad
https://codereview.chromium.org/195993007/

OLD:
Variant of:
Added non-prefixed navigator.getGamepads() with updated API
https://codereview.chromium.org/183313003/

...which shares the Blink implementation between the two IDL interfaces.

Note the use of [ImplementedAs] on both the interfaces
(to reuse the Gamepad implementation)
and on the |buttons| attribute (to provide a different
implementation of this attribute), as that differ between
Gamepad and WebKitGamepad.

For simplicity, CG-only changes in:
Add [DoNotGenerateClassBindings] extended attribute
https://codereview.chromium.org/196653003/

BUG=344556

[eseidel, 2014-03-12 20:05:48]

This looks really close.

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:45: virtual ~Gamepad();
What's virtual?  Do we need an OVERRIDE somewhere?

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:84: GamepadButtonVector m_buttons;
Unclear how these should relate.  If you call both webkitGetGamepads and
getGamepads should the buttons be identical?

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:8: #include "modules/gamepad/Gamepad.h"
This is presumably needed to keep the bindings generator happy?  Seems we should
add a comment about such?

Also, I don't think we need the header guards. :)

[eseidel, 2014-03-12 20:06:45]

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.cpp (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.cpp:52: for (unsigned i = 0; i < count; ++i) {
I'm not sure if it's simpler to do the copying into the old buttons array on the
setter or the getter.  One could also use a dirty-bit if we did it on the
getter.  Just depends on how often set vs. get is called I gues.  I doubt either
matters much.

[Nils Barth (inactive), 2014-03-13 08:55:46]

Thanks for reviews Eric!
Turns out sharing a C++ implementation really doesn't work,
and anyway C++ inheritance in clearer, so closing.

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.cpp (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.cpp:52: for (unsigned i = 0; i < count; ++i) {
On 2014/03/12 20:06:46, eseidel wrote:
> I'm not sure if it's simpler to do the copying into the old buttons array on
the
> setter or the getter.  One could also use a dirty-bit if we did it on the
> getter.  Just depends on how often set vs. get is called I gues.  I doubt
either
> matters much.

(This is from upstream; as noted, doesn't make much difference;
I'm guessing getter called more than setter, hence that's the simple one.)

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:45: virtual ~Gamepad();
On 2014/03/12 20:05:49, eseidel wrote:
> What's virtual?  Do we need an OVERRIDE somewhere?

(This was from upstream and is gone now.)

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.h:84: GamepadButtonVector m_buttons;
On 2014/03/12 20:05:49, eseidel wrote:
> Unclear how these should relate.  If you call both webkitGetGamepads and
> getGamepads should the buttons be identical?

This was mess from shoving both into the same class,
and is one reason we should use proper C++ inheritance
(with a separate base class).

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/W...
File Source/modules/gamepad/WebKitGamepad.h (right):

https://codereview.chromium.org/196503004/diff/40001/Source/modules/gamepad/W...
Source/modules/gamepad/WebKitGamepad.h:8: #include "modules/gamepad/Gamepad.h"
On 2014/03/12 20:05:49, eseidel wrote:
> This is presumably needed to keep the bindings generator happy?  Seems we
should
> add a comment about such?

Yup, this was for CG; agreed a comment would've been in order.
(This was just as "first look", and turned out it was abandoned.)

> Also, I don't think we need the header guards. :)

Probably safe ;)