diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index fdf2b550fbdb1..10aeee4927414 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -2598,6 +2598,9 @@ (relatedToCurrent && this.selectedTab); var t = document.createXULElement("tab", { is: "tabbrowser-tab" }); + // Tag the tab as being created so extension code can ignore events + // prior to TabOpen. + t.initializingTab = true; t.openerTab = openerTab; aURI = aURI || "about:blank"; @@ -2861,6 +2864,7 @@ // Dispatch a new tab notification. We do this once we're // entirely done, so that things are in a consistent state // even if the event listener opens or closes tabs. + delete t.initializingTab; let evt = new CustomEvent("TabOpen", { bubbles: true, detail: eventDetail || {}, diff --git a/browser/components/extensions/parent/ext-browser.js b/browser/components/extensions/parent/ext-browser.js index 2f1eae2ea2074..b988bbdb917b7 100644 --- a/browser/components/extensions/parent/ext-browser.js +++ b/browser/components/extensions/parent/ext-browser.js @@ -20,6 +20,11 @@ ChromeUtils.defineModuleGetter( "BrowserWindowTracker", "resource:///modules/BrowserWindowTracker.jsm" ); +ChromeUtils.defineModuleGetter( + this, + "PromiseUtils", + "resource://gre/modules/PromiseUtils.jsm" +); var { ExtensionError } = ExtensionUtils; @@ -324,6 +329,7 @@ class TabTracker extends TabTrackerBase { this._browsers = new WeakMap(); this._tabIds = new Map(); this._nextId = 1; + this._deferredTabOpenEvents = new WeakMap(); this._handleTabDestroyed = this._handleTabDestroyed.bind(this); } @@ -489,6 +495,23 @@ class TabTracker extends TabTrackerBase { tab.openerTab = openerTab; } + deferredForTabOpen(nativeTab) { + let deferred = this._deferredTabOpenEvents.get(nativeTab); + if (!deferred) { + deferred = PromiseUtils.defer(); + this._deferredTabOpenEvents.set(nativeTab, deferred); + deferred.promise.then(() => { + this._deferredTabOpenEvents.delete(nativeTab); + }); + } + return deferred; + } + + async maybeWaitForTabOpen(nativeTab) { + let deferred = this._deferredTabOpenEvents.get(nativeTab); + return deferred && deferred.promise; + } + /** * @param {Event} event * The DOM Event to handle. @@ -526,7 +549,9 @@ class TabTracker extends TabTrackerBase { // We need to delay sending this event until the next tick, since the // tab can become selected immediately after "TabOpen", then onCreated // should be fired with `active: true`. + let deferred = this.deferredForTabOpen(event.originalTarget); Promise.resolve().then(() => { + deferred.resolve(); if (!event.originalTarget.parentNode) { // If the tab is already be destroyed, do nothing. return; @@ -552,7 +577,7 @@ class TabTracker extends TabTrackerBase { 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.maybeWaitForTabOpen(nativeTab).then(() => { if (!nativeTab.parentNode) { // If the tab is already be destroyed, do nothing. return; @@ -565,6 +590,7 @@ class TabTracker extends TabTrackerBase { if (this.has("tabs-highlighted")) { // Because we are delaying calling emitCreated above, we also need to // delay sending this event because it shouldn't fire before onCreated. + // event.target is gBrowser, so we don't use maybeWaitForTabOpen. Promise.resolve().then(() => { this.emitHighlighted(event.target.ownerGlobal); }); diff --git a/browser/components/extensions/parent/ext-tabs.js b/browser/components/extensions/parent/ext-tabs.js index d8ee3d79391fb..97299983beb90 100644 --- a/browser/components/extensions/parent/ext-tabs.js +++ b/browser/components/extensions/parent/ext-tabs.js @@ -262,7 +262,7 @@ class TabsUpdateFilterEventManager extends EventManager { return true; } - let fireForTab = (tab, changed) => { + let fireForTab = (tab, changed, nativeTab) => { // Tab may be null if private and not_allowed. if (!tab || !matchFilters(tab, changed)) { return; @@ -270,11 +270,21 @@ class TabsUpdateFilterEventManager extends EventManager { let changeInfo = sanitize(extension, changed); if (changeInfo) { - fire.async(tab.id, changeInfo, tab.convert()); + tabTracker.maybeWaitForTabOpen(nativeTab).then(() => { + if (!nativeTab.parentNode) { + // If the tab is already be destroyed, do nothing. + return; + } + fire.async(tab.id, changeInfo, tab.convert()); + }); } }; let listener = event => { + // Ignore any events prior to TabOpen + if (event.originalTarget.initializingTab) { + return; + } if (!context.canAccessWindow(event.originalTarget.ownerGlobal)) { return; } @@ -337,7 +347,7 @@ class TabsUpdateFilterEventManager extends EventManager { changeInfo[prop] = tab[prop]; } - fireForTab(tab, changeInfo); + fireForTab(tab, changeInfo, event.originalTarget); }; let statusListener = ({ browser, status, url }) => { @@ -353,7 +363,7 @@ class TabsUpdateFilterEventManager extends EventManager { changed.url = url; } - fireForTab(tabManager.wrapTab(tabElem), changed); + fireForTab(tabManager.wrapTab(tabElem), changed, tabElem); } }; @@ -363,7 +373,7 @@ class TabsUpdateFilterEventManager extends EventManager { if (nativeTab && context.canAccessWindow(nativeTab.ownerGlobal)) { let tab = tabManager.getWrapper(nativeTab); - fireForTab(tab, { isArticle: message.data.isArticle }); + fireForTab(tab, { isArticle: message.data.isArticle }, nativeTab); } }; diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_events_order.js b/browser/components/extensions/test/browser/browser_ext_tabs_events_order.js index 955ee06b3b58d..ab998109de514 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_events_order.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_events_order.js @@ -68,13 +68,21 @@ add_task(async function testTabEvents() { `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], + for (let name of expectedEvents) { + browser.test.assertTrue( + events[tabId].includes(name), `Got expected ${name} event` ); } + + if (expectedEvents.includes("onCreated")) { + browser.test.assertEq( + events[tabId].indexOf("onCreated"), + 0, + "onCreated happened first" + ); + } + delete events[tabId]; } diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_onCreated.js b/browser/components/extensions/test/browser/browser_ext_tabs_onCreated.js index 58d9a86975c84..4f39e560e2d24 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_onCreated.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_onCreated.js @@ -8,19 +8,34 @@ add_task(async function test_onCreated_active() { manifest: { permissions: ["tabs"], }, - background: function() { + async background() { + let created = false; + let tabIds = (await browser.tabs.query({})).map(t => t.id); browser.tabs.onCreated.addListener(tab => { + created = tab.id; browser.tabs.remove(tab.id); browser.test.sendMessage("onCreated", tab); }); + // We always get at least one onUpdated event when creating tabs. + browser.tabs.onUpdated.addListener((tabId, changes, tab) => { + // ignore tabs created prior to extension startup + if (tabIds.includes(tabId)) { + return; + } + browser.test.assertTrue(created, tabId, "tab created before updated"); + browser.test.notifyPass("onUpdated"); + }); + browser.test.sendMessage("ready"); }, }); await extension.startup(); + await extension.awaitMessage("ready"); BrowserOpenTab(); let tab = await extension.awaitMessage("onCreated"); is(true, tab.active, "Tab should be active"); + await extension.awaitFinish("onUpdated"); await extension.unload(); }); diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js b/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js index 77a31c88224d0..f898ec2d90864 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated.js @@ -250,4 +250,44 @@ add_task(async function test_without_tabs_permission() { }, false /* withPermissions */); }); +add_task(async function test_onUpdated_after_onRemoved() { + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + permissions: ["tabs"], + }, + async background() { + const url = + "http://mochi.test:8888/browser/browser/components/extensions/test/browser/context_tabs_onUpdated_page.html"; + let removed = false; + let tab; + + // If remove happens fast and we never receive onUpdated, that is ok, but + // we never want to receive onUpdated after onRemoved. + browser.tabs.onUpdated.addListener(function onUpdated(tabId, changeInfo) { + if (!tab || tab.id !== tabId) { + return; + } + browser.test.assertFalse( + removed, + "tab has not been removed before onUpdated" + ); + }); + + browser.tabs.onRemoved.addListener((tabId, removedInfo) => { + if (!tab || tab.id !== tabId) { + return; + } + removed = true; + browser.test.notifyPass("onRemoved"); + }); + + tab = await browser.tabs.create({ url }); + browser.tabs.remove(tab.id); + }, + }); + await extension.startup(); + await extension.awaitFinish("onRemoved"); + await extension.unload(); +}); + add_task(forceGC); diff --git a/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated_filter.js b/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated_filter.js index 6f59d1f53cdab..6efea9bf54db0 100644 --- a/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated_filter.js +++ b/browser/components/extensions/test/browser/browser_ext_tabs_onUpdated_filter.js @@ -288,7 +288,7 @@ add_task(async function test_filter_property() { manifest: { permissions: ["tabs"], }, - background() { + async background() { // We expect only status updates, anything else is a failure. let properties = new Set([ "audible", @@ -301,8 +301,22 @@ add_task(async function test_filter_property() { "sharingState", "title", ]); + + // Test that updated only happens after created. + let created = false; + let tabIds = (await browser.tabs.query({})).map(t => t.id); + browser.tabs.onCreated.addListener(tab => { + created = tab.id; + }); + browser.tabs.onUpdated.addListener( (tabId, changeInfo) => { + // ignore tabs created prior to extension startup + if (tabIds.includes(tabId)) { + return; + } + browser.test.assertEq(created, tabId, "tab created before updated"); + browser.test.log(`got onUpdated ${JSON.stringify(changeInfo)}`); browser.test.assertTrue(!!changeInfo.status, "changeInfo has status"); if (Object.keys(changeInfo).some(p => properties.has(p))) { @@ -318,9 +332,11 @@ add_task(async function test_filter_property() { }, { properties: ["status"] } ); + browser.test.sendMessage("ready"); }, }); await extension.startup(); + await extension.awaitMessage("ready"); let ok = extension.awaitFinish("onUpdated"); let tab = await BrowserTestUtils.openNewForegroundTab(