Bug 136193 – Random Gamepad cleanup

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Random Gamepad cleanup

Suggested as review feedback in https://bugs.webkit.org/show_bug.cgi?id=134386

[Brady Eidson]

Created attachment 237025 [details]
Patch v1

[Sam Weinig]

Comment on attachment 237025 [details]
Patch v1

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

> Source/WebCore/Modules/gamepad/NavigatorGamepad.cpp:79
> +    if (index >= m_gamepads.size() || !m_gamepads[index])
> +        return adoptRef(*Gamepad::create(platformGamepad).leakRef());

This looks suspect. Why can't you just return what Gamepad::create() returns (you may have to chance Gamepad::create()  to return a PassRef<> I guess)?

> Source/WebCore/page/DOMWindow.h:451
> +#if ENABLE(GAMEPAD)
> +        WeakPtr<DOMWindow> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
> +#endif

While Gamepad may be the only one using this right now, I don't think it makes sense to put this under #if ENABLE(GAMEPAD).

[Brady Eidson]

(In reply to comment #2)
> (From update of attachment 237025 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237025&action=review
> 
> > Source/WebCore/Modules/gamepad/NavigatorGamepad.cpp:79
> > +    if (index >= m_gamepads.size() || !m_gamepads[index])
> > +        return adoptRef(*Gamepad::create(platformGamepad).leakRef());
> 
> This looks suspect. Why can't you just return what Gamepad::create() returns (you may have to chance Gamepad::create()  to return a PassRef<> I guess)?

Right, and since we have to maintain a sparse Vector of Gamepads and we can't have null Ref<>s in that Vector, I think it makes more sense to keep Gamepad::create() returning PassRefPtr<> and play this trick here.

> 
> > Source/WebCore/page/DOMWindow.h:451
> > +#if ENABLE(GAMEPAD)
> > +        WeakPtr<DOMWindow> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
> > +#endif
> 
> While Gamepad may be the only one using this right now, I don't think it makes sense to put this under #if ENABLE(GAMEPAD).

Sure thing.

[Brady Eidson]

http://trac.webkit.org/changeset/172890