1755405 - Non-persistent background page startup should not be delayed for addon updates/downgrades

Status: RESOLVED DUPLICATE of bug 1753308

(WebExtensions :: General, task)

Description

[Rob Wu [:robwu]]

Before non-persistent page startup mode was added in bug 1748524, the condition for a non-delayed background startup was startupReason !== "APP_STARTUP". This condition continues to be supported for persistent background pages only.

For non-persistent background pages, the condition for non-delayed background startup became startupReason being ADDON_INSTALL or ADDON_ENABLE. The motivation for this logic is spelled out in the source code of ext-backgroundPage.js:

    // Persistent backgrounds are started immediately except during APP_STARTUP.
    // Non-persistent backgrounds must be started immediately for new install or enable
    // to initialize the addon and create the persisted listeners.

Given this comment, it was correct to exclude APP_STARTUP (and undefined, which is the default reason in non-AddonManager extensions), and include ADDON_INSTALL and ADDON_ENABLE. However, ADDON_UPGRADE and ADDON_DOWNGRADE are missing. Since startup associated with these events can still involve a state change of persistent listeners (since the source code of the started extension has changed), the non-delayed background startup path should be taken (to ensure that the saved persistent listeners are updated as desired).

In short:

extension.startupReason !== "APP_STARTUP"

could have been replaced with the following (where the intent behind the left side is extension.startupReason !== undefined, and undefined can be replaced with whatever is passed to the Extension constructor in ExtensionTestCommon.generate):

extension.startupReason && extension.startupReason !== "APP_STARTUP"

which would be equivalent to

[
  "ADDON_INSTALL",   // <-- Added in bug 1748524
  "ADDON_ENABLE",    // <-- Added in bug 1748524
  "ADDON_UPGRADE",   // <-- currently missing
  "ADDON_DOWNGRADE", // <-- currently missing
].includes(extension.startupReason))

Comment 1

[Rob Wu [:robwu]]

In bug 1748524, the default behavior for Extension instances in xpcshell tests (with MV3) is to not start the extension by default. This default behavior is rather strange, because the most common expectation of tests that use non-AddonManager extensions is to start the extension.

I wonder, why don't we just revert to extension.startupReason !== "APP_STARTUP" and undo the the lot of additions of non-standard prefs in https://hg.mozilla.org/mozilla-central/rev/bee5e1a1f201 :

+// Since we're not using AOM, and MV3 forces event pages, bypass
+// delayed-startup for MV3 test.  These tests do not rely on startup events.
+Services.prefs.setBoolPref(
+  "extensions.webextensions.background-delayed-startup",
+  false
+);

Comment 2

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

(In reply to Rob Wu [:robwu] from comment #1)

In bug 1748524, the default behavior for Extension instances in xpcshell tests (with MV3) is to not start the extension by default. This default behavior is rather strange, because the most common expectation of tests that use non-AddonManager extensions is to start the extension.

I wonder, why don't we just revert to extension.startupReason !== "APP_STARTUP" and undo the the lot of additions of non-standard prefs in https://hg.mozilla.org/mozilla-central/rev/bee5e1a1f201 :

+// Since we're not using AOM, and MV3 forces event pages, bypass
+// delayed-startup for MV3 test.  These tests do not rely on startup events.
+Services.prefs.setBoolPref(
+  "extensions.webextensions.background-delayed-startup",
+  false
+);

All those additions that explicitly set "extensions.webextensions.background-delayed-startup" in tests are being removed as part of the patch attached to Bug 1753308.

Comment 3

[Shane Caraveo (:mixedpuppy)]

In fact, some tests fail due to this in the patch for Bug 1753308, so will handle it there.