Bug 1342207 - chrome.tabs.onActivated does not fire for new windows, r?kmag draft
authorBob Silverberg <bsilverberg@mozilla.com>
Tue, 04 Apr 2017 09:43:50 -0400
changeset 562209 a1564697592a97dae04a674eeae722b73b766acf
parent 561911 819a666afddc804b6099ee1b3cff3a0fdf35ec15
child 624207 60143cdbb366fb958fb571321f5e507171dfe68e
push id53992
push userbmo:bob.silverberg@gmail.com
push dateThu, 13 Apr 2017 17:20:53 +0000
reviewerskmag
bugs1342207
milestone55.0a1
Bug 1342207 - chrome.tabs.onActivated does not fire for new windows, r?kmag MozReview-Commit-ID: D9Nwd9lc57x
browser/components/extensions/ext-tabs.js
browser/components/extensions/ext-utils.js
browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -98,21 +98,25 @@ this.tabs = class extends ExtensionAPI {
 
       await tabListener.awaitTabReady(tab.nativeTab);
 
       return tab;
     }
 
     let self = {
       tabs: {
-        onActivated: new WindowEventManager(context, "tabs.onActivated", "TabSelect", (fire, event) => {
-          let nativeTab = event.originalTarget;
-          let tabId = tabTracker.getId(nativeTab);
-          let windowId = windowTracker.getId(nativeTab.ownerGlobal);
-          fire.async({tabId, windowId});
+        onActivated: new SingletonEventManager(context, "tabs.onActivated", fire => {
+          let listener = (eventName, event) => {
+            fire.async(event);
+          };
+
+          tabTracker.on("tab-activated", listener);
+          return () => {
+            tabTracker.off("tab-activated", listener);
+          };
         }).api(),
 
         onCreated: new SingletonEventManager(context, "tabs.onCreated", fire => {
           let listener = (eventName, event) => {
             fire.async(tabManager.convert(event.nativeTab));
           };
 
           tabTracker.on("tab-created", listener);
@@ -122,21 +126,25 @@ this.tabs = class extends ExtensionAPI {
         }).api(),
 
         /**
          * Since multiple tabs currently can't be highlighted, onHighlighted
          * essentially acts an alias for self.tabs.onActivated but returns
          * the tabId in an array to match the API.
          * @see  https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/Tabs/onHighlighted
         */
-        onHighlighted: new WindowEventManager(context, "tabs.onHighlighted", "TabSelect", (fire, event) => {
-          let nativeTab = event.originalTarget;
-          let tabIds = [tabTracker.getId(nativeTab)];
-          let windowId = windowTracker.getId(nativeTab.ownerGlobal);
-          fire.async({tabIds, windowId});
+        onHighlighted: new SingletonEventManager(context, "tabs.onHighlighted", fire => {
+          let listener = (eventName, event) => {
+            fire.async({tabIds: [event.tabId], windowId: event.windowId});
+          };
+
+          tabTracker.on("tab-activated", listener);
+          return () => {
+            tabTracker.off("tab-activated", listener);
+          };
         }).api(),
 
         onAttached: new SingletonEventManager(context, "tabs.onAttached", fire => {
           let listener = (eventName, event) => {
             fire.async(event.tabId, {newWindowId: event.newWindowId, newPosition: event.newPosition});
           };
 
           tabTracker.on("tab-attached", listener);
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -151,16 +151,17 @@ class TabTracker extends TabTrackerBase 
 
     this.adoptedTabs = new WeakMap();
 
     this._handleWindowOpen = this._handleWindowOpen.bind(this);
     this._handleWindowClose = this._handleWindowClose.bind(this);
 
     windowTracker.addListener("TabClose", this);
     windowTracker.addListener("TabOpen", this);
+    windowTracker.addListener("TabSelect", this);
     windowTracker.addOpenListener(this._handleWindowOpen);
     windowTracker.addCloseListener(this._handleWindowClose);
 
     /* eslint-disable mozilla/balanced-listeners */
     this.on("tab-detached", this._handleTabDestroyed);
     this.on("tab-removed", this._handleTabDestroyed);
     /* eslint-enable mozilla/balanced-listeners */
   }
@@ -258,16 +259,24 @@ class TabTracker extends TabTrackerBase 
           // opened.
           this.setId(adoptedBy, this.getId(nativeTab));
 
           this.emitDetached(nativeTab, adoptedBy);
         } else {
           this.emitRemoved(nativeTab, false);
         }
         break;
+
+      case "TabSelect":
+        // Because we are delaying calling emitCreated above, we also need to
+        // delay sending this event because it shouldn't fire before onCreated.
+        Promise.resolve().then(() => {
+          this.emitActivated(nativeTab);
+        });
+        break;
     }
   }
 
   /**
    * A private method which is called whenever a new browser window is opened,
    * and dispatches the necessary events for it.
    *
    * @param {DOMWindow} window
@@ -303,16 +312,19 @@ class TabTracker extends TabTrackerBase 
         }
       };
 
       this.on("tab-detached", listener);
     } else {
       for (let nativeTab of window.gBrowser.tabs) {
         this.emitCreated(nativeTab);
       }
+
+      // emitActivated to trigger tab.onActivated/tab.onHighlighted for a newly opened window.
+      this.emitActivated(window.gBrowser.tabs[0]);
     }
   }
 
   /**
    * A private method which is called whenever a browser window is closed,
    * and dispatches the necessary events for it.
    *
    * @param {DOMWindow} window
@@ -325,16 +337,29 @@ class TabTracker extends TabTrackerBase 
         this.emitDetached(nativeTab, this.adoptedTabs.get(nativeTab));
       } else {
         this.emitRemoved(nativeTab, true);
       }
     }
   }
 
   /**
+   * Emits a "tab-activated" event for the given tab element.
+   *
+   * @param {NativeTab} nativeTab
+   *        The tab element which has been activated.
+   * @private
+   */
+  emitActivated(nativeTab) {
+    this.emit("tab-activated", {
+      tabId: this.getId(nativeTab),
+      windowId: windowTracker.getId(nativeTab.ownerGlobal)});
+  }
+
+  /**
    * Emits a "tab-attached" event for the given tab element.
    *
    * @param {NativeTab} nativeTab
    *        The tab element in the window to which the tab is being attached.
    * @private
    */
   emitAttached(nativeTab) {
     let newWindowId = windowTracker.getId(nativeTab.ownerGlobal);
--- a/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js
@@ -20,16 +20,24 @@ add_task(function* testTabEvents() {
     browser.tabs.onActivated.addListener((info) => {
       if (info.tabId in events) {
         events[info.tabId].push("onActivated");
       } else {
         events[info.tabId] = ["onActivated"];
       }
     });
 
+    browser.tabs.onCreated.addListener((info) => {
+      if (info.id in events) {
+        events[info.id].push("onCreated");
+      } else {
+        events[info.id] = ["onCreated"];
+      }
+    });
+
     browser.tabs.onHighlighted.addListener((info) => {
       if (info.tabIds[0] in events) {
         events[info.tabIds[0]].push("onHighlighted");
       } else {
         events[info.tabIds[0]] = ["onHighlighted"];
       }
     });
 
@@ -38,17 +46,20 @@ add_task(function* testTabEvents() {
      * The events associated to the specified tab are removed after this check is made.
      *
      * @param {number} tabId
      * @param {Array<string>} expectedEvents
      */
     async function expectEvents(tabId, expectedEvents) {
       browser.test.log(`Expecting events: ${expectedEvents.join(", ")}`);
 
-      await new Promise(resolve => setTimeout(resolve, 0));
+      // Wait up to 5000 ms for the expected number of events.
+      for (let i = 0; i < 50 && (!events[tabId] || events[tabId].length < expectedEvents.length); i++) {
+        await new Promise(resolve => setTimeout(resolve, 100));
+      }
 
       browser.test.assertEq(expectedEvents.length, events[tabId].length,
                             `Got expected number of events for ${tabId}`);
 
       for (let [i, name] of expectedEvents.entries()) {
         browser.test.assertEq(name, i in events[tabId] && events[tabId][i],
                               `Got expected ${name} event`);
       }
@@ -56,33 +67,68 @@ add_task(function* testTabEvents() {
     }
 
     /**
      * Opens a new tab and asserts that the correct events are fired.
      *
      * @param {number} windowId
      */
     async function openTab(windowId) {
+      browser.test.assertEq(0, Object.keys(events).length,
+                           "No events remaining before testing openTab.");
+
       let tab = await browser.tabs.create({windowId});
 
       tabIds.push(tab.id);
       browser.test.log(`Opened tab ${tab.id}`);
 
       await expectEvents(tab.id, [
+        "onCreated",
         "onActivated",
         "onHighlighted",
       ]);
     }
 
     /**
+     * Opens a new window and asserts that the correct events are fired.
+     *
+     * @param {Array} urls A list of urls for which to open tabs in the new window.
+     */
+    async function openWindow(urls) {
+      browser.test.assertEq(0, Object.keys(events).length,
+                           "No events remaining before testing openWindow.");
+
+      let window = await browser.windows.create({url: urls});
+      browser.test.log(`Opened new window ${window.id}`);
+
+      for (let [i] of urls.entries()) {
+        let tab = window.tabs[i];
+        tabIds.push(tab.id);
+
+        let expectedEvents = [
+            "onCreated",
+            "onActivated",
+            "onHighlighted",
+        ];
+        if (i !== 0) {
+          expectedEvents.splice(1);
+        }
+        await expectEvents(window.tabs[i].id, expectedEvents);
+      }
+    }
+
+    /**
      * Highlights an existing tab and asserts that the correct events are fired.
      *
      * @param {number} tabId
      */
     async function highlightTab(tabId) {
+      browser.test.assertEq(0, Object.keys(events).length,
+                           "No events remaining before testing highlightTab.");
+
       browser.test.log(`Highlighting tab ${tabId}`);
       let tab = await browser.tabs.update(tabId, {active: true});
 
       browser.test.assertEq(tab.id, tabId, `Tab ${tab.id} highlighted`);
 
       await expectEvents(tab.id, [
         "onActivated",
         "onHighlighted",
@@ -102,16 +148,25 @@ add_task(function* testTabEvents() {
     ]);
 
     await Promise.all([
       highlightTab(tabIds[0]),
       highlightTab(tabIds[1]),
       highlightTab(tabIds[2]),
     ]);
 
+    await Promise.all([
+      openWindow(["http://example.com"]),
+      openWindow(["http://example.com", "http://example.org"]),
+      openWindow(["http://example.com", "http://example.org", "http://example.net"]),
+    ]);
+
+    browser.test.assertEq(0, Object.keys(events).length,
+                          "No events remaining after tests.");
+
     await Promise.all(tabIds.map(id => browser.tabs.remove(id)));
 
     browser.test.notifyPass("tabs.highlight");
   }
 
   let extension = ExtensionTestUtils.loadExtension({
     manifest: {
       "permissions": ["tabs"],