1762800 - WebExtension code should take into account that adoptTab can abort

Status: ASSIGNED

(WebExtensions :: Frontend, defect, P3)

Description

[Oriol Brufau [:Oriol]]

Inspect a WebExtension and run:

var events = [];
browser.tabs.onDetached.addListener((...args) => {
  events.push(["onDetached", ...args]);
});
browser.tabs.onAttached.addListener((...args) => {
  events.push(["onAttached", ...args]);
});
browser.tabs.onRemoved.addListener((...args) => {
  events.push(["onRemoved", ...args]);
});
browser.tabs.onCreated.addListener((...args) => {
  events.push(["onCreated", ...args]);
});

Then, open the browser console and run:

gBrowser.addTab("about:preferences", {
  triggeringPrincipal: Services.scriptSecurityManager.getSystemPrincipal(),
  createLazyBrowser: true,
  allowInheritPrincipal: true,
  index: 0,
});

Back in the WebExtension:

var [tab] = await browser.tabs.query({index: 0, currentWindow: true});
tab.id; // 239
var win = await browser.windows.create({state: "minimized"});
await browser.tabs.move(tab.id, {windowId: win.id, index: 0}); // Error: An unexpected error occurred
await browser.tabs.move(tab.id, {windowId: win.id, index: 0}); // Error: Invalid tab ID: 239
await browser.tabs.get(tab.id); // Error: Invalid tab ID: 239
(await browser.tabs.query({index: 0, currentWindow: true}))[0].id; // 239
events; /* [
  ["onCreated", {id: 239, index: 0, windowId: 1, url: "about:preferences", ...}],
  ["onCreated", {id: 240, index: 0, windowId: 61, url: "about:blank", ...}],
  ["onDetached", 239, {oldWindowId: 1, oldPosition: 0}],
  ["onAttached", 239, {newWindowId: 61, newPosition: 0}],
  ["onRemoved", 239, {windowId: 61, isWindowClosing: false}],
] */

What's happening here is that adoptTab aborts due to bug bug 1758307. And this breaks WebExtensions:

• browser.tabs.move fails: https://searchfox.org/mozilla-central/rev/78c8fd2e91ca0f4f85ed11fa2a19564f50e75b34/browser/components/extensions/parent/ext-tabs.js#1138,1143

  nativeTab = gBrowser.adoptTab(nativeTab, insertionPoint, false);
  // That's null! Later:
  lastInsertion.set(window, nativeTab._tPos);
  // nativeTab._tPos throws
  

  That's not that bad, since browser.tabs.move is supposed to reject in case of failure, but probably it should provide a more meaningful message than "An unexpected error occurred".

• WebExt tab data is erased

  1. adoptTab 1st opens a new tab, passing the original one as adoptedTab, so webext code handles it in https://searchfox.org/mozilla-central/rev/78c8fd2e91ca0f4f85ed11fa2a19564f50e75b34/browser/components/extensions/parent/ext-browser.js#502.
  2. This associates the ID of the original tab with the new tab. https://searchfox.org/mozilla-central/source/browser/components/extensions/parent/ext-browser.js#396
  3. It also ends up assigning tab-specific extension data to the new tab https://searchfox.org/mozilla-central/rev/78c8fd2e91ca0f4f85ed11fa2a19564f50e75b34/browser/components/extensions/parent/ext-browser.js#241-244
  4. And the old tab is considered to be destroyed, so it's disassociated from its ID https://searchfox.org/mozilla-central/rev/78c8fd2e91ca0f4f85ed11fa2a19564f50e75b34/browser/components/extensions/parent/ext-browser.js#425-431
  5. adoptTab aborts, and just removes the new tab.
  6. Result: the old tab still exists, but webextensions can no longer get it by its ID, and tab-specific data is no longer there.

• Wrong events

  1. When the new temporary tab is opened, webextensions receive an onDetached event on the old window, and an onAttached event in the new one.
  2. When the adoption fails and the new tab is removed, this is just an onRemoved event on the new window.
  3. This utterly confuses extensions, making them think that the tab was moved and then removed, but actually it's still in the original window! E.g. my Tab Counter Plus is affected.

Comment 1

[Tomislav Jovanovic :zombie]

Thank you Oriol, that part about adopting has always been messy. Since you already investigated, do you maybe want to take this?