Bug 893785 - Don't fire gamepadconnected events for windows that have already received them. r=smaug, a=bajaj
authorTed Mielczarek <ted@mielczarek.org>
Thu, 01 Aug 2013 13:52:27 -0400
changeset 148217 d759cc9de1dff8befd24d97c38ac80ab6cf5da30
parent 148216 7e1add37886ac9099b85dc145bb7524fa442f041
child 148218 cf6c28aac228acb8271e271681f32f8c7d3b2606
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, bajaj
bugs893785
milestone24.0a2
Bug 893785 - Don't fire gamepadconnected events for windows that have already received them. r=smaug, a=bajaj
dom/gamepad/GamepadService.cpp
dom/gamepad/GamepadService.h
dom/tests/mochitest/gamepad/Makefile.in
dom/tests/mochitest/gamepad/gamepad_frame.html
dom/tests/mochitest/gamepad/test_gamepad_connect_events.html
--- a/dom/gamepad/GamepadService.cpp
+++ b/dom/gamepad/GamepadService.cpp
@@ -208,26 +208,30 @@ GamepadService::NewButtonEvent(uint32_t 
     --i;
 
     // Only send events to non-background windows
     if (!listeners[i]->GetOuterWindow() ||
         listeners[i]->GetOuterWindow()->IsBackground()) {
       continue;
     }
 
+    bool first_time = false;
     if (!WindowHasSeenGamepad(listeners[i], aIndex)) {
-      SetWindowHasSeenGamepad(listeners[i], aIndex);
       // This window hasn't seen this gamepad before, so
       // send a connection event first.
-      NewConnectionEvent(aIndex, true);
+      SetWindowHasSeenGamepad(listeners[i], aIndex);
+      first_time = true;
     }
 
     nsRefPtr<Gamepad> gamepad = listeners[i]->GetGamepad(aIndex);
     if (gamepad) {
       gamepad->SetButton(aButton, aPressed, aValue);
+      if (first_time) {
+        FireConnectionEvent(listeners[i], gamepad, true);
+      }
       if (mNonstandardEventsEnabled) {
         // Fire event
         FireButtonEvent(listeners[i], gamepad, aButton, aValue);
       }
     }
   }
 }
 
@@ -268,26 +272,30 @@ GamepadService::NewAxisMoveEvent(uint32_
     --i;
 
     // Only send events to non-background windows
     if (!listeners[i]->GetOuterWindow() ||
         listeners[i]->GetOuterWindow()->IsBackground()) {
       continue;
     }
 
+    bool first_time = false;
     if (!WindowHasSeenGamepad(listeners[i], aIndex)) {
-      SetWindowHasSeenGamepad(listeners[i], aIndex);
       // This window hasn't seen this gamepad before, so
       // send a connection event first.
-      NewConnectionEvent(aIndex, true);
+      SetWindowHasSeenGamepad(listeners[i], aIndex);
+      first_time = true;
     }
 
     nsRefPtr<Gamepad> gamepad = listeners[i]->GetGamepad(aIndex);
     if (gamepad) {
       gamepad->SetAxis(aAxis, aValue);
+      if (first_time) {
+        FireConnectionEvent(listeners[i], gamepad, true);
+      }
       if (mNonstandardEventsEnabled) {
         // Fire event
         FireAxisMoveEvent(listeners[i], gamepad, aAxis, aValue);
       }
     }
   }
 }
 
