Updating Blink-side WebGamepad interface

Updating Blink-side Gamepad API to match latest spec

BUG=344556
R=abarth@chromium.org, scottmg@chromium.org

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

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

[scottmg, 2014-02-18 23:46:31]

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

https://codereview.chromium.org/170993002/diff/30001/Source/modules/gamepad/G...
Source/modules/gamepad/Gamepad.cpp:31: const float ButtonPressedThreshold =
0.1f;
As noted in the chromium-side review, I think this should happen in the data
fetcher, not at this level.

[bajones, 2014-02-20 00:45:02]

Okay, here's the story with this CL at the moment: The Gamepad API as we have it
implemented today does not match the latest spec or Firefox's implementation. We
want to update, but there's also concerns about backwards compatibility. The
current idea is to possibly expose the newer API as an unprefixed version
alongside the prefixed API with the older standard. That still needs some
discussion before we move on it.

It's clear, however, that the WebGamepad API will need to be expanded to handle
the newer API structure, regardless of how we eventually choose to expose it to
Javascript. This requires both Blink and Chrome changes. In order to synchronize
the two we will land this CL first, with the newer structure hidden behind
#ifdefs. This CL does NOT break the existing Javascript API or introduce a new
one. 

Once the Blink has landed the corresponding Chrome CL
(https://codereview.chromium.org/165983005/) will land, rolling the DEPS to the
appropriate place and adding the required #define as a compile option. Follow up
CLs will then remove the #ifdef scaffolding on both ends. At that point all of
the data required for the newer Gamepad API will be accessible and it will be
far easier to negotiate how to expose that to the web.

[scottmg, 2014-02-20 00:59:46]

lgtm

https://codereview.chromium.org/170993002/diff/190001/Source/modules/gamepad/...
File Source/modules/gamepad/Gamepad.h (right):

https://codereview.chromium.org/170993002/diff/190001/Source/modules/gamepad/...
Source/modules/gamepad/Gamepad.h:56: void connected(const bool val) {
m_connected = val; }
nit; const bool seems odd

[abarth-chromium, 2014-02-20 06:12:44]

lgtm

https://codereview.chromium.org/170993002/diff/190001/public/platform/WebGame...
File public/platform/WebGamepad.h (right):

https://codereview.chromium.org/170993002/diff/190001/public/platform/WebGame...
public/platform/WebGamepad.h:113: #endif
What's the point of these COMPILE_ASSERTs if we're just going to change them to
some random value?

[bajones, 2014-02-20 17:03:24]

https://codereview.chromium.org/170993002/diff/190001/public/platform/WebGame...
File public/platform/WebGamepad.h (right):

https://codereview.chromium.org/170993002/diff/190001/public/platform/WebGame...
public/platform/WebGamepad.h:113: #endif
On 2014/02/20 06:12:45, abarth wrote:
> What's the point of these COMPILE_ASSERTs if we're just going to change them
to
> some random value?

Truthfully I'm not sure. The class must be a static size, consist of POD types,
and there are PPAPI structures that must match it exactly (which has it's own
compile-time check). I don't think this check helps enforce any of those goals,
though. It seems like more of an annoyance to me than anything. Maybe scottmg@
has some insight?

[scottmg, 2014-02-20 17:51:43]

I'd have to look what's happened over time, but at some point they were
stored in shared memory and reinterpreted as the other type on the other
side, so it was an attempt to make sure the two sides had the same compile
flags/padding.


On Thu, Feb 20, 2014 at 9:03 AM, <bajones@chromium.org> wrote:

>
> https://codereview.chromium.org/170993002/diff/190001/
> public/platform/WebGamepad.h
> File public/platform/WebGamepad.h (right):
>
> https://codereview.chromium.org/170993002/diff/190001/
> public/platform/WebGamepad.h#newcode113
> public/platform/WebGamepad.h:113: #endif
> On 2014/02/20 06:12:45, abarth wrote:
>
>> What's the point of these COMPILE_ASSERTs if we're just going to
>>
> change them to
>
>> some random value?
>>
>
> Truthfully I'm not sure. The class must be a static size, consist of POD
> types, and there are PPAPI structures that must match it exactly (which
> has it's own compile-time check). I don't think this check helps enforce
> any of those goals, though. It seems like more of an annoyance to me
> than anything. Maybe scottmg@ has some insight?
>
> https://codereview.chromium.org/170993002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[bajones, 2014-02-20 21:00:20]

The CQ bit was checked by bajones@chromium.org

[commit-bot: I haz the power, 2014-02-20 21:00:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bajones@chromium.org/170993002/270001

[bajones, 2014-02-20 21:49:59]

Committed patchset #6 manually as r167539 (presubmit successful).

[bajones, 2014-02-20 22:14:31]

A revert of this CL has been created in
https://codereview.chromium.org/171563007/ by bajones@chromium.org.

The reason for reverting is: Compiler errors on chromium.webkit waterfall.

[bajones, 2014-02-20 23:11:08]

Committed patchset #7 manually as r167548 (presubmit successful).