Random Gamepad cleanup Suggested as review feedback in https://bugs.webkit.org/show_bug.cgi?id=134386
Created attachment 237025 [details] Patch v1
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).
(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.
http://trac.webkit.org/changeset/172890