1598223 - Land the DoH Rollout add-on into browser/extensions

Status: RESOLVED FIXED

(Firefox :: Security, task, P1)

Description

[Maxx Crawford [:maxx]]

Add DoH Rollout add-on to the browser/extensions folder, along with a moz.build file.

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Import doh-rollout src from github into browser/extensions. r=mixedpuppy!,maxxcrawford,bdanforth

Based on upstream https://github.com/mozilla/doh-rollout/commit/0bb6e992f70fa90c7c70b0529038a34d099a286d

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Fix issues caught by eslint in initial import. r=mixedpuppy!,maxxcrawford,bdanforth

Depends on D54617

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Add moz.build. r=mixedpuppy!,maxxcrawford,bdanforth

Depends on D54618

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Remove Normandy study remnants. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54619

Comment 5

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Pass an object to ExtensionPreferencesManager.getSettingsAPI instead of split arguments. r=mixedpuppy!,maxxcrawford,bdanforth

Depends on D54620

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Fix scoping issues with imports in WEE api scripts. r=mixedpuppy!,maxxcrawford,bdanforth

Depends on D54621

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Clean up experiment APIs and schemas. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54622

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Clean up background scripts. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54623

Comment 9

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Replace netChange WEE with networkStatus API. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54624

Comment 10

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Clean up and fix captive portal logic. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54625

Comment 11

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Properly abort at init if disabled. r=mixedpuppy!,maxxcrawford!,bdanforth

Depends on D54626

Comment 12

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1598223 - Change to a settings-style api for preferences

Draft of moving to settings api for preferences. Not necessary to land system extension, but would be nice. Prefer someone take over and test.

Comment 13

[Andrew Swan [:aswan]]

Adding a system add-on requires coordinating with the gofaster folks to ensure that system addon update checks work correctly:
http://firefox-source-docs.mozilla.org/toolkit/mozapps/extensions/addon-manager/SystemAddons.html

Do you expect to ship updates to this? If not, does it need to be an extension at all? Most of the code here appears to run with chrome privileges, can this just be a jsm started from BrowserGlue.jsm?

Comment 14

[(inactive) Nihanth Subramanya [:nhnt11]]

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

Adding a system add-on requires coordinating with the gofaster folks to ensure that system addon update checks work correctly:
http://firefox-source-docs.mozilla.org/toolkit/mozapps/extensions/addon-manager/SystemAddons.html

Thanks for the link! I'll do the due diligence.

Do you expect to ship updates to this? If not, does it need to be an extension at all? Most of the code here appears to run with chrome privileges, can this just be a jsm started from BrowserGlue.jsm?

This is the first step of a transition plan, the plan is to have a JSM at the end of the road. For now we are landing the add-on in-tree and moving development over to m-c from Github. This will be converted to a JSM asap (depending on prioritization).

Comment 15

