1720487 - at least MultiAccountContainer extension frequently losing its extension storage: onboarding launched again, sites assigned to container forgotten

Status: RESOLVED FIXED

(WebExtensions :: Storage, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Firefox Nightly 91.0a1 + 92.0a1 on Windows 8.1, Multi Account Container extension 7.3.0 - https://addons.mozilla.org/en-GB/firefox/addon/multi-account-containers/

At least the MultiAccountContainer extension started to frequent lose its extension storage: the onboarding gets launched again, sites assigned to a container have been forgotten. This happened three times in the last week, the first time after the update to 91.0a1 20210709094609.

There are no other add-ons installed which use the extension storage (it seems). Because the add-on version has been installed since January 2021 (and the issue hasn't been observed for the 3+ years it had been installed), I suspect an issue with Firefox.

Comment 1

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

Hi Sebastian,
do you recall if the disk space (in the same partition where the Firefox profile is stored) was almost fully used?

---

I just looked to the multi-account-containers manifest.json file and I see that the extension isn't requesting the "unlimitedStorage" permission:

• the data that the extension stored using the storage.local API is internally stored into an IndexedDB backend
• without the "unlimitedStorage" the data is going to be stored in an IndexedDB database with the storage type PERSISTENT_TYPE_DEFAULT (on the contrary with the "unlimitedStorage" the type would be set to PERSISTENT_TYPE_PERSISTENT internally by the WebExtensions internals).
• based on a quick look to QuotaManager internals related to the eviction logic, there is currently no special case related to the extensions, which makes me think that the cause may be that under a certain "free disk space threshold" QuotaManager may be also evicting the moz-extensions origins that are not PERSISTENT_TYPE_PERSISTENT (and the same would also happen with data stored directly into IndexedDB by the extension, when using the IndexedDB WebAPI directly).

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Yes, I think the first time this happened, free disk space was very low (~200MB). The second time there were ~3GB free, the last time ~1GB.

Do you have an idea why the issue hit now (any changes to storage)? The low levels of free disk space have been frequently hit in the past.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Messaging System' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 4

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

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #2)

Yes, I think the first time this happened, free disk space was very low (~200MB). The second time there were ~3GB free, the last time ~1GB.

Do you have an idea why the issue hit now (any changes to storage)? The low levels of free disk space have been frequently hit in the past.

I'm not sure, but I think it would be good to double-check that with someone actively working on the QuotaManager.

Hey Jan,
May I ask your perspective on this issue? in particular:

• does the rationale in comment 1 (paired with the free disk space Sebastian did mention in comment 2) looks like a possible scenario to you?
• was there any recent change in this area that could make it more likely that we hit that scenario now?

Thanks in advance for your help on investigating this issue.

Comment 5

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

bad bot!

Moving the issue back into the WebExtensions bugzilla components.
(I guess that the "onboarding" in the issue description triggered moving the bug into "Firefox::Messaging System" bugzilla component)

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Yeah, this data can be cleared on storage pressure. The 9th byte of the .metadata-v2 file indicates whether the origin called navigator.storage.persist() and had it granted or if someone called nsIQuotaManagerService.persist for the origin, and that byte doesn't seem to be set in the synthetic origins generated for these cases. That is, the ^userContextId=4294967295 OriginAttribute that is used creates a synthetic origin, so there's nothing the extension can do directly to impact that origin, only the underlying webext storage code backend can do this.

It probably makes sense for the webext code to explicitly call persist for these synthetic origins at least. Longer term, we could potentially establish a better policy mechanism for the webext scheme to try and indicate what it wants for different principals, including setting specific quotas for synthetic origins like this. There is the AddonPolicy() off the principal, but that's main-thread-only and QuotaManager lives on PBackground, so that won't work as-is.

Comment 7

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

Thanks for the confirmation Andrew.

The "unlimitedStorage" permission does ensure that the WebExtensions internals will call persist for the synthetic origin used as a backend for the storage.local API (behavior that is also covered explicitly by the test_ext_unlimitesStorage.html mochitest).

I think we may want to re-evaluate the behavior when "unlimitedStorage" isn't requested, because the storage.local API without it is expected to have a more restrictive quota limit (which is currently left to the QuotaManager to enforce), but evicting the data for the extension origin is not exactly like evicting the data from a webapp (which can usually retrieve the data again from the server side) and I think it would be surprising (for the extension developers and the users) even when the extension is using IndexedDB WebAPI directly and not through storage.local (e.g. some extension does use it along with storage.local to store binary data as Blobs).

We may repurpose this issue for that or file a new one (but given that this already contain an analysis and a confirmation of that analysis, I think it may be reasonable to track a fix on the Firefox side using this one).

In the meantime, multi-account-container should really be requesting the "unlimitedStorage" permission, and so I created a pull request in the multi-account-container and linked here as a see also.

Comment 8

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

Attachment: Bug 1720487 - Ensure QuotaManager does not evict PERSISTENT_TYPE_DEFAULT data associated to a WebExtension origin. r?#dom-storage-reviewers

This patch introduce an additional check inside the Helper::GetInactiveOriginInfos static method
used internally by CollectOriginsForEviction to ensure that QuotaManager will not select an
extension origin as an inactive origin to evict non persisted data from.

The rationale is that unlike websites (which may more likely using the locally stored data
as a local cache, but still able to retrieve the same data again from the server side),
extensions do not have a remote counterpart and so evicting their data would result
into potential data loss for the users.

Besides that, the browser extensions (unlike websites) are explicitly installed and uninstalled by
the user and all the data associated to the extension will be completely removed when the
extension is uninstalled.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4fb4bcebb6c4
Ensure QuotaManager does not evict PERSISTENT_TYPE_DEFAULT data associated to a WebExtension origin. r=dom-storage-reviewers,asuth

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/4fb4bcebb6c4

Comment 11

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

Comment on attachment 9232376 [details]
Bug 1720487 - Ensure QuotaManager does not evict PERSISTENT_TYPE_DEFAULT data associated to a WebExtension origin. r?#dom-storage-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Data stored by installed extensions in a storage backend that is managed by QuotaManager (e.g. IndexedDB WebAPI and browser.storage.local WebExtensions API would be ones of the most commonly used ones) may be evicted if the disk space is below certain threshold and the extension is detected as one of the least active origin, resulting in potential data loss for the extensions users.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The fix is just a few line changes (and, given the kind of changes introduces, we don't expect any unexpected side-effects).

The change is also covered by an automated test that is making sure that:

• the "web origins data eviction" is still behaving as expected
• and that the extension origin are not selected anymore as potential inactive origin to evict data from

• String changes made/needed:

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9232376 [details]
Bug 1720487 - Ensure QuotaManager does not evict PERSISTENT_TYPE_DEFAULT data associated to a WebExtension origin. r?#dom-storage-reviewers

Approved for 91 beta 6, thanks.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/a281973a407a