Gamepad API: add support for gamepadconnected and gamepaddisconnected events

Source/modules/gamepad/NavigatorGamepad.cpp

 /*
  * Copyright (C) 2011, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
  * ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE
  * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
  * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
  * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
  * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
  * DAMAGE.
  */
 
 #include "config.h"
 #include "modules/gamepad/NavigatorGamepad.h"
 
+#include "core/frame/DOMWindow.h"
+#include "core/frame/LocalFrame.h"
 #include "core/frame/Navigator.h"
+#include "modules/gamepad/GamepadEvent.h"
 #include "modules/gamepad/GamepadList.h"
 #include "modules/gamepad/WebKitGamepadList.h"
 #include "public/platform/Platform.h"
+#include "public/platform/WebGamepadListener.h"
+#include "wtf/HashSet.h"
 #include "wtf/PassOwnPtr.h"
 
 namespace WebCore {
 
+class GlobalGamepadListener : public blink::WebGamepadListener {

[abarth-chromium, 2014/03/19 18:00:33]

Can you put this class in its own file?  For consistency with other modules, we
might want to rename GlobalGamepadListener to GamepadDispatcher

+public:
+    GlobalGamepadListener();
+
+    void addClient(NavigatorGamepad* observer);
+    void removeClient(NavigatorGamepad* observer);
+
+private:
+    virtual void didConnectGamepad(unsigned index, const blink::WebGamepad&) OVERRIDE;
+    virtual void didDisconnectGamepad(unsigned index, const blink::WebGamepad&) OVERRIDE;
+
+    typedef WTF::HashSet<NavigatorGamepad*> ClientSet;
+    ClientSet m_clients;
+};
+
+GlobalGamepadListener::GlobalGamepadListener()
+{
+    blink::Platform::current()->setGamepadListener(this);

[abarth-chromium, 2014/03/19 18:00:33]

If you look at how DeviceOrientationDispatcher, you'll see that we usually use a
pattern that lets us unregister for this notifications when there aren't any
more listeners.  Hopefully that will save battery once no one cares about these
events.

[kbalazs, 2014/03/20 18:57:21]

Good point. After looking at orientation and motion I realized that the
requirements here are similar and it makes sense to reuse the same pattern. So
now NavigatorGamepad is a DeviceSensorEventController and GamepadDispatcher is a
DeviceSensorEventDispatcher. There is some difference in how I implemented the
virtual methods though because with gamepad a script can sample gamepads without
first adding the event listener.

+}
+
+void GlobalGamepadListener::addClient(NavigatorGamepad* observer)
+{
+    m_clients.add(observer);
+}
+
+void GlobalGamepadListener::removeClient(NavigatorGamepad* observer)
+{
+    m_clients.remove(observer);
+}
+
+void GlobalGamepadListener::didConnectGamepad(unsigned index, const blink::WebGamepad& gamepad)
+{
+    for (ClientSet::iterator it = m_clients.begin(), end = m_clients.end(); it != end; ++it)
+        (*it)->didConnectGamepad(index, gamepad);

[abarth-chromium, 2014/03/19 18:00:33]

What stops m_clients from being mutated during this iteration?  We're running
arbitrary script here, which means someone could add or remove a client.  Take a
look at how the DeviceOrientationDispatcher solves this problem.

[kbalazs, 2014/03/20 18:57:21]

Solved by using DeviceSensorDispatcher and DeviceSensorEventController.

+}
+
+void GlobalGamepadListener::didDisconnectGamepad(unsigned index, const blink::WebGamepad& gamepad)
+{
+    for (ClientSet::iterator it = m_clients.begin(), end = m_clients.end(); it != end; ++it)
+        (*it)->didDisconnectGamepad(index, gamepad);
+}
+
+static GlobalGamepadListener& globalListener()
+{
+    DEFINE_STATIC_LOCAL(GlobalGamepadListener, listener, ());
+    return listener;
+}
+
 template<typename T>
 static void sampleGamepad(unsigned index, T& gamepad, const blink::WebGamepad& webGamepad)
 {
     gamepad.setId(webGamepad.id);
     gamepad.setIndex(index);
     gamepad.setConnected(webGamepad.connected);
     gamepad.setTimestamp(webGamepad.timestamp);
     gamepad.setMapping(webGamepad.mapping);
     gamepad.setAxes(webGamepad.axesLength, webGamepad.axes);
     gamepad.setButtons(webGamepad.buttonsLength, webGamepad.buttons);
@@ skipping 13 matching lines @@
             if (!gamepad)
                 gamepad = GamepadType::create();
             sampleGamepad(i, *gamepad, webGamepad);
             into->set(i, gamepad);
         } else {
             into->set(i, nullptr);
         }
     }
 }
 
