[webkit-reviews] review granted: [Bug 134670] PlatformGamepad should have an inherent index : [Attachment 234470] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jul 7 10:35:58 PDT 2014


Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 134670: PlatformGamepad should have an inherent index
https://bugs.webkit.org/show_bug.cgi?id=134670

Attachment 234470: Patch v1
https://bugs.webkit.org/attachment.cgi?id=234470&action=review

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


> Source/WebCore/Modules/gamepad/Gamepad.h:60
> +    Gamepad(const PlatformGamepad&);

Should mark this explicit.

> Source/WebCore/Modules/gamepad/GamepadManager.cpp:86
>	   unsigned size = platformGamepads.size();
>	   for (unsigned i = 0; i < size; ++i) {
>	       if (platformGamepads[i])
> -		   navigator->gamepadConnected(i);
> +		   navigator->gamepadConnected(*platformGamepads[i]);
>	   }

Should write this as a modern for loop:

    for (auto& gamepad : GamepadProvider::shared().platformGamepads()) {
	if (gamepad)
	    navigator->gamepadConnected(*gamepad);
    }

> Source/WebCore/platform/PlatformGamepad.h:48
> +    PlatformGamepad(unsigned index)

Should mark this explicit.

> Source/WebCore/platform/mac/HIDGamepadProvider.cpp:240
> +std::unique_ptr<HIDGamepad> 
HIDGamepadProvider::removeGamepadForDevice(IOHIDDeviceRef device)

Extra space here after the ">" character.

> Source/WebCore/platform/mac/HIDGamepadProvider.cpp:249
>      for (unsigned i = 0; i < m_gamepadVector.size(); ++i) {
> -	   if (m_gamepadVector[i] == result.first.get()) {
> -	       result.second = i;
> +	   if (m_gamepadVector[i] == result.get()) {
>	       m_gamepadVector[i] = nullptr;
>	       break;
>	   }

Could write this with Vector::find:

    auto i = m_gamepadVector.find(result.get());
    if (i != notFound)
	m_gamepadVector[i] = nullptr;


More information about the webkit-reviews mailing list