Bug 1463751 - Tab-specific data not updated when tab is moved to another window and old window closes
MozReview-Commit-ID: IUC8OwV6YHY
--- a/browser/components/extensions/parent/ext-browser.js
+++ b/browser/components/extensions/parent/ext-browser.js
@@ -137,18 +137,18 @@ global.TabContext = class extends EventE
this.getDefaultPrototype = getDefaultPrototype;
this.tabData = new WeakMap();
windowTracker.addListener("progress", this);
windowTracker.addListener("TabSelect", this);
- this.tabDetached = this.tabDetached.bind(this);
- tabTracker.on("tab-detached", this.tabDetached);
+ this.tabAdopted = this.tabAdopted.bind(this);
+ tabTracker.on("tab-adopted", this.tabAdopted);
}
/**
* Returns the context data associated with `keyObject`.
*
* @param {XULElement|ChromeWindow} keyObject
* Browser tab or browser chrome window.
* @returns {Object}
@@ -183,34 +183,45 @@ global.TabContext = class extends EventE
onLocationChange(browser, webProgress, request, locationURI, flags) {
let gBrowser = browser.ownerGlobal.gBrowser;
let tab = gBrowser.getTabForBrowser(browser);
// fromBrowse will be false in case of e.g. a hash change or history.pushState
let fromBrowse = !(flags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT);
this.emit("location-change", tab, fromBrowse);
}
- tabDetached(eventType, {nativeTab, adoptedBy}) {
- if (!this.tabData.has(nativeTab)) {
+ /**
+ * Persists context data when a tab is moved between windows.
+ *
+ * @param {string} eventType
+ * Event type, should be "tab-adopted".
+ * @param {NativeTab} adoptingTab
+ * The tab which is being opened and adopting `adoptedTab`.
+ * @param {NativeTab} adoptedTab
+ * The tab which is being closed and adopted by `adoptingTab`.
+ */
+ tabAdopted(eventType, adoptingTab, adoptedTab) {
+ if (!this.tabData.has(adoptedTab)) {
return;
}
// Create a new object (possibly with different inheritance) when a tab is moved
// into a new window. But then reassign own properties from the old object.
- let newData = this.get(adoptedBy);
- let oldData = this.tabData.get(nativeTab);
+ let newData = this.get(adoptingTab);
+ let oldData = this.tabData.get(adoptedTab);
+ this.tabData.delete(adoptedTab);
Object.assign(newData, oldData);
}
/**
* Makes the TabContext instance stop emitting events.
*/
shutdown() {
windowTracker.removeListener("progress", this);
windowTracker.removeListener("TabSelect", this);
- tabTracker.off("tab-detached", this.tabDetached);
+ tabTracker.off("tab-adopted", this.tabAdopted);
}
};
class WindowTracker extends WindowTrackerBase {
addProgressListener(window, listener) {
window.gBrowser.addTabsProgressListener(listener);
}
@@ -299,16 +310,34 @@ class TabTracker extends TabTrackerBase
setId(nativeTab, id) {
this._tabs.set(nativeTab, id);
if (nativeTab.linkedBrowser) {
this._browsers.set(nativeTab.linkedBrowser, id);
}
this._tabIds.set(id, nativeTab);
}
+ /**
+ * Handles tab adoption when a tab is moved between windows.
+ * Ensures the new tab will have the same ID as the old one,
+ * and emits a "tab-adopted" event.
+ *
+ * @param {NativeTab} adoptingTab
+ * The tab which is being opened and adopting `adoptedTab`.
+ * @param {NativeTab} adoptedTab
+ * The tab which is being closed and adopted by `adoptingTab`.
+ */
+ adopt(adoptingTab, adoptedTab) {
+ if (!this.adoptedTabs.has(adoptedTab)) {
+ this.adoptedTabs.set(adoptedTab, adoptingTab);
+ this.setId(adoptingTab, this.getId(adoptedTab));
+ this.emit("tab-adopted", adoptingTab, adoptedTab);
+ }
+ }
+
_handleTabDestroyed(event, {nativeTab}) {
let id = this._tabs.get(nativeTab);
if (id) {
this._tabs.delete(nativeTab);
if (this._tabIds.get(id) === nativeTab) {
this._tabIds.delete(id);
}
}
@@ -358,21 +387,19 @@ class TabTracker extends TabTrackerBase
*/
handleEvent(event) {
let nativeTab = event.target;
switch (event.type) {
case "TabOpen":
let {adoptedTab} = event.detail;
if (adoptedTab) {
- this.adoptedTabs.set(adoptedTab, event.target);
-
// This tab is being created to adopt a tab from a different window.
- // Copy the ID from the old tab to the new.
- this.setId(nativeTab, this.getId(adoptedTab));
+ // Handle the adoption.
+ this.adopt(nativeTab, adoptedTab);
adoptedTab.linkedBrowser.messageManager.sendAsyncMessage("Extension:SetFrameData", {
windowId: windowTracker.getId(nativeTab.ownerGlobal),
});
}
// Save the current tab, since the newly-created tab will likely be
// active by the time the promise below resolves and the event is
@@ -389,20 +416,20 @@ class TabTracker extends TabTrackerBase
}
});
break;
case "TabClose":
let {adoptedBy} = event.detail;
if (adoptedBy) {
// This tab is being closed because it was adopted by a new window.
- // Copy its ID to the new tab, in case it was created as the first tab
- // of a new window, and did not have an `adoptedTab` detail when it was
+ // Handle the adoption in case it was created as the first tab of a
+ // new window, and did not have an `adoptedTab` detail when it was
// opened.
- this.setId(adoptedBy, this.getId(nativeTab));
+ this.adopt(adoptedBy, nativeTab);
this.emitDetached(nativeTab, adoptedBy);
} else {
this.emitRemoved(nativeTab, false);
}
break;
case "TabSelect":
@@ -446,19 +473,17 @@ class TabTracker extends TabTrackerBase
//
// Note that this event handler depends on running before the
// delayed startup code in browser.js, which is currently triggered
// by the first MozAfterPaint event. That code handles finally
// adopting the tab, and clears it from the arguments list in the
// process, so if we run later than it, we're too late.
let nativeTab = window.arguments[0];
let adoptedBy = window.gBrowser.tabs[0];
-
- this.adoptedTabs.set(nativeTab, adoptedBy);
- this.setId(adoptedBy, this.getId(nativeTab));
+ this.adopt(adoptedBy, nativeTab);
// We need to be sure to fire this event after the onDetached event
// for the original tab.
let listener = (event, details) => {
if (details.nativeTab === nativeTab) {
this.off("tab-detached", listener);
Promise.resolve().then(() => {
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_context.js
@@ -661,23 +661,29 @@ add_task(async function testMultipleWind
async expect => {
browser.test.log("Move tab from old window to the new one. Tab-specific data"
+ " is preserved but inheritance is from the new window");
await browser.tabs.move(tabs[1], {windowId: windows[1], index: -1});
await browser.tabs.update(tabs[1], {active: true});
expect(details[3], details[2], null, details[0]);
},
async expect => {
- browser.test.log("Close the tab, expect window values.");
- await browser.tabs.remove(tabs[1]);
- expect(null, details[2], null, details[0]);
+ browser.test.log("Close the initial tab of the new window.");
+ let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0});
+ await browser.tabs.remove(id);
+ expect(details[3], details[2], null, details[0]);
},
async expect => {
- browser.test.log("Close the new window and go back to the previous one.");
- await browser.windows.remove(windows[1]);
+ browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close.");
+ await browser.windows.create({tabId: tabs[1]});
+ expect(details[3], null, null, details[0]);
+ },
+ async expect => {
+ browser.test.log("Close the tab, go back to the 1st window.");
+ await browser.tabs.remove(tabs[1]);
expect(null, details[1], null, details[0]);
},
async expect => {
browser.test.log("Assert failures for bad parameters. Expect no change");
let calls = {
setIcon: {path: "default.png"},
setPopup: {popup: "default.html"},
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
@@ -251,18 +251,29 @@ add_task(async function testMultipleWind
},
async expect => {
browser.test.log("Move tab from old window to the new one, expect old values.");
await browser.tabs.move(tabs[1], {windowId: windows[1], index: -1});
await browser.tabs.update(tabs[1], {active: true});
expect(details[1]);
},
async expect => {
- browser.test.log("Close the new window and go back to the previous one.");
- await browser.windows.remove(windows[1]);
+ browser.test.log("Close the initial tab of the new window.");
+ let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0});
+ await browser.tabs.remove(id);
+ expect(details[1]);
+ },
+ async expect => {
+ browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close.");
+ await browser.windows.create({tabId: tabs[1]});
+ expect(details[1]);
+ },
+ async expect => {
+ browser.test.log("Close the tab, go back to the 1st window.");
+ await browser.tabs.remove(tabs[1]);
expect(null);
},
];
},
});
});
add_task(async function testNavigationClearsData() {
--- a/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js
@@ -560,23 +560,29 @@ add_task(async function testMultipleWind
async expect => {
browser.test.log("Move tab from old window to the new one. Tab-specific data"
+ " is preserved but inheritance is from the new window");
await browser.tabs.move(tabs[1], {windowId: windows[1], index: -1});
await browser.tabs.update(tabs[1], {active: true});
expect(details[3], details[2], null, details[0]);
},
async expect => {
- browser.test.log("Close the tab, expect window values.");
- await browser.tabs.remove(tabs[1]);
- expect(null, details[2], null, details[0]);
+ browser.test.log("Close the initial tab of the new window.");
+ let [{id}] = await browser.tabs.query({windowId: windows[1], index: 0});
+ await browser.tabs.remove(id);
+ expect(details[3], details[2], null, details[0]);
},
async expect => {
- browser.test.log("Close the new window and go back to the previous one.");
- await browser.windows.remove(windows[1]);
+ browser.test.log("Move the previous tab to a 3rd window, the 2nd one will close.");
+ await browser.windows.create({tabId: tabs[1]});
+ expect(details[3], null, null, details[0]);
+ },
+ async expect => {
+ browser.test.log("Close the tab, go back to the 1st window.");
+ await browser.tabs.remove(tabs[1]);
expect(null, details[1], null, details[0]);
},
async expect => {
browser.test.log("Assert failures for bad parameters. Expect no change");
let calls = {
setIcon: {path: "default.png"},
setPanel: {panel: "default.html"},