[webkit-reviews] review granted: [Bug 134381] HIDGamepads should populate themselves with initial input values : [Attachment 233962] Patch v2 - Now with 100% more ChangeLog

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 134381: HIDGamepads should populate themselves with initial input values
https://bugs.webkit.org/show_bug.cgi?id=134381

Attachment 233962: Patch v2 - Now with 100% more ChangeLog
https://bugs.webkit.org/attachment.cgi?id=233962&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=233962&action=review


Patch didn’t apply in EWS.

> Source/WebCore/platform/mac/HIDGamepad.cpp:75
> +    IOHIDValueRef value;

I think a local just before each call to IOHIDDeviceGetValue would be nicer
than sharing this local.

> Source/WebCore/platform/mac/HIDGamepad.cpp:79
> +	   IOHIDElementRef element = button->iohidElement.get();
> +	   if (IOHIDDeviceGetValue(IOHIDElementGetDevice(element), element,
&value) == kIOReturnSuccess)
> +	       valueChanged(value);

I know this sounds a little silly, but a helper that takes an const
HIDGamepadElement& and does all this would make the code tigther and easier to
read.

> Source/WebCore/platform/mac/HIDGamepad.h:80
>  struct HIDGamepadAxis : public HIDGamepadElement {

Normally with struct inheritance we would omit the "public" (it's the default).