Bug 214664

Summary: Hard code some MFi controller devices instead of dynamically managing HID vs GameController.framework
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
thorton: review+
Patch for landing none

Description Brady Eidson 2020-07-22 16:20:26 PDT 
Hard code some MFi controller devices instead of dynamically managing HID vs GameController.framework

On pre-BigSur operating systems, dynamically answering whether we should use HID or GCF is intractable.

Therefore we should just hard code devices we think it's important for GCF to handle, and use HID for the others.
Comment 1 Radar WebKit Bug Importer 2020-07-22 16:20:43 PDT 
<rdar://problem/65961406>
Comment 2 Brady Eidson 2020-07-22 16:21:00 PDT 
I have this in progress, will wait on https://bugs.webkit.org/show_bug.cgi?id=214188 to land so we can keep testing with future changes
Comment 3 Radar WebKit Bug Importer 2020-07-22 16:21:16 PDT 
<rdar://problem/65961432>
Comment 4 Brady Eidson 2020-07-24 14:57:06 PDT 
Created attachment 405187 [details]
Patch
Comment 5 Brady Eidson 2020-07-24 15:14:34 PDT 
Created attachment 405188 [details]
Patch
Comment 6 Brady Eidson 2020-07-24 15:18:16 PDT 
Created attachment 405190 [details]
Patch
Comment 7 Brady Eidson 2020-07-24 15:30:42 PDT 
Created attachment 405191 [details]
Patch
Comment 8 Tim Horton 2020-07-24 16:11:07 PDT 
Comment on attachment 405191 [details]
Patch

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

Vast majority of this is a rs (test code)

> Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:42
> +bool GameControllerGamepadProvider::willHandleVendorAndProduct(uint16_t vendorID, uint16_t productID)

Allow me to register my objection at this code living in WebKit :D

> Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.mm:206
> +    CFNumberGetValue(cfVendorID, kCFNumberIntType, &vendorID);
> +    CFNumberGetValue(cfProductID, kCFNumberIntType, &productID);

Please don't use this; toll-free bridge to NSNumber and use the not-scary API. Even though it's fine in this case, it's nonideal to promote the dangerous one by using it.
Comment 9 Brady Eidson 2020-07-24 17:37:45 PDT 
(In reply to Tim Horton from comment #8)
> Comment on attachment 405191 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405191&action=review
> 
> Vast majority of this is a rs (test code)
> 
> > Source/WebCore/platform/gamepad/cocoa/GameControllerGamepadProvider.mm:42
> > +bool GameControllerGamepadProvider::willHandleVendorAndProduct(uint16_t vendorID, uint16_t productID)
> 
> Allow me to register my objection at this code living in WebKit :D

Noted. Your objection is second behind mine.

Can't wait to remove it.

> > Source/WebCore/platform/gamepad/mac/HIDGamepadProvider.mm:206
> > +    CFNumberGetValue(cfVendorID, kCFNumberIntType, &vendorID);
> > +    CFNumberGetValue(cfProductID, kCFNumberIntType, &productID);
> 
> Please don't use this; toll-free bridge to NSNumber and use the not-scary
> API. Even though it's fine in this case, it's nonideal to promote the
> dangerous one by using it.

Done.
Comment 10 Brady Eidson 2020-07-24 17:38:13 PDT 
Created attachment 405208 [details]
Patch for landing
Comment 11 EWS 2020-07-24 18:28:08 PDT 
Committed r264874: <https://trac.webkit.org/changeset/264874>

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