Bug 161086 – GameController.framework backend for gamepad API

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

GameController.framework backend for gamepad API

This will be necessary for iOS support

[Brady Eidson]

And will include getting the tests working on iOS-sim

[Brady Eidson]

Created attachment 286923 [details]
Patch

Attaching WIP patch.

Still some cleanup to do in WebCore, and the WK2 part is obviously a hack just to let me test it...

...but it works as is!

[Brady Eidson]

(In reply to comment #1)
> And will include getting the tests working on iOS-sim

Not actually true - Filing a new separate bug for that.

[Brady Eidson]

(In reply to comment #3)
> (In reply to comment #1)
> > And will include getting the tests working on iOS-sim
> 
> Not actually true - Filing a new separate bug for that.

Filed https://bugs.webkit.org/show_bug.cgi?id=161198 for that.

[Brady Eidson]

Created attachment 286979 [details]
Patch

[WebKit Commit Bot]

Attachment 286979 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:61:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:85:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:88:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:91:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:94:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:97:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:100:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:103:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:106:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:110:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:113:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:116:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:119:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:122:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:125:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:144:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:147:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:150:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:153:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:156:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:159:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:163:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:166:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 24 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 286988 [details]
Patch

[WebKit Commit Bot]

Attachment 286988 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 24 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 286994 [details]
Patch

[WebKit Commit Bot]

Attachment 286994 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 24 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Created attachment 286996 [details]
Patch

[WebKit Commit Bot]

Attachment 286996 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:60:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:84:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:87:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:90:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:93:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:96:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:99:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:102:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:105:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:109:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:112:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:115:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:118:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:121:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:124:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:135:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:143:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:146:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:149:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:152:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:155:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:158:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:162:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:165:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 24 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Comment on attachment 286996 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286996&action=review

> Source/WebCore/ChangeLog:16
> +        * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp.
> +        * platform/gamepad/cocoa/GameControllerGamepad.mm: Added.
> +        (WebCore::GameControllerGamepad::GameControllerGamepad):
> +        (WebCore::GameControllerGamepad::setupAsExtendedGamepad):
> +        (WebCore::GameControllerGamepad::setupAsGamepad):

Do these actually get used by the UI process? Seems a bit weird for WK2 code to be conjuring up classes from WebCore/platform.

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:28
> +#if ENABLE(GAMEPAD) && defined(__LP64__)

Shouldn't ENABLE(GAMEPAD) only be set in 64-bit, via xcconfig files?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:45
> +    GameControllerGamepad(GCController *, unsigned index);

Gamepads are identified by index? What happens when they plug and unplug?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:82
> +    m_axisValues.resize(6);
> +    m_buttonValues.resize(8);
> +
> +    m_buttonValues[0] = m_extendedGamepad.get().buttonA.value;
> +    m_buttonValues[1] = m_extendedGamepad.get().buttonB.value;
> +    m_buttonValues[2] = m_extendedGamepad.get().buttonX.value;
> +    m_buttonValues[3] = m_extendedGamepad.get().buttonY.value;
> +    m_buttonValues[4] = m_extendedGamepad.get().leftShoulder.value;
> +    m_buttonValues[5] = m_extendedGamepad.get().rightShoulder.value;
> +    m_buttonValues[6] = m_extendedGamepad.get().leftTrigger.value;
> +    m_buttonValues[7] = m_extendedGamepad.get().rightTrigger.value;
> +
> +    m_axisValues[0] = m_extendedGamepad.get().leftThumbstick.xAxis.value;
> +    m_axisValues[1] = m_extendedGamepad.get().leftThumbstick.yAxis.value;
> +    m_axisValues[2] = m_extendedGamepad.get().rightThumbstick.xAxis.value;
> +    m_axisValues[3] = m_extendedGamepad.get().rightThumbstick.yAxis.value;
> +    m_axisValues[4] = m_extendedGamepad.get().dpad.xAxis.value;
> +    m_axisValues[5] = m_extendedGamepad.get().dpad.yAxis.value;

Cognitive dissonance that you resize m_axisValues first, then fill it second.

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:86
> +    m_extendedGamepad.get().buttonA.valueChangedHandler = ^(GCControllerButtonInput *, float value, BOOL) {
> +        m_buttonValues[0] = value;
> +    };

Are these callbacks called on the main thread? Is there really one per button?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136
> +        m_lastUpdateTime = monotonicallyIncreasingTime();

Performance.now() time?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86
> +    Timer m_inputNotificationTimer;

I don't think Timers work in the UI process.

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:37
> +static const double InputNotificationDelay = 0.05;

What units?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51
> +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility)

Can GCController be a reference?

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:64
> +
> +

Two lines.

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:76
> +void GameControllerGamepadProvider::controllerDidDisconnect(GCController *controller)

Ditto.

> Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:297
> +    WebKit::UIGamepadProvider::setUsesGameControllerFramework();

What is happening here?

[Brady Eidson]

(In reply to comment #13)
> Comment on attachment 286996 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=286996&action=review
> 
> > Source/WebCore/ChangeLog:16
> > +        * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp.
> > +        * platform/gamepad/cocoa/GameControllerGamepad.mm: Added.
> > +        (WebCore::GameControllerGamepad::GameControllerGamepad):
> > +        (WebCore::GameControllerGamepad::setupAsExtendedGamepad):
> > +        (WebCore::GameControllerGamepad::setupAsGamepad):
> 
> Do these actually get used by the UI process? Seems a bit weird for WK2 code
> to be conjuring up classes from WebCore/platform.

Yes they do, and that's not weird.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:28
> > +#if ENABLE(GAMEPAD) && defined(__LP64__)
> 
> Shouldn't ENABLE(GAMEPAD) only be set in 64-bit, via xcconfig files?

ENABLE(GAMEPAD) works fine in 32 bit with the IOHID backend.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.h:45
> > +    GameControllerGamepad(GCController *, unsigned index);
> 
> Gamepads are identified by index?

Yes.

> What happens when they plug and unplug?

When you unplug, that index is freed.
e.g. Plug-in two gamepads, indices 0 and 1 are taken.
Unplug gamepad 0, index 0 is now null.
Plug in a new gamepad, it takes over index 0.

This is specced behavior we already implement in IOHID and the other browsers implement.

> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:82
> > +    m_axisValues.resize(6);
> > +    m_buttonValues.resize(8);
> > +
> > +    m_buttonValues[0] = m_extendedGamepad.get().buttonA.value;
> > +    m_buttonValues[1] = m_extendedGamepad.get().buttonB.value;
> > +    m_buttonValues[2] = m_extendedGamepad.get().buttonX.value;
> > +    m_buttonValues[3] = m_extendedGamepad.get().buttonY.value;
> > +    m_buttonValues[4] = m_extendedGamepad.get().leftShoulder.value;
> > +    m_buttonValues[5] = m_extendedGamepad.get().rightShoulder.value;
> > +    m_buttonValues[6] = m_extendedGamepad.get().leftTrigger.value;
> > +    m_buttonValues[7] = m_extendedGamepad.get().rightTrigger.value;
> > +
> > +    m_axisValues[0] = m_extendedGamepad.get().leftThumbstick.xAxis.value;
> > +    m_axisValues[1] = m_extendedGamepad.get().leftThumbstick.yAxis.value;
> > +    m_axisValues[2] = m_extendedGamepad.get().rightThumbstick.xAxis.value;
> > +    m_axisValues[3] = m_extendedGamepad.get().rightThumbstick.yAxis.value;
> > +    m_axisValues[4] = m_extendedGamepad.get().dpad.xAxis.value;
> > +    m_axisValues[5] = m_extendedGamepad.get().dpad.yAxis.value;
> 
> Cognitive dissonance that you resize m_axisValues first, then fill it second.

Maybe I misunderstand... Are you suggesting reserveInitialCapacity with uncheckedAppend?
Because while I might be able to do that in this patch, I would immediately have to revert back to this in a near-future patch, where the number of elements might dynamically change.

> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:86
> > +    m_extendedGamepad.get().buttonA.valueChangedHandler = ^(GCControllerButtonInput *, float value, BOOL) {
> > +        m_buttonValues[0] = value;
> > +    };
> 
> Are these callbacks called on the main thread?

Yes

> Is there really one per button?

Yup.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepad.mm:136
> > +        m_lastUpdateTime = monotonicallyIncreasingTime();
> 
> Performance.now() time?

I'm deferring until https://bugs.webkit.org/show_bug.cgi?id=161150, as that will require changing more than just the platform layers.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86
> > +    Timer m_inputNotificationTimer;
> 
> I don't think Timers work in the UI process.

Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in the UI process.

What problem should I be on the lookout for?

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:37
> > +static const double InputNotificationDelay = 0.05;
> 
> What units?

Will update with std::chrono.

> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51
> > +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility)
> 
> Can GCController be a reference?

Probably!

> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:64
> > +
> > +
> 
> Two lines.
> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:76
> > +void GameControllerGamepadProvider::controllerDidDisconnect(GCController *controller)
> 
> Ditto.
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKProcessPool.mm:297
> > +    WebKit::UIGamepadProvider::setUsesGameControllerFramework();
> 
> What is happening here?

Allows an SPI client (such as MiniBrowser) tell WK2 on Mac to use GameController.framework instead of the default IOHID.

Critical for testing.

[Brady Eidson]

(In reply to comment #14)
> (In reply to comment #13)
> > Comment on attachment 286996 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=286996&action=review
> > 
> > > Source/WebCore/ChangeLog:16
> > > +        * platform/gamepad/cocoa/GameControllerGamepad.h: Copied from Source/WebKit2/UIProcess/Gamepad/mac/UIGamepadProviderHID.cpp.
> > > +        * platform/gamepad/cocoa/GameControllerGamepad.mm: Added.
> > > +        (WebCore::GameControllerGamepad::GameControllerGamepad):
> > > +        (WebCore::GameControllerGamepad::setupAsExtendedGamepad):
> > > +        (WebCore::GameControllerGamepad::setupAsGamepad):
> > 
> > Do these actually get used by the UI process? Seems a bit weird for WK2 code
> > to be conjuring up classes from WebCore/platform.
> 
> Yes they do, and that's not weird.

I meant to expand on this before saving the comment: It's not weird in that this is certainly not the first use, and there's no real reason I'm aware of that it shouldn't be done.

[Tim Horton]

> > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86
> > > +    Timer m_inputNotificationTimer;
> > 
> > I don't think Timers work in the UI process.
> 
> Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in
> the UI process.
> 
> What problem should I be on the lookout for?

It works, but you shouldn't, because of WK1/WK2 coexistence issues (web thread vs main thread confusion IIRC). Use RunLoop timer instead, like you'll notice all of the WK2 UI process code does.

[Brady Eidson]

(In reply to comment #14)
> (In reply to comment #13)> 
> > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:51
> > > +void GameControllerGamepadProvider::controllerDidConnect(GCController *controller, ConnectionVisibility visibility)
> > 
> > Can GCController be a reference?
> 
> Probably!

Update: GCController is an Objective-C object from a framework - Should not be passed around as a C++ reference.

[Brady Eidson]

(In reply to comment #16)
> > > > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.h:86
> > > > +    Timer m_inputNotificationTimer;
> > > 
> > > I don't think Timers work in the UI process.
> > 
> > Hmmmm... Weird. The HIDGamepadProvider also uses them and works just fine in
> > the UI process.
> > 
> > What problem should I be on the lookout for?
> 
> It works, but you shouldn't, because of WK1/WK2 coexistence issues (web
> thread vs main thread confusion IIRC). Use RunLoop timer instead, like
> you'll notice all of the WK2 UI process code does.

Changing the WebCore::Timer over in this patch, and filed https://bugs.webkit.org/show_bug.cgi?id=161407 for the IOHID provider.

[Brady Eidson]

Created attachment 287435 [details]
Patch

[Brady Eidson]

This new patch addresses all of Simon's feedback.

[Brady Eidson]

https://trac.webkit.org/changeset/205199