[webkit-reviews] review granted: [Bug 214188] Add Gamepad tests that exercise the native frameworks : [Attachment 405012] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 214188: Add Gamepad tests that exercise the native frameworks
https://bugs.webkit.org/show_bug.cgi?id=214188

Attachment 405012: Patch

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




--- Comment #12 from Tim Horton <thorton at apple.com> ---
Comment on attachment 405012
  --> https://bugs.webkit.org/attachment.cgi?id=405012
Patch

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

> Tools/TestWebKitAPI/Tests/mac/HIDGamepads.mm:98
> +    [[webView window] resignKeyWindow];
> +    [[webView window] makeKeyWindow];

This could use a comment (also I think it's technically working around a bug?
but I'm not sure what component the bug is in?)

> Tools/TestWebKitAPI/mac/VirtualGamepad.h:35
> + at class NSString;
> + at class HIDDevice;
> + at class HIDUserDevice;

Sort it! Also, any chance of this header ending up in C++ code? Maybe
OBJC_CLASS? (the rest of it seems C++-clean at a glance).

> Tools/TestWebKitAPI/mac/VirtualGamepad.mm:45
> +const uint8_t NimbusDescriptor[] = {

This all hurts a bit. Are there not constants for this? Also, I think
everything inside the {} should be indented.

> Tools/TestWebKitAPI/mac/VirtualGamepad.mm:130
> +    m_dispatchQueue = adoptNS(dispatch_queue_create(0,
DISPATCH_QUEUE_SERIAL));

I think this should be OSObjectPtr/adoptOSObject? But not 100%, and it may not
matter.

> Tools/TestWebKitAPI/mac/VirtualGamepad.mm:133
> +    auto uuid = adoptNS([[NSUUID alloc] init]);
> +    m_uniqueID = [uuid UUIDString];

I think NSUUID.UUID.UUIDString instead of these two