Bug 1262976 - browser.windows.onFocusChanged sometimes fires twice, even after the event listener is removed, r?kmag draft
authorBob Silverberg <bsilverberg@mozilla.com>
Fri, 08 Apr 2016 09:46:45 -0400
changeset 352878 e42cdd4e7153e8ec96aee2c5d31a2e76c95ce280
parent 352654 4bc053de842538e99e56927b3c03fdc539374a16
child 518775 ec8072259b06b79afc68eb46c302164323649604
push id15832
push userbmo:bob.silverberg@gmail.com
push dateMon, 18 Apr 2016 23:35:41 +0000
reviewerskmag
bugs1262976
milestone48.0a1
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();
 });