User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0 Build ID: 20160425115534 Steps to reproduce: I tried to install the Stylish extension from addons.mozilla.org in FF 46 (on Ubuntu 14.04). Downloading and the installation worked fine, and then FF told me it needs to restart. Actual results: Before the restart the extension was shown in the add-ons list, but after the restart the extension wasn't there any more. The following error was shown on restart: 1464793560318 addons.xpi ERROR Unable to read add-on manifest from /u/m525025/.mozilla/firefox/7c14k6yq.default/extensions/staged/{46551EC9-40F0-4e47-8E18-8E5CF550CFB8}.xpi: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]" nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)" location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm :: XPCU_serviceLambda :: line 230" data: no] Stack trace: XPCU_serviceLambda()@resource://gre/modules/XPCOMUtils.jsm:230 < XPCU_defineLazyGetter/<.get()@resource://gre/modules/XPCOMUtils.jsm:198 < defineSyncGUID()@resource://gre/modules/addons/XPIProvider.jsm:1254 < loadManifestFromZipReader<()@resource://gre/modules/addons/XPIProvider.jsm:1433 < TaskImpl_run()@resource://gre/modules/Task.jsm:315 < Handler.prototype.process()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:933 < this.PromiseWalker.walkerLoop()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:812 < this.PromiseWalker.scheduleWalkerLoop/<()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746 < syncLoadManifestFromFile()@resource://gre/modules/addons/XPIProvider.jsm:1489 < this.XPIProvider.processPendingFileChanges()@resource://gre/modules/addons/XPIProvider.jsm:3279 < this.XPIProvider.checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3568 < this.XPIProvider.startup()@resource://gre/modules/addons/XPIProvider.jsm:2657 < callProvider()@resource://gre/modules/AddonManager.jsm:227 < _startProvider()@resource://gre/modules/AddonManager.jsm:833 < AddonManagerInternal.startup()@resource://gre/modules/AddonManager.jsm:1016 < this.AddonManagerPrivate.startup()@resource://gre/modules/AddonManager.jsm:2782 < amManager.prototype.observe()@resource://gre/components/addonManager.js:58 Expected results: The extension should have been installed. Btw. this also happened in FF 45. But after downgrading to FF 40 I was able to install Stylish, and then after upgrading FF to version 46, Stylish was still installed and working correctly. I think I have had the same problem with other extensions, which I tried to install, but cannot say if the same problem (output in terminal) had occured. I also treid startgin with a new user profile, but this didn't change anything.

Are you able to install and use this add-on with a fresh profile? https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

As I said in the last sentence (btw., sorry for the typos), I tried it with a new profile (deleted the whole ~/.mozilla folder), but the problem is still there.

Wfm in Fx 46.0.1, Mac 10.11.5.

Wfm in Fx 46.0.1, Ubuntu 14.04 as well.

I had some hopes after the last comments, but unfortunately I still have the same problem with FF 46.0.1.

This sounds a lot like https://bugzilla.mozilla.org/show_bug.cgi?id=717904 In any case, this is the code that imports that storage service that is failing: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#1227-1229 I suspect this code dates back to when the add-on database was in sqlite instead of json, and I'm inclined to just remove it. (If this line really is pivotal to ensuring things get loaded in the right order, it probably should be somewhere else, not buried in XPIProvider.jsm!) Dave, does this seem reasonable? Any guidance on how much coverage to go for in a try run?

Hitting precisely same issue in BlueGriffon 2.0 on Windows. I try/catch'd the storage line and it works as expected.

