1643795 - ExtensionPreferencesManager is loaded too late and misses uninstall/disable/enabling events at startup

Status: NEW

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1639705#c6

In bug 1639705, there is a bug that caused an update to be processed as a staged uninstall + install. Due to that, I feared that it's possible for ExtensionPreferencesManager to inadvertently reset preferences, for example when an add-on with the contextualIdentities API is uninstalled (and nuke containers as a side effect - bug 1549204). This did surprisingly not happen, so I investigated and found that ExtensionPreferencesManager.jsm only starts when loaded indirectly via ext-chrome-settings-overrides.js (which is a SIDE EFFECT of loading built-in search engines).

We should rework the implementation to ensure that the events of ExtensionPreferencesManager are triggered as expected, even when none of the add-ons have started yet. Possibly by not using the events, but directly calling the methods from where the events are triggered.

• uninstall would fit better at a place after calling onUninstall of all modules at https://searchfox.org/mozilla-central/rev/baf1cd492406a9ac31d9ccb7a51c924c7fbb151f/toolkit/components/extensions/ExtensionParent.jsm#132

• disable/enabling could be done here: https://searchfox.org/mozilla-central/rev/baf1cd492406a9ac31d9ccb7a51c924c7fbb151f/toolkit/components/extensions/ExtensionParent.jsm#242-247

Comment 1

[Andrew Swan [:aswan]]

I vaguely recall some discussion of this in the past but can't find a link. The reliable way to handle this is API onUninstall methods. It seems like the ideal fix here would just be for individual APIs that can use permissions to call into (after loading of course) ExtensionPreferencesManager from their onUninstall methods. That would mean it only gets loaded when needed and would avoid the framework having to know about it (any more than it already does...) The drawback is that APIs that use preferences each need to have this call inside onUninstall but we're talking about a handful of APIs, it sounds better than the alternatives...

Comment 2

[Rob Wu [:robwu]]

This bug should not be fixed until bug 1639705 is resolved, because otherwise there may be dataloss (e.g. if a container extension is affected by bug 1639705, then the containers could be reset after an update due to bug 1549204).

Comment 3

[Shane Caraveo (:mixedpuppy)]

Simply calling await Management.asyncLoadSettingsModules(); in the appropriate places would handle this, that has to happen regardless in order to properly handle changing anything in EPM.

Comment 4

[Emma Malysz]

Shane, do you still plan on working on this? Is it the correct severity?

Comment 5

[Rob Wu [:robwu]]

Builtin search engines moved off WebExtensions in bug 1833829, and therefore that trigger for loading ExtensionPreferencesManager is gone. This increases the chance of missing the lifecycle notifications. The file name changed, but the registrations are still at https://searchfox.org/mozilla-central/rev/53e7823ee5662d353b3f81aa364426ab01fd1f70/toolkit/components/extensions/ExtensionPreferencesManager.sys.mjs#40-66