Bug 214664 - Hard code some MFi controller devices instead of dynamically managing HID vs GameController.framework
Summary: Hard code some MFi controller devices instead of dynamically managing HID vs ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-22 16:20 PDT by Brady Eidson
Modified: 2020-07-24 23:11 PDT (History)
2 users (show)

See Also:

Attachments
Patch (107.54 KB, patch)
2020-07-24 14:57 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Patch (107.70 KB, patch)
2020-07-24 15:14 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Patch (107.71 KB, patch)
2020-07-24 15:18 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Patch (107.76 KB, patch)
2020-07-24 15:30 PDT, Brady Eidson 		thorton: review+
Details | Formatted Diff | Diff
Patch for landing (107.73 KB, patch)
2020-07-24 17:38 PDT, Brady Eidson 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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].