This problem seems to also affect bundled FF extensions. After trying out the (upstream) FF 47, I get this warning/error on every launch: 1465377475731 addons.xpi-utils WARN updateMetadata: Add-on firefox@getpocket.com is invalid: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]" nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)" location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm :: XPCU_serviceLambda :: line 230" data: no] Stack trace: XPCU_serviceLambda()@resource://gre/modules/XPCOMUtils.jsm:230 < XPCU_defineLazyGetter/<.get()@resource://gre/modules/XPCOMUtils.jsm:198 < defineSyncGUID()@resource://gre/modules/addons/XPIProvider.jsm:1205 < loadManifestFromZipReader<()@resource://gre/modules/addons/XPIProvider.jsm:1384 < TaskImpl_run()@resource://gre/modules/Task.jsm:319 < Handler.prototype.process()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937 < this.PromiseWalker.walkerLoop()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816 < this.PromiseWalker.scheduleWalkerLoop/<()@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:750 < syncLoadManifestFromFile()@resource://gre/modules/addons/XPIProvider.jsm:1440 < updateMetadata()@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1776 < processFileChanges()@resource://gre/modules/addons/XPIProvider.jsm -> resource://gre/modules/addons/XPIProviderUtils.js:1973 < this.XPIProvider.checkForChanges()@resource://gre/modules/addons/XPIProvider.jsm:3620 < this.XPIProvider.startup()@resource://gre/modules/addons/XPIProvider.jsm:2622 < callProvider()@resource://gre/modules/AddonManager.jsm:227 < _startProvider()@resource://gre/modules/AddonManager.jsm:755 < AddonManagerInternal.startup()@resource://gre/modules/AddonManager.jsm:938 < this.AddonManagerPrivate.startup()@resource://gre/modules/AddonManager.jsm:2773 < amManager.prototype.observe()@resource://gre/components/addonManager.js:57 1465377475734 addons.xpi-utils WARN Could not uninstall invalid item from locked install location

Can we bisect to figure out what caused this to start failing? While we might not need the storage service any more it is possible that if we change the initialization order we might break someone else.

Here's a try run with the reference to Services.storage removed (and the uuid generation changed to the more concise uuid-generator). https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab3b89c683d There are a bunch of clearly unrelated intermittent failures, I retriggered some of the non-obvious ones and they all passed on a subsequent run. In any case, I would think that if this change revealed a startup ordering problem, we would see much more spectacular failure. Dave, you were hesitant about this when we talked before, what do you think?

