1575948 - Add new install install location for application addons

Status: RESOLVED FIXED

(Firefox :: Normandy Client, enhancement)

Description

[Rehan Dalal (Inactive)]

This comment is outdated. See comment #3 for revised details.

Currently we install addons in the default extensions folder of the user's profile.

As per the addon manager docs there are four levels of priority each with their own install location:

1. temporary install (about:debugging)
2. normal user install into profile (about:addons or AMO/TestPilot/etc.)
3. system add-on updates
4. built-in system add-ons

We should add 3 new levels/locations:

1. temporary install (about:debugging)
2. normandy high priority rollouts
3. normal user install into profile (about:addons or AMO/TestPilot/etc.)
4. normandy rollouts
5. normandy studies
6. system add-on updates
7. built-in system add-ons

Potential directory names for these new types:

• normandy high priority rollouts: normandy-priority-rollouts
• normandy rollouts: normandy-rollouts
• normandy studies: normandy-studies

These would all live inside the profile directory.

The priorities and locations are defined in: toolkit/mozapps/extensions/internal/XPIProvider.jsm

Comment 1

[Rehan Dalal (Inactive)]

From :rhelmer on Slack:

so one thing you want to think about, anti-virus vendors tend to rip our stuff out if we do anything unusual, which is so terrible

Comment 2

[Rehan Dalal (Inactive)]

Attachment: Bug 1575948 - Add ability to specify install location

Comment 3

[Rehan Dalal (Inactive)]

The current plan:

• Create a new installation location for all Normandy-installed extensions (eg: ProfD/normandy)
• It will be lower priority than user installed extensions and temporary addons but higher than all other locations
• It will be in the profile scope

Comment 4

[Andrew Swan [:aswan]]

This bug describes a "what" without any of the "why". There was some limited discussion of the rationale in bug 1613647 but this bug appears to be a more appropriate place for that discussion.

The justifications given in bug 1613647 are vague. E.g.:

Having a separate install location means we can go forward confidently, knowing exactly how the system will behave in the face of user intervention, built in addons, and other sources.
Exactly what do we not know about how the current system will behave in this situation?
Also, "knowing how the system will behave" is desirable but it is not the same as "will behave in the way we want it to". For instance, if Normandy has its own install location that is lower priority than user-installed extensions, then consider the situation described in https://bugzilla.mozilla.org/show_bug.cgi?id=1613647#c8 : a crisis like the addon signing certificate expiration. If some hotfix addon is created with the intention of distributing it through Normandy and some users install it manually, then an update is created, Normandy won't be able to push updates to those users and they will have to wait for the slower addon update check timer (if in fact we even publish updates for a hotfix intended to be distributed via Normandy). This doesn't sound desirable.

Anyway, a long argument here isn't in anybody's interest. If the resources are available to make this happen, I would recommend at a minimum:

• Reopen bug 557710 and make it block this bug
• Open a new bug to handle graceful updates when installing/uninstalling addons that are present in multiple locations
• Think through requirements for this new directory: the add-ons team has been busy recently limiting nasty things that third parties can do by sideloading. Will there be requirements for addons in this directory (e.g., they must be signed with the "privileged" signature?) Sideloading is currently controlled by a location's scope property but comment 3 indicates that this location will use the existing PROFILE scope where sideloading is current enabled, so any additional policy here will involved ad hoc code enforcement.
• The patch here is also incomplete as it doesn't provide a way to uninstall from a particular location. Implementing that correctly will overlap with bug 557710, I would suggest dealing with 557710 first.

Comment 5

[Rehan Dalal (Inactive)]

Attachment: Bug 1575948 - Add normandy directory location to AddonManager

Comment 6

[Rehan Dalal (Inactive)]

Think through requirements for this new directory: the add-ons team has been busy recently limiting nasty things that third parties can do by sideloading. Will there be requirements for addons in this directory (e.g., they must be signed with the "privileged" signature?) Sideloading is currently controlled by a location's scope property but comment 3 indicates that this location will use the existing PROFILE scope where sideloading is current enabled, so any additional policy here will involved ad hoc code enforcement.

All addons installed in this directory must be signed with the Mozilla privileged key or system addon key.

The patch here is also incomplete as it doesn't provide a way to uninstall from a particular location. Implementing that correctly will overlap with bug 557710, I would suggest dealing with 557710 first.

Can we not uninstall from a particular location by getting the addon from XPIStates and then calling its uninstall method?

Comment 7

[Robert Helmer [:rhelmer]]

