Bug 214094 – Aggressively prime GameController.framework gamepad provider

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Aggressively prime GameController.framework gamepad provider

GC.framework does some work on app activation assuming it was hard linked.

Since we soft link it, it doesn't quite work right.

So let's give it a kickstart.

[Brady Eidson]

Created attachment 403815 [details]
Patch

[Darin Adler]

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.

[Brady Eidson]

Created attachment 403829 [details]
PFL

[Tim Horton]

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?

[Brady Eidson]

(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)

[EWS]

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

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

[Radar WebKit Bug Importer]

<rdar://problem/65250548>