1386295 - System addon installed via getNewSideloads are never started

Status: NEW

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Julian Descottes [:jdescottes]]

Found while working on devtools as a system addon (Bug 1369801).

When starting a Firefox build containing a new system addon with an existing profile, the system addon is only installed, but is not started.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1369801#c276 for more details, or use the STRs below to reproduce with Pocket:

- comment out pocket in browser/extensions/moz.build
- ./mach clobber && ./mach build && ./mach run (firefox starts, creates a new tmp profile without pocket)
- uncomment pocket in browser/extensions/moz.build
- ./mach build && ./mach run (firefox starts with the profile created earlier)
- Pocket will be missing from Firefox (no icon in the toolbar etc)
- ./mach run
- Pocket now appears 

The system addon is installed by the following code:
http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b20929b56489e2e55438de81fa/toolkit/mozapps/extensions/internal/XPIProviderUtils.js#1674-1681

But nothing is starting the system addon afterwards. Maybe by design sideloaded addons should not be immediately started, but for system addons since they are not visible in about:addons, this means user has to restart Firefox to enable the system addon.

Comment 1

[Julian Descottes [:jdescottes]]

The addon is installed by the following call stack:

install@resource://gre/modules/addons/XPIProvider.jsm -> /Nightly.app/Contents/Resources/browser/features/[e-mail]/bootstrap.js:65:15
callBootstrapMethod@resource://gre/modules/addons/XPIProvider.jsm:4391:11
processFileChanges@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1677:13
getNewSideloads@resource://gre/modules/addons/XPIProvider.jsm:3234:7
async*getNewSideloads@resource://gre/modules/AddonManager.jsm:3105:12
_checkForSideloaded@resource:///modules/ExtensionsUI.jsm:56:28
async*init@resource:///modules/ExtensionsUI.jsm:52:5
async*BG__onWindowsRestored@/Nightly.app/Contents/Resources/browser/components/nsBrowserGlue.js:1075:5
BG_observe@/Nightly.app/Contents/Resources/browser/components/nsBrowserGlue.js:340:9
initializeWindow@resource:///modules/sessionstore/SessionStore.jsm:1161:9
onBeforeBrowserWindowShown/<@resource:///modules/sessionstore/SessionStore.jsm:1306:9
promise callback*onBeforeBrowserWindowShown@resource:///modules/sessionstore/SessionStore.jsm:1291:5
ssi_observe@resource:///modules/sessionstore/SessionStore.jsm:758:9
onLoad@chrome://browser/content/browser.js:1358:5
onload@chrome://browser/content/browser.xul:1:1

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1386295 - call startup after install in case of addon sideload

Review commit: https://reviewboard.mozilla.org/r/163454/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163454/

Comment 3

[Julian Descottes [:jdescottes]]

Comment on attachment 8892481 [details]
Bug 1386295 - call startup after install in case of addon sideload

This is of course terrible and I know very little about this part of the codebase, so I'm mostly attaching this here to get started.

This "fixes" the STRs I described here but is probably too naive. 

We should maybe check if the addon is a system addon and only call startup in this case?
Maybe another part of the code is supposed to be responsible for the startup here and is not called at the right time?

Comment 4

[Robert Helmer [:rhelmer]]

Thanks! Will take a look today.

I am still confused about this bug, though - does this only happen in a local build, or are you seeing this in packaged builds?

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to Robert Helmer [:rhelmer] from comment #4)
> Thanks! Will take a look today.
> 
> I am still confused about this bug, though - does this only happen in a
> local build, or are you seeing this in packaged builds?

It happens with both. 

I usually test with local builds but this happened at a large scale when Bug 1369801 got released in Nightly.
And I just tried by going through ./mach package and using the built artifact rather than simply using ./mach run => same issue.

Comment 6

[Robert Helmer [:rhelmer]]

OK, I've verified that this does *not* happen in 54, and *does* happen in 55, so I believe this is a regression.

Comment 7

[Robert Helmer [:rhelmer]]

Looks like this regressed in bug 1356826 specifically https://hg.mozilla.org/mozilla-central/log?rev=527435fc20db - Kris do you have time to take a look?

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8892481 [details]
Bug 1386295 - call startup after install in case of addon sideload

Obsoleting this, as it's probably irrelevant now. Thanks for looking into this :rhelmer!

Comment 9

[Julian Descottes [:jdescottes]]

Andy, this ni? is just so that you are aware of this issue, as it may impact any system addon and not just DevTools. 

The gist of the issue is that system addons are not started the first time they are installed on an existing profile. Other teams might be interested in following this, but I'm not sure which system addons are getting released in 55 or 56 (given that the regression was apparently introduced in 55).

Comment 10

[Julian Descottes [:jdescottes]]

No longer blocking Bug 1369801 based on our conversation by email. Fixing Bug 1391187 should be enough to fix the main issue for Nightly users. Will also try to leverage extensions.startupScanScopes for developer profiles with local builds.