(In reply to Dave Townsend [:mossop] from comment #10) > Can we bisect to figure out what caused this to start failing? Oops, we talked about this on IRC but for posterity, I'm unable to reproduce this myself so I can't bisect. glazou, any chance you could try an older build and/or do a mozregression?

(In reply to Andrew Swan [:aswan] from comment #11) > Here's a try run with the reference to Services.storage removed (and the > uuid generation changed to the more concise uuid-generator). > https://treeherder.mozilla.org/#/jobs?repo=try&revision=eab3b89c683d > > There are a bunch of clearly unrelated intermittent failures, I retriggered > some of the non-obvious ones and they all passed on a subsequent run. In > any case, I would think that if this change revealed a startup ordering > problem, we would see much more spectacular failure. > > Dave, you were hesitant about this when we talked before, what do you think? I think we can go ahead and land on central. Did we want to uplift? The original report is from 46. If so we might need to think of some specific things for QA to focus on here.

FWIW, the guilty line is now try/catch'd in BlueGriffon with thousands of users happy with the result on all platforms. Without that change, installation of add-ons was failing every time ; with the change in, I got tons of feedback the whole thing works normally again.

(In reply to Andrew Swan [:aswan] from comment #12) > Oops, we talked about this on IRC but for posterity, I'm unable to reproduce > this myself so I can't bisect. glazou, any chance you could try an older > build and/or do a mozregression? Hardly. BlueGriffon 2.0 was for me a major uplift and the regular changes in the build system don't make it easy for me to regress since I'm not updating the core on a daily basis...

(In reply to Dave Townsend [:mossop] from comment #13) > I think we can go ahead and land on central. Did we want to uplift? The > original report is from 46. If so we might need to think of some specific > things for QA to focus on here. Right, both to check that the original problem is fixed and to ensure we're not introducing a regression. Both of those seem challenging! Once this lands I can ask for approval for Aurora and Beta, is this worth going all the way to release? (we are at the beginning of an extended release cycle so 48 going to release is still 7+ weeks away)

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

Kris, can you review? Mossop already gave verbal approval in comment 13 but he's not accepting reviews right now.

Comment on attachment 8761613 [details] Bug 1277295 Remove obsolete reference to storage service https://reviewboard.mozilla.org/r/58724/#review55736 ::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1233 (Diff revision 1) > Object.defineProperty(aAddon, "syncGUID", { > get: () => { > // Generate random GUID used for Sync. > - // This was lifted from util.js:makeGUID() from services-sync. > - let rng = Cc["@mozilla.org/security/random-generator;1"]. > - createInstance(Ci.nsIRandomGenerator); > + let guid = Cc["@mozilla.org/uuid-generator;1"] > + .getService(Ci.nsIUUIDGenerator) > + .generateUUID().toString(); s/.toString()/.number/ I assume we're OK with the fact that this is going to produce a GUID with a completely different format from the old code?

(In reply to Kris Maglione [:kmag] from comment #19) > Comment on attachment 8761613 [details] > Bug 1277295 Remove obsolete reference to storage service > > https://reviewboard.mozilla.org/r/58724/#review55736 > > ::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1233 > (Diff revision 1) > > Object.defineProperty(aAddon, "syncGUID", { > > get: () => { > > // Generate random GUID used for Sync. > > - // This was lifted from util.js:makeGUID() from services-sync. > > - let rng = Cc["@mozilla.org/security/random-generator;1"]. > > - createInstance(Ci.nsIRandomGenerator); > > + let guid = Cc["@mozilla.org/uuid-generator;1"] > > + .getService(Ci.nsIUUIDGenerator) > > + .generateUUID().toString(); > > s/.toString()/.number/ > > I assume we're OK with the fact that this is going to produce a GUID with a > completely different format from the old code? Please have a sync person sign off on this...

mconnor, can you take a look at the change above or suggest somebody who can?

FWIW, I got a bug report about this happening on Debian with esr45.

No love from mconnor. gps, it looks like you touched this code most recently even though it was a few years ago. can you take a look at those changes or suggest a good person to take a look?

Comment on attachment 8761613 [details] Bug 1277295 Remove obsolete reference to storage service https://reviewboard.mozilla.org/r/58724/#review57454 This should be OK. Note that there are tests (e.g. toolkit/mozapps/extensions/test/xpcshell/test_syncGUID.js) that verify `syncGUID` is 9+ characters. You may want to rewrite those to assert syncGUID is the expected UUID length.

https://reviewboard.mozilla.org/r/58724/#review55736 > s/.toString()/.number/ > > I assume we're OK with the fact that this is going to produce a GUID with a completely different format from the old code? Yes. The SyncGUID is used to uniquely identify an add-on in the install. IIRC it is treated as a black box by Sync.

Comment on attachment 8761613 [details] Bug 1277295 Remove obsolete reference to storage service Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58724/diff/1-2/

Thanks for the feedback. Rebased to updated m-c, updated the test as suggested, and doing one more try run out of caution.

Pushed by aswan@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5f93302810d0 Remove obsolete reference to storage service r=gps,kmag

I tried to reproduce the initial issue using Firefox 46.0.1 and 45.0 on Ubuntu 14.04 64bit but with no success. I also installed top 10 of the most used add-ons that required restart and all of them installed successfully and were still active after each restart. Testing was done across platforms (Windows 7 64bit, Mac OS X 10.12 and Ubuntu 14.04 64bit) using Firefox 50.0 RC build. Carsten Sommer can you please verify that 50.0 RC build works for you since you were able to reproduce this problem in the first place? Link to build: https://archive.mozilla.org/pub/firefox/candidates/50.0-candidates/build1/

Hello Bogdan Maris, yes, the FF 50.0 RC build fixes the problem for me. I tried to install the Stylish extension in the current FF 49 three times, and each time it failed with the same error messages, as reported in the initial bug report. Then I tried to install the extension in FF 50 RC three times, and this was successful each time. All tests were done on the same user account and with the same user profile.

(In reply to Carsten Sommer from comment #31) > Hello Bogdan Maris, yes, the FF 50.0 RC build fixes the problem for me. > > I tried to install the Stylish extension in the current FF 49 three times, > and each time it failed with the same error messages, as reported in the > initial bug report. Then I tried to install the extension in FF 50 RC three > times, and this was successful each time. All tests were done on the same > user account and with the same user profile. Thanks for this! I'll mark this as verified then.