Bug 1262976 - browser.windows.onFocusChanged sometimes fires twice, even after the event listener is removed, r=kmag
authorBob Silverberg <bsilverberg@mozilla.com>
Fri, 08 Apr 2016 09:46:45 -0400
changeset 331664 7e649f9c72b0dd167b2e11578af012e1261e1232
parent 331663 78d531b24e536bd9a76b38ce2bb9882182d35d5f
child 331665 c3262228d62f8eb0545e28cb2ff3953ed8c28713
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1262976
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1262976 - browser.windows.onFocusChanged sometimes fires twice, even after the event listener is removed, r=kmag MozReview-Commit-ID: 4wfFjPBn7zJ
browser/components/extensions/ext-utils.js
browser/components/extensions/ext-windows.js
browser/components/extensions/test/browser/browser_ext_windows_events.js
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -902,49 +902,54 @@ global.AllWindowEvents = {
     list.add(listener);
 
     // Register listener on all existing windows.
     for (let window of WindowListManager.browserWindows()) {
       this.addWindowListener(window, type, listener);
     }
   },
 
-  removeListener(type, listener) {
-    if (type == "domwindowopened") {
+  removeListener(eventType, listener) {
+    if (eventType == "domwindowopened") {
       return WindowListManager.removeOpenListener(listener);
-    } else if (type == "domwindowclosed") {
+    } else if (eventType == "domwindowclosed") {
       return WindowListManager.removeCloseListener(listener);
     }
 
-    let listeners = this._listeners.get(type);
+    let listeners = this._listeners.get(eventType);
     listeners.delete(listener);
     if (listeners.size == 0) {
-      this._listeners.delete(type);
+      this._listeners.delete(eventType);
       if (this._listeners.size == 0) {
         WindowListManager.removeOpenListener(this.openListener);
       }
     }
 
     // Unregister listener from all existing windows.
+    let useCapture = eventType === "focus" || eventType === "blur";
     for (let window of WindowListManager.browserWindows()) {
-      if (type == "progress") {
+      if (eventType == "progress") {
         window.gBrowser.removeTabsProgressListener(listener);
       } else {
-        window.removeEventListener(type, listener);
+        window.removeEventListener(eventType, listener, useCapture);
       }
     }
   },
 
+  /* eslint-disable mozilla/balanced-listeners */
   addWindowListener(window, eventType, listener) {
+    let useCapture = eventType === "focus" || eventType === "blur";
+
     if (eventType == "progress") {
       window.gBrowser.addTabsProgressListener(listener);
     } else {
-      window.addEventListener(eventType, listener);
+      window.addEventListener(eventType, listener, useCapture);
     }
   },
+  /* eslint-enable mozilla/balanced-listeners */
 
   // Runs whenever the "load" event fires for a new window.
   openListener(window) {
     for (let [eventType, listeners] of AllWindowEvents._listeners) {
       for (let listener of listeners) {
         this.addWindowListener(window, eventType, listener);
       }
     }
--- a/browser/components/extensions/ext-windows.js
+++ b/browser/components/extensions/ext-windows.js
@@ -24,21 +24,26 @@ extensions.registerSchemaAPI("windows", 
       }).api(),
 
       onRemoved:
       new WindowEventManager(context, "windows.onRemoved", "domwindowclosed", (fire, window) => {
         fire(WindowManager.getId(window));
       }).api(),
 
       onFocusChanged: new EventManager(context, "windows.onFocusChanged", fire => {
-        // FIXME: This will send multiple messages for a single focus change.
+        // Keep track of the last windowId used to fire an onFocusChanged event
+        let lastOnFocusChangedWindowId;
+
         let listener = event => {
           let window = WindowManager.topWindow;
           let windowId = window ? WindowManager.getId(window) : WindowManager.WINDOW_ID_NONE;
-          fire(windowId);
+          if (windowId !== lastOnFocusChangedWindowId) {
+            fire(windowId);
+            lastOnFocusChangedWindowId = windowId;
+          }
         };
         AllWindowEvents.addListener("focus", listener);
         AllWindowEvents.addListener("blur", listener);
         return () => {
           AllWindowEvents.removeListener("focus", listener);
           AllWindowEvents.removeListener("blur", listener);
         };
       }).api(),
--- a/browser/components/extensions/test/browser/browser_ext_windows_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_windows_events.js
@@ -1,39 +1,70 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 add_task(function* testWindowsEvents() {
   function background() {
     browser.windows.onCreated.addListener(function listener(window) {
-      browser.windows.onCreated.removeListener(listener);
       browser.test.assertTrue(Number.isInteger(window.id),
                               "Window object's id is an integer");
       browser.test.assertEq("normal", window.type,
                             "Window object returned with the correct type");
       browser.test.sendMessage("window-created", window.id);
     });
 
+    let lastWindowId;
+    browser.windows.onFocusChanged.addListener(function listener(windowId) {
+      browser.test.assertTrue(lastWindowId !== windowId,
+                              "onFocusChanged fired once for the given window");
+      lastWindowId = windowId;
+      browser.test.assertTrue(Number.isInteger(windowId),
+                              "windowId is an integer");
+      browser.windows.getLastFocused().then(window => {
+        browser.test.assertEq(windowId, window.id,
+                              "Last focused window has the correct id");
+        browser.test.sendMessage(`window-focus-changed-${windowId}`);
+      });
+    });
+
     browser.windows.onRemoved.addListener(function listener(windowId) {
-      browser.windows.onRemoved.removeListener(listener);
       browser.test.assertTrue(Number.isInteger(windowId),
                               "windowId is an integer");
       browser.test.sendMessage(`window-removed-${windowId}`);
       browser.test.notifyPass("windows.events");
     });
 
     browser.test.sendMessage("ready");
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     background: `(${background})()`,
   });
 
   yield extension.startup();
   yield extension.awaitMessage("ready");
+
+  let {WindowManager} = Cu.import("resource://gre/modules/Extension.jsm", {});
+  let currentWindow = window;
+  let currentWindowId = WindowManager.getId(currentWindow);
+
   let win1 = yield BrowserTestUtils.openNewBrowserWindow();
-  let windowId = yield extension.awaitMessage("window-created");
+  let win1Id = yield extension.awaitMessage("window-created");
+  yield extension.awaitMessage(`window-focus-changed-${win1Id}`);
+
+  let win2 = yield BrowserTestUtils.openNewBrowserWindow();
+  let win2Id = yield extension.awaitMessage("window-created");
+  yield extension.awaitMessage(`window-focus-changed-${win2Id}`);
+
+  yield focusWindow(win1);
+  yield extension.awaitMessage(`window-focus-changed-${win1Id}`);
+
+  yield BrowserTestUtils.closeWindow(win2);
+  yield extension.awaitMessage(`window-removed-${win2Id}`);
+
   yield BrowserTestUtils.closeWindow(win1);
-  yield extension.awaitMessage(`window-removed-${windowId}`);
+  yield extension.awaitMessage(`window-removed-${win1Id}`);
+
+  yield extension.awaitMessage(`window-focus-changed-${currentWindowId}`);
   yield extension.awaitFinish("windows.events");
   yield extension.unload();
 });