-NavigatorGamepad::NavigatorGamepad()
+NavigatorGamepad::NavigatorGamepad(LocalFrame* frame)
+    : DOMWindowLifecycleObserver(frame ? frame->domWindow() : 0)
 {
 }
 
 NavigatorGamepad::~NavigatorGamepad()
 {
+    globalListener().removeClient(this);

[abarth-chromium, 2014/03/19 18:00:33]

The problem with only calling removeClient in the destructor is that we'll leak
information to the web page about when the destructor is called.  That's
dependent on garbage collection, which means we'll leak information about when
GC occurs to the web site.  Instead, we should unregister for these
notifications at a defined point in the lifecycle.  Take a look at how device
orientation solves this problem.  My guess is that we're subclassing
DOMWindowProperty and stop listening when the DOMWindow disconnects from the
frame.

[kbalazs, 2014/03/20 18:57:21]

Orientation and motion unregisters when the event listener is removed. But this
doesn't work here because the gamepads can be sampled without event listeners so
I again inherit from DOMWindowProperty as well and unregister in
willDestroyGlobalObjectInFrame and willDetachGlobalObjectInFrame.

 }
 
 const char* NavigatorGamepad::supplementName()
 {
     return "NavigatorGamepad";
 }
 
 NavigatorGamepad& NavigatorGamepad::from(Navigator& navigator)
 {
     NavigatorGamepad* supplement = static_cast<NavigatorGamepad*>(Supplement<Navigator>::from(navigator, supplementName()));
     if (!supplement) {
-        supplement = new NavigatorGamepad();
+        supplement = new NavigatorGamepad(navigator.frame());
         provideTo(navigator, supplementName(), adoptPtr(supplement));
     }
     return *supplement;
 }
 
 WebKitGamepadList* NavigatorGamepad::webkitGetGamepads(Navigator& navigator)
 {
     return NavigatorGamepad::from(navigator).webkitGamepads();
 }
 
@@ skipping 11 matching lines @@
 }
 
 GamepadList* NavigatorGamepad::gamepads()
 {
     if (!m_gamepads)
         m_gamepads = GamepadList::create();
     sampleGamepads<Gamepad>(m_gamepads.get());
     return m_gamepads.get();
 }
 
+void NavigatorGamepad::didConnectGamepad(unsigned index, const blink::WebGamepad& webGamepad)
+{
+    if (index >= blink::WebGamepads::itemsLengthCap || !webGamepad.connected)
+        return;

[abarth-chromium, 2014/03/19 18:00:33]

How could index >= blink::WebGamepads::itemsLengthCap?  That seems like it ought
to be an assert...

[kbalazs, 2014/03/20 18:57:21]

Done.

+
+    if (!m_gamepads)
+        m_gamepads = GamepadList::create();
+
+    RefPtrWillBeRawPtr<Gamepad> gamepad = m_gamepads->item(index);
+    if (!gamepad)
+        gamepad = Gamepad::create();
+    sampleGamepad(index, *gamepad, webGamepad);
+    m_gamepads->set(index, gamepad);
+
+    if (window()) {

[abarth-chromium, 2014/03/19 18:00:33]

Do we want to do the work above if we don't have a window?  It seems like once
we lose the connection to the window, we should stop sampling the gamepad.

[kbalazs, 2014/03/20 18:57:21]

Done.

+        RefPtr<GamepadEvent> event = GamepadEvent::create(EventTypeNames::gamepadconnected, false, true, gamepad.get());
+        window()->dispatchEvent(event);
+    }
+}
+
+void NavigatorGamepad::didDisconnectGamepad(unsigned index, const blink::WebGamepad& webGamepad)
+{
+    if (index >= blink::WebGamepads::itemsLengthCap)
+        return;
+
+    if (!m_gamepads)
+        m_gamepads = GamepadList::create();
+
+    RefPtrWillBeRawPtr<Gamepad> gamepad = m_gamepads->item(index);
+    if (!gamepad)
+        gamepad = Gamepad::create();
+    sampleGamepad(index, *gamepad, webGamepad);
+
+    if (window()) {
+        RefPtr<GamepadEvent> event = GamepadEvent::create(EventTypeNames::gamepaddisconnected, false, true, gamepad.get());
+        window()->dispatchEvent(event);
+    }
+}
+
+void NavigatorGamepad::didAddEventListener(DOMWindow*, const AtomicString& eventType)
+{
+    if (eventType == EventTypeNames::gamepadconnected || eventType == EventTypeNames::gamepaddisconnected)
+        globalListener()
+}
+
 } // namespace WebCore