[(inactive) Nihanth Subramanya [:nhnt11]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fd5b8ccf27f4869d1a03ab1c86fe1f10e2a2f7a9
https://treeherder.mozilla.org/#/jobs?repo=try&revision=933b1c282b5d448040f793f8c71a593d232e723e
https://treeherder.mozilla.org/#/jobs?repo=try&revision=be300380bbe3a56114e824581728e878f891a7d2

Comment 16

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Remove en_CA and en_GB strings - identical to en_US for now. r=maxxcrawford!

Comment 17

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Move default and base CSP definitions for WebExtensions into StaticPrefList.yaml. r=mconley!,zombie!

Comment 18

[(inactive) Nihanth Subramanya [:nhnt11]]

Try revealed that browser_preferences_usage.js fails due to an extra 4 (on my local macOS) accesses of the pref extensions.webextensions.default-content-security-policy with my patches. Talked to mconley and came up with two workarounds:

1. Use StaticPrefs for the webextension CSP prefs (try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf93be87c9937bea84644c0c253d23863b77a834)
2. Bump the max allowed accesses of this pref in the test to 51 (try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9db8ecbdd3cccdf218e0af72f5ba08c240ca41b)

Comment 19

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Bump max startup accesses of webextension default CSP pref to 51. r=mconley!

Comment 20

[(inactive) Nihanth Subramanya [:nhnt11]]

Static prefs route doesn't really work, can't add mirrored String prefs it seems.

Try push including talos jobs for bumping the max allowed accesses of this pref in the test to 51: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f110cb6744f61a1f5d0c8f346e2cd67340cc5f7

Comment 21

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Set strict_min_version to 72.0a1. r=maxxcrawford!

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/289f65edbaea
Import doh-rollout src from github into browser/extensions. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/ba3625505aa9
Fix issues caught by eslint in initial import. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/2e01f0052e3f
Add moz.build. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/e7630c5ce7de
Remove Normandy study remnants. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/06baa550f86e
Pass an object to ExtensionPreferencesManager.getSettingsAPI instead of split arguments. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/a076a3ddde96
Fix scoping issues with imports in WEE api scripts. r=mixedpuppy
https://hg.mozilla.org/integration/autoland/rev/7cc7fe778ae7
Clean up experiment APIs and schemas. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/228ccf0ee453
Clean up background scripts. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/b7bc6d796b8c
Replace netChange WEE with networkStatus API. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/c82e86c51e6e
Clean up and fix captive portal logic. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/0878e4bac240
Properly abort at init if disabled. r=mixedpuppy,maxxcrawford
https://hg.mozilla.org/integration/autoland/rev/28ce51bf535d
Remove en_CA and en_GB strings - identical to en_US for now. r=maxxcrawford,flod
https://hg.mozilla.org/integration/autoland/rev/0f1d580092b7
Bump max startup accesses of webextension default CSP pref to 51. r=mconley
https://hg.mozilla.org/integration/autoland/rev/3fcb7b24eef8
Set strict_min_version to 72.0a1. r=maxxcrawford

Comment 23

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1598223 - Bump version to 1.3.0. r=maxxcrawford!

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3eca0e7441e2
Bump version to 1.3.0. r=rhelmer

Comment 25

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/289f65edbaea
https://hg.mozilla.org/mozilla-central/rev/ba3625505aa9
https://hg.mozilla.org/mozilla-central/rev/2e01f0052e3f
https://hg.mozilla.org/mozilla-central/rev/e7630c5ce7de
https://hg.mozilla.org/mozilla-central/rev/06baa550f86e
https://hg.mozilla.org/mozilla-central/rev/a076a3ddde96
https://hg.mozilla.org/mozilla-central/rev/7cc7fe778ae7
https://hg.mozilla.org/mozilla-central/rev/228ccf0ee453
https://hg.mozilla.org/mozilla-central/rev/b7bc6d796b8c
https://hg.mozilla.org/mozilla-central/rev/c82e86c51e6e
https://hg.mozilla.org/mozilla-central/rev/0878e4bac240
https://hg.mozilla.org/mozilla-central/rev/28ce51bf535d
https://hg.mozilla.org/mozilla-central/rev/0f1d580092b7
https://hg.mozilla.org/mozilla-central/rev/3fcb7b24eef8
https://hg.mozilla.org/mozilla-central/rev/3eca0e7441e2

Comment 26

[Andrew Swan [:aswan]]

(In reply to Nihanth Subramanya [:nhnt11] from comment #14)

Do you expect to ship updates to this? If not, does it need to be an extension at all? Most of the code here appears to run with chrome privileges, can this just be a jsm started from BrowserGlue.jsm?

This is the first step of a transition plan, the plan is to have a JSM at the end of the road. For now we are landing the add-on in-tree and moving development over to m-c from Github. This will be converted to a JSM asap (depending on prioritization).

Is there a follow-up bug for this effort?

Comment 27

[(inactive) Nihanth Subramanya [:nhnt11]]

There wasn't, but I filed bug 1603779. We're targeting 75 for this change. Thanks for following up on this!