1762249 - tabs.create() fails when it is called with "openerTabId" for a discarded tab

Status: RESOLVED FIXED

(WebExtensions :: Compatibility, defect, P3)

Description

[YUKI "Piro" Hiroshi]

browser.tabs.create() throws an error unexpectedly when openerTabId is an ID of a discarded tab.

Original report: https://github.com/piroor/treestyletab/issues/3099

Steps to reproduce

1. Open two or more tabs.
2. Discard one of tabs.
3. Call browser.tabs.create({ openerTabId: <the ID of a discarded tab> }).

Expected result

A new tab is opened successfully.

Actual result

No tab is created. If you catch errors of the API call, you get an error with a message "Error: An unexpected error occurred".

Environment

• Windows 10 21H2
• Nightly 100.0a1 build ID: 20220329204247
• Firefox 98.0.2 build ID: 20220322144853

Comment 1

[YUKI "Piro" Hiroshi]

Attachment: testcase: openerTabId-testcase.xpi

Here is a testcase addon. You can confirm that the result is different between Firefox and Google Chrome.

On Firefox:

1. Go to about:debugging.
2. Click "This Firefox" (or "This Nightly").
3. Click "Load Temporary Add-on...".
4. Choose the testcase XPI.
   • Expected result:
     • Two tabs https://example.com/?parent and https://example.com/?child are opened.
   • Actual result:
     • Only one tab https://example.com/?parent is opened.
     • If you open the debugger for the testcase addon, an error "Error: An unexpected error occurred" is reported.

On Google Chrome (I've used Chrome 99.0.4844.84):

1. Go to chrome://extensions/.
2. Activate developer mode.
3. Click "Load unpacked extension".
4. Choose the directory extracted the testcase XPI.
   • Result:
     • Two tabs https://example.com/?parent and https://example.com/?child are opened.

Comment 2

[YUKI "Piro" Hiroshi]

Firefox gets tracked tabs based on the given openerTabId with:
https://searchfox.org/mozilla-central/rev/766773ca7e3495a3f1ba365f47dffefdea29e3ee/browser/components/extensions/parent/ext-tabs.js#732
https://searchfox.org/mozilla-central/rev/766773ca7e3495a3f1ba365f47dffefdea29e3ee/browser/components/extensions/parent/ext-tabs.js#898
Something further operation looks to raise the "Error: An unexpected error occurred" error, but I've not found which code does that yet.

Comment 3

[YUKI "Piro" Hiroshi]

I've got that the error comes from https://searchfox.org/mozilla-central/rev/625c3d0c8ae46502aed83f33bd530cb93e926e9f/browser/base/content/tabbrowser.js#2837 , the raw error is: CanonicalBrowsingContext.setCrossGroupOpener: Argument 1 is not an object.

Comment 4

[YUKI "Piro" Hiroshi]

Just skipping CanonicalBrowsingContext.setCrossGroupOpener() for null openerBrowser.browsingContext looks reasonable for me, because the only one code using the returned value of GetCrossGroupOpenerId() looks to be here and the function is designed to do nothing when the opener tab is already discarded. Is it a correct approach?

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Skipping that call if the opener browser is not available seems very reasonable, yes.

Comment 6

[YUKI "Piro" Hiroshi]

Attachment: Bug 1762249 - Accept already-discarded owner tab r?Gijs

Comment 7

[YUKI "Piro" Hiroshi]

Thanks!

Comment 8

[YUKI "Piro" Hiroshi]

I apologize my long silence about the patch. I've rewrite it based on the latest mozilla-central, with fixes commented by the reviewer.

Comment 9

[Elhem Enohpi]

Thanks for the patch, Piro!

I just wanted to note that this bug seems to cause a data loss issue with Tab Session Manager, when used with Tree Style Tab / Sidebery support. When restoring a session, in some conditions a tab will fail to open, without user notification, causing subtle corruption of the session:

Tab is not restored if it's a Tree Style Tab child tab, and was the active tab when saved. · Issue #1548 · sienori/Tab-Session-Manager

I tried the above testcase xpi with Developer Edition 147, and the console error message is the same as with Tab Session Manager:

DOMException: CanonicalBrowsingContext.crossGroupOpener setter: Can't set crossGroupOpener to null

Comment 10

[YUKI "Piro" Hiroshi]

The patch has been reviewed and accepted. I have no authority so could anyone check-in it?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e8d015b620ce
https://hg.mozilla.org/integration/autoland/rev/112e58af4ddc
Accept already-discarded owner tab r=Gijs,tabbrowser-reviewers,dao

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/112e58af4ddc