(In reply to Andrew Swan [:aswan] from comment #4)

• The patch here is also incomplete as it doesn't provide a way to uninstall from a particular location. Implementing that correctly will overlap with bug 557710, I would suggest dealing with 557710 first.

I've re-opened this bug and will finish it up.

Comment 8

[Andrew Swan [:aswan]]

(In reply to Rehan Dalal [:rehan, :rdalal] from comment #6)

Can we not uninstall from a particular location by getting the addon from XPIStates and then calling its uninstall method?

First of all, XPIStates is an internal part of the addon manager, not directly accessible to Normandy. In any case, instances of XPIState don't have an uninstall() method.

The only external interface to uninstall an addon is uninstall() on an AddonWrapper (i.e., what you get back from various AddonManager.getAddonByXXX() methods). And that method uninstalls the active instance of the addon. If Normandy has uninstalled some addon and that same addon was manually installed by the user, that would not do the right thing.

Comment 9

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1575948 add new system-extensions location for Normandy

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/527ed2d24327
add new app-addons location in AddonManager r=aswan

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebece03cb084
Backed out changeset 527ed2d24327 for browser chrome failures on browser_startup_mainthreadio.js. CLOSED TREE

Comment 12

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=windows%2C10%2Cx64%2Casan%2Cmochitests%2Ctest-windows10-64-asan%2Fopt-mochitest-browser-chrome-e10s-7%2Cm%28bc7%29&revision=527ed2d24327f38bb1363ad6f12bfbcf7df67c9a&selectedTaskRun=O9jjBQN4RdqZlybQ-PZN_Q-0

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=301747848&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/ebece03cb084

[task 2020-05-11T16:33:27.729Z] 16:33:27     INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_mainthreadio.js | fsync on C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\xulstore\data.mdb as many times as expected before profile selection - 
[task 2020-05-11T16:33:27.729Z] 16:33:27     INFO - whitelisted paths before opening first browser window:
[task 2020-05-11T16:33:27.729Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner - condition: true, stat: 1, listedPath: ProfD:
[task 2020-05-11T16:33:27.730Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\cookies.sqlite-journal - condition: true, stat: 3, write: 4, listedPath: ProfD:cookies.sqlite-journal
[task 2020-05-11T16:33:27.730Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\cookies.sqlite - condition: true, stat: 2, read: 2, write: 1, listedPath: ProfD:cookies.sqlite
[task 2020-05-11T16:33:27.730Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\cookies.sqlite-wal - condition: true, stat: 2, listedPath: ProfD:cookies.sqlite-wal
[task 2020-05-11T16:33:27.730Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\extensions - condition: true, stat: 1, listedPath: ProfD:extensions
[task 2020-05-11T16:33:27.731Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\extensions - condition: true, stat: 1, listedPath: XCurProcD:extensions
[task 2020-05-11T16:33:27.731Z] 16:33:27     INFO -   Z:\task_1589214406\AppData\Roaming\Mozilla\Firefox - ignoreIfUnused: true, condition: true, stat: 1, listedPath: UAppData:
[task 2020-05-11T16:33:27.731Z] 16:33:27     INFO -   C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\xulstore\data.mdb - condition: true, read: 1, listedPath: ProfD:xulstore/data.mdb
[task 2020-05-11T16:33:27.731Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\features\[e-mail] - stat: 3, close: 2, listedPath: XREAppFeat:[e-mail]
[task 2020-05-11T16:33:27.732Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\features\[e-mail] - stat: 3, close: 2, listedPath: XREAppFeat:[e-mail]
[task 2020-05-11T16:33:27.732Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\features\[e-mail] - stat: 3, close: 2, listedPath: XREAppFeat:[e-mail]
[task 2020-05-11T16:33:27.732Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\features\[e-mail] - stat: 3, close: 2, listedPath: XREAppFeat:[e-mail]
[task 2020-05-11T16:33:27.732Z] 16:33:27     INFO -   Z:\task_1589214406\build\application\firefox\browser\features\[e-mail] - stat: 3, close: 2, listedPath: XREAppFeat:[e-mail]
[task 2020-05-11T16:33:27.733Z] 16:33:27     INFO - (PoisonIOInterposer) stat - C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\extensions
[task 2020-05-11T16:33:27.733Z] 16:33:27     INFO - Buffered messages finished
[task 2020-05-11T16:33:27.733Z] 16:33:27     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup_mainthreadio.js | unexpected stat on C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\system-extensions before opening first browser window - 
[task 2020-05-11T16:33:27.733Z] 16:33:27     INFO - Stack trace:
[task 2020-05-11T16:33:27.734Z] 16:33:27     INFO -   nsLocalFile::ResolveAndStat
[task 2020-05-11T16:33:27.734Z] 16:33:27     INFO -   from (self-hosted:505:18)
[task 2020-05-11T16:33:27.734Z] 16:33:27     INFO -   readAddons (resource://gre/modules/addons/XPIProvider.jsm:1080:12)
[task 2020-05-11T16:33:27.734Z] 16:33:27     INFO -   scanForChanges (resource://gre/modules/addons/XPIProvider.jsm:1430:16)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   checkForChanges (resource://gre/modules/addons/XPIProvider.jsm:2915:17)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   startup (resource://gre/modules/addons/XPIProvider.jsm:2417:9)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   callProvider (resource://gre/modules/AddonManager.jsm:206:21)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   _startProvider (resource://gre/modules/AddonManager.jsm:574:16)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   startup (resource://gre/modules/AddonManager.jsm:638:9)
[task 2020-05-11T16:33:27.735Z] 16:33:27     INFO -   startup (resource://gre/modules/AddonManager.jsm:3461:9)
[task 2020-05-11T16:33:27.736Z] 16:33:27     INFO -   observe (resource://gre/modules/addonManager.js:84:9)
[task 2020-05-11T16:33:27.736Z] 16:33:27     INFO -   XPCWrappedJS method call
[task 2020-05-11T16:33:27.736Z] 16:33:27     INFO -   XREMain::XRE_main
[task 2020-05-11T16:33:27.737Z] 16:33:27     INFO - (PoisonIOInterposer) stat - C:\Users\task_1589214406\AppData\Local\Temp\tmpoan1gd.mozrunner\system-extensions
[task 2020-05-11T16:33:27.737Z] 16:33:27     INFO - (PoisonIOInterposer) stat - Z:\task_1589214406\build\application\firefox\browser\features\[e-mail]
[task 2020-05-11T16:33:27.738Z] 16:33:27     INFO - (PoisonIOInterposer) stat - Z:\task_1589214406\build\application\firefox\browser\features\[e-mail]
[task 2020-05-11T16:33:27.738Z] 16:33:27     INFO - (PoisonIOInterposer) stat - Z:\task_1589214406\build\application\firefox\browser\features\[e-mail]

Comment 13

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=48ebd518372f3239471e428cdcf6a76219309927

Comment 14

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b41e55e3ebf7a8cb83d8d27df2c64ea421e2a1b7

Comment 15

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dca926ec497688e4e6e2ed9e5e3819030e206a6

Comment 16

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa135e04e17dba168801336b1b20020d97c65261

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c867966e6eb8
add new system-extensions location for Normandy r=aswan

Comment 18

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on browser_startup_mainthreadio.js

backout: https://hg.mozilla.org/integration/autoland/rev/a9d0bbab09f04876531524d9cf03b41f07001c5c

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=c867966e6eb850c942d6261d9655be3f2c0500e4&searchStr=m%28bc1%29&selectedTaskRun=RGrdz_6iSHeYGKULZjiIMA-0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302510457&repo=autoland&lineNumber=1688

[task 2020-05-15T21:25:51.223Z] 21:25:51 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_mainthreadio.js | stat on C:\Users\task_1589575356\AppData\Local\Temp\tmpavhy7i.mozrunner\extensions as many times as expected before opening first browser window -
[task 2020-05-15T21:25:51.223Z] 21:25:51 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_mainthreadio.js | stat on Z:\task_1589575356\build\application\firefox\browser\extensions allowed 1 more times before opening first browser window -
[task 2020-05-15T21:25:51.223Z] 21:25:51 INFO - Buffered messages finished
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup_mainthreadio.js | unused whitelist entry before opening first browser window: Z:\task_1589575356\build\application\firefox\browser\extensions (XCurProcD:extensions) -
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - Stack trace:
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_startup_mainthreadio.js:null:863
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1064
[task 2020-05-15T21:25:51.224Z] 21:25:51 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1104
[task 2020-05-15T21:25:51.225Z] 21:25:51 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:927
[task 2020-05-15T21:25:51.225Z] 21:25:51 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:918
[task 2020-05-15T21:25:51.225Z] 21:25:51 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_mainthreadio.js | stat on Z:\task_1589575356\AppData\Roaming\Mozilla\Firefox allowed 1 more times before opening first browser window -

Comment 19

[Shane Caraveo (:mixedpuppy)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=91d0a0efbf4f779d0d9f79b14b2be04ee98c0a08

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/092d6f1c2993
add new system-extensions location for Normandy r=aswan

Comment 21

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/092d6f1c2993