1819794 - browser.tabs.create discarded tabs lose cookieStoreId

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[Vitalii]

Attachment: discarded-tabs-lose-cookieStoreId.zip

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/110.0

Steps to reproduce:

If you create tabs using the discarded = true parameter and a non-standard cookieStoreId, after restarting the browser, or restoring a closed window, those tabs lose their cookieStoreId, and become regular tabs.

Actual results:

Tabs lose their cookieStoreId after the session is restored.

Expected results:

Tabs must not lose their cookieStoreId under any circumstances.

Test addon attached.
The problem repeats on both Windows and linux

Comment 1

[Vitalii]

Steps to produce a problem:

1. Enable session recovery after restarting the browser
2. load a temporary addon
3. click the addon button in the topbar
4. restart the browser

Created tabs no longer have cookieStoreId set to them

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Gijs,
the test extension attached to this bug is basically calling the browser.tabs.create method with the discarded and cookieStoreId options and in the tabs.create implementation we basically just pass to gBrowser.addTab the options createLazyBrowser (to create an already discarded tab) and userContextId along with the url to set on the newly created discarded tab.

I confirmed that when the tab are created the expected properties are set of the tab DOM properties (pending is set to "true" and usercontextid is set to the expected container) as well as the linked browser DOM properties (usercontextid is set), but then after restart the browser to restore the session those tabs (and linked browsers) are losing the usercontextid property.

The part that surprised me the most was that even if I active the discarded tabs before quitting and restoring the session, the tab is not in the container anymore after restarting the browser.

The fact that the tab was created as discarded is what seems to make the issue to be hit, because if I manually create a container tab (without event going through an extension, by just long clicking the + icon on the tab bar and selecting a container in a nightly build) and I load an url in it, then that tab is actually restored as a container tab (even after restoring the session a second time without activating the tabs after the first session restore).

It seems we may be missing something to handle this scenario around discarded tab correctly, and I suspect that may be around SessionStore, but I haven't dig deep enough to say for sure what and where.

Do you have some ideas about what we may missing or some specific parts of SessionStore that would be worth looking more closely to determine what we may be missing?

Comment 3

[:Gijs (he/him)]

The first thing I'd check is what SessionStore.getTabState(tab) returns for this discarded tab after it is created. It sounds like the userContextId might be missing there? If so, that's where I would start investigating. Does that help?

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1819794 - Ensure userContextId is set in the session store tab state for tabs created as discarded. r?Gijs!,willdurand!

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to :Gijs (he/him) from comment #3)

The first thing I'd check is what SessionStore.getTabState(tab) returns for this discarded tab after it is created. It sounds like the userContextId might be missing there? If so, that's where I would start investigating. Does that help?

That was extremely helpful, in fact it immediately allowed to confirm what was missing and then where it should have been done right after that.
And so I ended up drafting a patch.

Thanks a lot!

Comment 6

[Eliot Cole]

Not sure if this helps with detail / replication, but there's a few related issues to this on the mozilla github pages. Obviously delete this if all of this is stored in some secret place that I don't know of. :)

Here's the main one (I think):
https://github.com/mozilla/multi-account-containers/issues/1719

Here's my one:
https://github.com/mozilla/multi-account-containers/issues/2453

And here's another:
https://github.com/mozilla/multi-account-containers/issues/2037

Comment 7

[:Gijs (he/him)]

Luca, is this ready to land?

Comment 8

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to :Gijs (he/him) from comment #7)

Luca, is this ready to land?

Yes, thanks for the reminder, I deferred landing it to get it in m-c at the start of the cycle and let it have a bit of baking time.

I'm going to push it to autoland in a few minutes

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9db0b49bb116
Ensure userContextId is set in the session store tab state for tabs created as discarded. r=Gijs,willdurand

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9db0b49bb116