@@ -323,23 +331,24 @@ GamepadService::NewConnectionEvent(uint3
   nsTArray<nsRefPtr<nsGlobalWindow> > listeners(mListeners);
 
   if (aConnected) {
     for (uint32_t i = listeners.Length(); i > 0 ; ) {
       --i;
 
       // Only send events to non-background windows
       if (!listeners[i]->GetOuterWindow() ||
-          listeners[i]->GetOuterWindow()->IsBackground())
+          listeners[i]->GetOuterWindow()->IsBackground()) {
         continue;
+      }
 
       // We don't fire a connected event here unless the window
       // has seen input from at least one device.
-      if (aConnected && !listeners[i]->HasSeenGamepadInput()) {
-        return;
+      if (!listeners[i]->HasSeenGamepadInput()) {
+        continue;
       }
 
       SetWindowHasSeenGamepad(listeners[i], aIndex);
 
       nsRefPtr<Gamepad> gamepad = listeners[i]->GetGamepad(aIndex);
       if (gamepad) {
         // Fire event
         FireConnectionEvent(listeners[i], gamepad, aConnected);
@@ -355,19 +364,16 @@ GamepadService::NewConnectionEvent(uint3
       // deal with the hassle of syncing the state of removed gamepads.
 
       if (WindowHasSeenGamepad(listeners[i], aIndex)) {
         nsRefPtr<Gamepad> gamepad = listeners[i]->GetGamepad(aIndex);
         if (gamepad) {
           gamepad->SetConnected(false);
           // Fire event
           FireConnectionEvent(listeners[i], gamepad, false);
-        }
-
-        if (gamepad) {
           listeners[i]->RemoveGamepad(aIndex);
         }
       }
     }
   }
 }
 
 void
--- a/dom/gamepad/GamepadService.h
+++ b/dom/gamepad/GamepadService.h
@@ -42,40 +42,58 @@ class GamepadService : public nsIObserve
   void RemoveListener(nsGlobalWindow* aWindow);
 
   // Add a gamepad to the list of known gamepads, and return its index.
   uint32_t AddGamepad(const char* aID, GamepadMappingType aMapping,
                       uint32_t aNumButtons, uint32_t aNumAxes);
   // Remove the gamepad at |aIndex| from the list of known gamepads.
   void RemoveGamepad(uint32_t aIndex);
 
+  // Update the state of |aButton| for the gamepad at |aIndex| for all
+  // windows that are listening and visible, and fire one of
+  // a gamepadbutton{up,down} event at them as well.
   // aPressed is used for digital buttons, aValue is for analog buttons.
   void NewButtonEvent(uint32_t aIndex, uint32_t aButton, bool aPressed,
                       double aValue);
   // When only a digital button is available the value will be synthesized.
   void NewButtonEvent(uint32_t aIndex, uint32_t aButton, bool aPressed);
+
+  // Update the state of |aAxis| for the gamepad at |aIndex| for all
+  // windows that are listening and visible, and fire a gamepadaxismove
+  // event at them as well.
   void NewAxisMoveEvent(uint32_t aIndex, uint32_t aAxis, double aValue);
 
   // Synchronize the state of |aGamepad| to match the gamepad stored at |aIndex|
   void SyncGamepadState(uint32_t aIndex, Gamepad* aGamepad);
 
  protected:
   GamepadService();
   virtual ~GamepadService() {};
   void StartCleanupTimer();
 
+  // Fire a gamepadconnected or gamepaddisconnected event for the gamepad
+  // at |aIndex| to all windows that are listening and have received
+  // gamepad input.
   void NewConnectionEvent(uint32_t aIndex, bool aConnected);
+
+  // Fire a gamepadaxismove event to the window at |aTarget| for |aGamepad|.
   void FireAxisMoveEvent(EventTarget* aTarget,
                          Gamepad* aGamepad,
                          uint32_t axis,
                          double value);
+
+  // Fire one of gamepadbutton{up,down} event at the window at |aTarget| for
+  // |aGamepad|.
   void FireButtonEvent(EventTarget* aTarget,
                        Gamepad* aGamepad,
                        uint32_t aButton,
                        double aValue);
+
+  // Fire one of gamepad{connected,disconnected} event at the window at
+  // |aTarget| for |aGamepad|.
   void FireConnectionEvent(EventTarget* aTarget,
                            Gamepad* aGamepad,
                            bool aConnected);
 
   // true if this feature is enabled in preferences
   bool mEnabled;
   // true if non-standard events are enabled in preferences
   bool mNonstandardEventsEnabled;
--- a/dom/tests/mochitest/gamepad/Makefile.in
+++ b/dom/tests/mochitest/gamepad/Makefile.in
@@ -7,16 +7,17 @@ topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir	= dom/tests/mochitest/gamepad
 
 include $(DEPTH)/config/autoconf.mk
 
 MOCHITEST_FILES = \
   test_gamepad.html \
+  test_gamepad_connect_events.html \
   test_gamepad_frame_state_sync.html \
   gamepad_frame_state.html \
   test_gamepad_hidden_frame.html \
   gamepad_frame.html \
   test_navigator_gamepads.html \
   mock_gamepad.js \
   $(NULL)
 
--- a/dom/tests/mochitest/gamepad/gamepad_frame.html
+++ b/dom/tests/mochitest/gamepad/gamepad_frame.html
@@ -1,16 +1,20 @@
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
 <!DOCTYPE HTML>
 <html>
 <head>
   <title>frame</title>
   <script type="text/javascript">
+    var connectedEvents = 0;
     var buttonPresses = 0;
+    window.addEventListener("gamepadconnected", function() {
+      connectedEvents++;
+});
     window.addEventListener("gamepadbuttondown", function() {
       buttonPresses++;
 });
   </script>
 </head>
 <body>
 </body>
 </html>
copy from dom/tests/mochitest/gamepad/test_gamepad_hidden_frame.html
copy to dom/tests/mochitest/gamepad/test_gamepad_connect_events.html
--- a/dom/tests/mochitest/gamepad/test_gamepad_hidden_frame.html
+++ b/dom/tests/mochitest/gamepad/test_gamepad_connect_events.html
@@ -1,10 +1,13 @@
 <!-- Any copyright is dedicated to the Public Domain.
    - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<!-- bug 893785 - Test that sending a gamepadconnected event to a new window
+     doesn't result in a gamepadconnected event being sent to existing
+     windows that have already received it. -->
 <!DOCTYPE HTML>
 <html>
 <head>
   <title>Test hidden frames</title>
   <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
 <body>
@@ -16,57 +19,49 @@ var index = GamepadService.addGamepad("t
                                       4, // buttons
                                       2);// axes
 
 function pressButton() {
   GamepadService.newButtonEvent(index, 0, true);
   GamepadService.newButtonEvent(index, 0, false);
 }
 
-function setFrameVisible(f, visible) {
-  var Ci = SpecialPowers.Ci;
-  var docshell = SpecialPowers.wrap(f.contentWindow).QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation).QueryInterface(Ci.nsIDocShell);
-  docshell.isActive = visible;
-}
-
-var frames_loaded = 0;
 var f1, f2;
 function frame_loaded() {
-  frames_loaded++;
- if (frames_loaded == 2) {
-    f1 = document.getElementById('f1');
-    f2 = document.getElementById('f2');
-    pressButton();
-  }
+  f1 = document.getElementById('f1');
+  pressButton();
 }
 
 window.addEventListener("gamepadbuttondown", function() {
   // Wait to ensure that all frames received the button press as well.
- SpecialPowers.executeSoon(tests[testNum++]);
+  SpecialPowers.executeSoon(tests[testNum++]);
 });
 
 var testNum = 0;
 var tests = [
   test1,
   test2,
 ];
 
 function test1() {
-  is(f1.contentWindow.buttonPresses, 1, "right number of button presses in frame 1");
-  is(f2.contentWindow.buttonPresses, 1, "right number of button presses in frame 2");
+  is(f1.contentWindow.connectedEvents, 1, "right number of connection events in frame 1");
 
-  // Now hide the second frame and send another button press.
-  setFrameVisible(f2, false);
-  SpecialPowers.executeSoon(function() { pressButton(); });
+  // Now add another frame.
+  f2 = document.createElement("iframe");
+  f2.addEventListener("load", function() {
+    // When the frame is loaded, press a button again.
+    pressButton();
+  });
+  f2.src = "gamepad_frame.html";
+  document.body.appendChild(f2);
 }
 
 function test2() {
-  is(f1.contentWindow.buttonPresses, 2, "right number of button presses in frame 1");
-  is(f2.contentWindow.buttonPresses, 1, "right number of button presses in frame 2");
+  is(f1.contentWindow.connectedEvents, 1, "right number of connection events in frame 1");
+  is(f2.contentWindow.connectedEvents, 1, "right number of connection events in frame 2");
   GamepadService.removeGamepad(index);
   SimpleTest.finish();
 }
 
 </script>
 <iframe id="f1" src="gamepad_frame.html" onload="frame_loaded()"></iframe>
-<iframe id="f2" src="gamepad_frame.html" onload="frame_loaded()"></iframe>
 </body>
 </html>