Bug 214094

Created attachment 403815 [details]
Patch

Comment on attachment 403815 [details]
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.

Created attachment 403829 [details]
PFL

Comment on attachment 403815 [details]
Patch

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

>> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:107
>> +    });
> 
> Is thread safety an issue here? Otherwise, we would probably just want to use a boolean, not a dispatch_once_t.

That is a super-internal-looking method. What is the actual way that this works normally? Just linking the framework probably isn't it; is it +initialize on some class or something?

(In reply to Tim Horton from comment #4)
> Comment on attachment 403815 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=403815&action=review
> 
> >> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:107
> >> +    });
> > 
> > Is thread safety an issue here? Otherwise, we would probably just want to use a boolean, not a dispatch_once_t.
> 
> That is a super-internal-looking method. What is the actual way that this
> works normally? Just linking the framework probably isn't it; is it
> +initialize on some class or something?

No, it's just linking the framework. (Will ping you on slack)

Committed r264143: <https://trac.webkit.org/changeset/264143>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403829 [details].

<rdar://problem/65250548>