[webkit-reviews] review granted: [Bug 214094] Aggressively prime GameController.framework gamepad provider : [Attachment 403815] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 214094: Aggressively prime GameController.framework gamepad provider
https://bugs.webkit.org/show_bug.cgi?id=214094

Attachment 403815: Patch

https://bugs.webkit.org/attachment.cgi?id=403815&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 403815
  --> https://bugs.webkit.org/attachment.cgi?id=403815
Patch

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

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:107
> +    static dispatch_once_t onceToken;
> +    dispatch_once(&onceToken, ^{
> +	   LOG(Gamepad, "GameControllerGamepadProvider explicitly starting
GameController framework monitoring");
> +	   [getGCControllerClass()
__openXPC_and_CBApplicationDidBecomeActive__];
> +    });

Is thread safety an issue here? Otherwise, we would probably just want to use a
boolean, not a dispatch_once_t.

> Source/WebKit/WebProcess/Gamepad/WebGamepadProvider.cpp:72
> +    LOG(Gamepad, "WebGamepadProvider::gamepadConnected - Gamepad index %i
attached (visibility: %i)\n", gamepadData.index(), (int)eventVisibility);

Since gamepadData.index() is an unsigned, this should be %u, not %i.

> Source/WebKit/WebProcess/Gamepad/WebGamepadProvider.cpp:95
> +    LOG(Gamepad, "WebGamepadProvider::gamepadDisconnected - Gamepad index %i
detached (m_gamepads size %lu, m_rawGamepads size %lu\n", index,
m_gamepads.size(), m_rawGamepads.size());

Since index is an unsigned, this should be %u, not %i.

Since a Vector size is a size_t, the others should be %zu, not %lu.

> Source/WebKit/WebProcess/Gamepad/WebGamepadProvider.cpp:103
> +    LOG(Gamepad, "WebGamepadProvider::gamepadActivity - %lu gamepad datas
with %lu local web gamepads\n", gamepadDatas.size(), m_gamepads.size());

Since a Vector size is a size_t, these should be %zu, not %lu.