GameController.framework backend for gamepad API This will be necessary for iOS support
And will include getting the tests working on iOS-sim
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!
(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.
(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.
Created attachment 286979 [details] Patch
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.
Created attachment 286988 [details] Patch
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.
Created attachment 286994 [details] Patch
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.
Created attachment 286996 [details] Patch
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.
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?
(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.
(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.
> > > 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.
(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.
(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.
Created attachment 287435 [details] Patch
This new patch addresses all of Simon's feedback.
https://trac.webkit.org/changeset/205199