From c608331f52eb9f8834ca185079fee44aab4288b5 Mon Sep 17 00:00:00 2001 From: Bob Silverberg Date: Tue, 4 Apr 2017 09:43:50 -0400 Subject: [PATCH] Bug 1342207 - chrome.tabs.onActivated does not fire for new windows, r=kmag MozReview-Commit-ID: D9Nwd9lc57x --HG-- extra : rebase_source : 65ff7814eaddd00acaf0ce88bd96309448a6d003 --- browser/components/extensions/ext-tabs.js | 28 +++++---- browser/components/extensions/ext-utils.js | 25 ++++++++ .../browser/browser_ext_tabs_onHighlighted.js | 57 ++++++++++++++++++- 3 files changed, 99 insertions(+), 11 deletions(-) diff --git a/browser/components/extensions/ext-tabs.js b/browser/components/extensions/ext-tabs.js index 2b91ee454e3b..89c62bf610a9 100644 --- a/browser/components/extensions/ext-tabs.js +++ b/browser/components/extensions/ext-tabs.js @@ -99,11 +99,15 @@ this.tabs = class extends ExtensionAPI { 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 => { @@ -123,11 +127,15 @@ this.tabs = class extends ExtensionAPI { * 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 => { diff --git a/browser/components/extensions/ext-utils.js b/browser/components/extensions/ext-utils.js index ed0a6c8c0282..c35a2df0c7b9 100644 --- a/browser/components/extensions/ext-utils.js +++ b/browser/components/extensions/ext-utils.js @@ -155,6 +155,7 @@ class TabTracker extends TabTrackerBase { windowTracker.addListener("TabClose", this); windowTracker.addListener("TabOpen", this); + windowTracker.addListener("TabSelect", this); windowTracker.addOpenListener(this._handleWindowOpen); windowTracker.addCloseListener(this._handleWindowClose); @@ -262,6 +263,14 @@ class TabTracker extends TabTrackerBase { 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; } } @@ -307,6 +316,9 @@ class TabTracker extends TabTrackerBase { 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]); } } @@ -328,6 +340,19 @@ class TabTracker extends TabTrackerBase { } } + /** + * 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. * diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js b/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js index 9cc2554d6856..6bd76930c4c4 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_onHighlighted.js @@ -25,6 +25,14 @@ add_task(function* testTabEvents() { } }); + 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"); @@ -43,7 +51,10 @@ add_task(function* testTabEvents() { 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}`); @@ -61,23 +72,58 @@ add_task(function* testTabEvents() { * @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}); @@ -107,6 +153,15 @@ add_task(function* testTabEvents() { 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");