Bug 1218443: [webext] Fix some instances of window listeners not being added correctly. r=billm
☠☠ backed out by c699b2c839a9 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Sun, 15 Nov 2015 19:34:09 -0800
changeset 273007 6fa3b2df62cc80058ea2f63834dcbdd6ec7caa36
parent 273006 e85794f108f04dd2ee460c6b3e030a148185e81c
child 273008 ba16287438d4f4f6e3a5bdb43e00bda8cde4cfea
push id29692
push usercbook@mozilla.com
push dateWed, 18 Nov 2015 13:48:23 +0000
treeherdermozilla-central@f8cc032951d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1218443
milestone45.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 1218443: [webext] Fix some instances of window listeners not being added correctly. r=billm
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser_ext_pageAction_context.js
browser/components/extensions/test/browser/head.js
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -416,73 +416,86 @@ global.WindowManager = {
 // closed when a "domwindowclosed" notification fires for it.
 global.WindowListManager = {
   _openListeners: new Set(),
   _closeListeners: new Set(),
 
   // Returns an iterator for all browser windows. Unless |includeIncomplete| is
   // true, only fully-loaded windows are returned.
   *browserWindows(includeIncomplete = false) {
-    let e = Services.wm.getEnumerator("navigator:browser");
+    // The window type parameter is only available once the window's document
+    // element has been created. This means that, when looking for incomplete
+    // browser windows, we need to ignore the type entirely for windows which
+    // haven't finished loading, since we would otherwise skip browser windows
+    // in their early loading stages.
+    // This is particularly important given that the "domwindowcreated" event
+    // fires for browser windows when they're in that in-between state, and just
+    // before we register our own "domwindowcreated" listener.
+
+    let e = Services.wm.getEnumerator("");
     while (e.hasMoreElements()) {
       let window = e.getNext();
-      if (includeIncomplete || window.document.readyState == "complete") {
+
+      let ok = includeIncomplete;
+      if (window.document.readyState == "complete") {
+        ok = window.document.documentElement.getAttribute("windowtype") == "navigator:browser";
+      }
+
+      if (ok) {
         yield window;
       }
     }
   },
 
   addOpenListener(listener) {
-    if (this._openListeners.length == 0 && this._closeListeners.length == 0) {
+    if (this._openListeners.size == 0 && this._closeListeners.size == 0) {
       Services.ww.registerNotification(this);
     }
     this._openListeners.add(listener);
 
     for (let window of this.browserWindows(true)) {
       if (window.document.readyState != "complete") {
         window.addEventListener("load", this);
       }
     }
   },
 
   removeOpenListener(listener) {
     this._openListeners.delete(listener);
-    if (this._openListeners.length == 0 && this._closeListeners.length == 0) {
+    if (this._openListeners.size == 0 && this._closeListeners.size == 0) {
       Services.ww.unregisterNotification(this);
     }
   },
 
   addCloseListener(listener) {
-    if (this._openListeners.length == 0 && this._closeListeners.length == 0) {
+    if (this._openListeners.size == 0 && this._closeListeners.size == 0) {
       Services.ww.registerNotification(this);
     }
     this._closeListeners.add(listener);
   },
 
   removeCloseListener(listener) {
     this._closeListeners.delete(listener);
-    if (this._openListeners.length == 0 && this._closeListeners.length == 0) {
+    if (this._openListeners.size == 0 && this._closeListeners.size == 0) {
       Services.ww.unregisterNotification(this);
     }
   },
 
   handleEvent(event) {
     event.currentTarget.removeEventListener(event.type, this);
     let window = event.target.defaultView;
     if (window.document.documentElement.getAttribute("windowtype") != "navigator:browser") {
       return;
     }
 
     for (let listener of this._openListeners) {
       listener(window);
     }
   },
 
-  queryInterface: XPCOMUtils.generateQI([Ci.nsISupports, Ci.nsIObserver]),
-
   observe(window, topic, data) {
     if (topic == "domwindowclosed") {
       if (window.document.documentElement.getAttribute("windowtype") != "navigator:browser") {
         return;
       }
 
       window.removeEventListener("load", this);
       for (let listener of this._closeListeners) {
@@ -563,16 +576,18 @@ global.AllWindowEvents = {
     for (let [eventType, listeners] of AllWindowEvents._listeners) {
       for (let listener of listeners) {
         this.addWindowListener(window, eventType, listener);
       }
     }
   },
 };
 
+AllWindowEvents.openListener = AllWindowEvents.openListener.bind(AllWindowEvents);
+
 // Subclass of EventManager where we just need to call
 // add/removeEventListener on each XUL window.
 global.WindowEventManager = function(context, name, event, listener)
 {
   EventManager.call(this, context, name, fire => {
     let listener2 = (...args) => listener(fire, ...args);
     AllWindowEvents.addListener(event, listener2);
     return () => {
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
@@ -22,19 +22,19 @@ add_task(function* testTabSwitchContext(
         { "icon": browser.runtime.getURL("1.png"),
           "popup": browser.runtime.getURL("default.html"),
           "title": "Default Title" },
         { "icon": browser.runtime.getURL("2.png"),
           "popup": browser.runtime.getURL("2.html"),
           "title": "Title 2" },
       ];
 
-      var tabs = [];
-
-      var tests = [
+      var tabs;
+      var tests;
+      var allTests = [
         expect => {
           browser.test.log("Initial state. No icon visible.");
           expect(null);
         },
         expect => {
           browser.test.log("Show the icon on the first tab, expect default properties.");
           browser.pageAction.show(tabs[0]);
           expect(details[0]);
@@ -108,17 +108,16 @@ add_task(function* testTabSwitchContext(
 
       // Gets the current details of the page action, and returns a
       // promise that resolves to an object containing them.
       function getDetails() {
         return new Promise(resolve => {
           return browser.tabs.query({ active: true, currentWindow: true }, resolve);
         }).then(tabs => {
           var tabId = tabs[0].id;
-
           return Promise.all([
             new Promise(resolve => browser.pageAction.getTitle({tabId}, resolve)),
             new Promise(resolve => browser.pageAction.getPopup({tabId}, resolve))])
         }).then(details => {
           return Promise.resolve({ title: details[0],
                                    popup: details[1] });
         });
       }
@@ -150,60 +149,90 @@ add_task(function* testTabSwitchContext(
             });
           } else {
             finish();
           }
         });
       }
 
       browser.test.onMessage.addListener((msg) => {
-        if (msg != "runNextTest") {
-          browser.test.fail("Expecting 'runNextTest' message");
+        if (msg == "runTests") {
+          runTests();
+        } else if (msg == "runNextTest") {
+          nextTest();
+        } else {
+          browser.test.fail(`Unexpected message: ${msg}`);
         }
-
-        nextTest();
       });
 
-      browser.tabs.query({ active: true, currentWindow: true }, resultTabs => {
-        tabs[0] = resultTabs[0].id;
+      function runTests() {
+        tabs = [];
+        tests = allTests.slice();
 
-        nextTest();
-      });
+        browser.tabs.query({ active: true, currentWindow: true }, resultTabs => {
+          tabs[0] = resultTabs[0].id;
+
+          nextTest();
+        });
+      }
+
+      runTests();
     },
   });
 
   let pageActionId = makeWidgetId(extension.id) + "-page-action";
+  let currentWindow = window;
+  let windows = [];
 
   function checkDetails(details) {
-    let image = document.getElementById(pageActionId);
+    let image = currentWindow.document.getElementById(pageActionId);
     if (details == null) {
       ok(image == null || image.hidden, "image is hidden");
     } else {
       ok(image, "image exists");
 
       is(image.src, details.icon, "icon URL is correct");
       is(image.getAttribute("tooltiptext"), details.title, "image title is correct");
       is(image.getAttribute("aria-label"), details.title, "image aria-label is correct");
       // TODO: Popup URL.
     }
   }
 
+  let testNewWindows = 1;
+
   let awaitFinish = new Promise(resolve => {
     extension.onMessage("nextTest", (expecting, testsRemaining) => {
       checkDetails(expecting);
 
       if (testsRemaining) {
         extension.sendMessage("runNextTest")
+      } else if (testNewWindows) {
+        testNewWindows--;
+
+        BrowserTestUtils.openNewBrowserWindow().then(window => {
+          windows.push(window);
+          currentWindow = window;
+          return focusWindow(window);
+        }).then(() => {
+          extension.sendMessage("runTests")
+        });
       } else {
         resolve();
       }
     });
   });
 
   yield extension.startup();
 
   yield awaitFinish;
 
   yield extension.unload();
 
   let node = document.getElementById(pageActionId);
   is(node, undefined, "pageAction image removed from document");
+
+  for (let win of windows) {
+    node = win.document.getElementById(pageActionId);
+    is(node, undefined, "pageAction image removed from second document");
+
+    yield BrowserTestUtils.closeWindow(win);
+  }
 });
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -1,25 +1,25 @@
 var {CustomizableUI} = Cu.import("resource:///modules/CustomizableUI.jsm");
 
 function makeWidgetId(id)
 {
   id = id.toLowerCase();
   return id.replace(/[^a-z0-9_-]/g, "_");
 }
 
-function* focusWindow(win)
+var focusWindow = Task.async(function* focusWindow(win)
 {
   let fm = Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager);
   if (fm.activeWindow == win) {
     return;
   }
 
   let promise = new Promise(resolve => {
     win.addEventListener("focus", function listener() {
       win.removeEventListener("focus", listener, true);
       resolve();
     }, true);
   });
 
   win.focus();
   yield promise;
-}
+});