1691486 - Generalize `app.update.auto` to more per-installation preferences

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement, P1)

Description

[Nick Alexander :nalexander [he/him]]

Right now, app.update.auto is handled specially. Per the discussion in Bug 1483399, we want to add some additional per-installation prefs. This ticket tracks generalizing the per-installation prefs mechanism to handle additional prefs (e.g., app.update.background.enabled) and default pref branches (needed for Normandy support).

Comment 1

[Nick Alexander :nalexander [he/him]]

This might also be a pre-cursor to Bug 1657533.

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1691486 - Part 1: Generalize `auto-update-config-changed` to `app-update-config-changed`. r?bytesized!,chutten!,#preferences-reviewers!

This renames the notification and changes the data from a boolean
encoded as a string (i.e., "true" or "false") to a plain object like
{"app.update.auto": true}. This paves the way to adding additional
keys.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1691486 - Part 2: Extract whether per-installation prefs are supported. r?bytesized!

This just rearranges some deck chairs, but it will make it easy to
flip this on for macOS, when the time comes, and helps when developing
under macOS.

Depends on D104900

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1691486 - Part 3: Generalize "app.update.auto" to arbitrary per-installation prefs. r?bytesized!

This is mostly a mechanical transition to a data-driven expression.
We introduce a data structure allowing to capture variation across
per-installation prefs; update the existing code to use this data
definition; and then pass through to maintain the existing API
surface.

The interesting wrinkles are around caching and the persistence layer.
Since we can now have multiple values, we need to initialize the
cached values more carefully. We make no attempt to witness other
processes modifying our installation's config file except when
explicitly reading a per-installation pref. This both avoids
technical complexity but also keeps a simple model for consumers,
where the state read does not "shift" underneath us before amended
state is written. It is expected that concurrent modification will be
extremely infrequent, and that the last-writer-wins model will accord
with user expectation.

Depends on D104901

Comment 5

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1691486 - Generalize update config JSON API so it will be usable with other prefs r=nalexander!

Currently, we only support reading and writing app.update.auto to the update config JSON. We want to be able to support multiple installation-specific prefs.

This patch also adds additional features to this pref API that will be useful for the other prefs that we plan to add:

• A default preference branch with behavior mimicking that of the existing preference system.
• Multiple types of prefs including boolean, string, and integer.
• Sticky Prefs.
• Expandable: More prefs can be added with little effort.

This also maintains previous functionality. app.update.auto migration is maintained. This shouldn't strictly be necessary, since a watershed has occurred since migration. But it is possible for someone to use a really old profile with a new installation, so I thought it would be nice to maintain it for now.

Existing functionality for app.update.auto is now also made available to new prefs, including:

• Ability to be overridden by policy.
• Observer notifications on pref value changes.
• Fallback to the existing pref system on unsupported platforms.

Comment 6

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1691486 - Change tests that manually read the config JSON r=nalexander!

Some tests manually open the config JSON and verify their contents. We shouldn't do this anymore now that the structure of this JSON file has become somewhat more complicated.

Depends on D106996

Comment 7

[Robin Steuber (they/them) [:bytesized]]

Attachment: Bug 1691486 - Add testing for the new features of per-installation prefs r=nalexander!

Note that this feature is also covered by these existing tests:
browser/components/enterprisepolicies/tests/browser/browser_policy_app_auto_update.js
toolkit/mozapps/update/tests/browser/browser_aboutPrefs_settings.js
toolkit/mozapps/update/tests/unit_aus_update/updateAutoPrefMigrate.js

Depends on D106997

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17587aa3ec61
Generalize update config JSON API so it will be usable with other prefs r=nalexander
https://hg.mozilla.org/integration/autoland/rev/5fdcaae015bf
Change tests that manually read the config JSON r=nalexander,application-update-reviewers
https://hg.mozilla.org/integration/autoland/rev/6ea4d69aa5c6
Add testing for the new features of per-installation prefs r=nalexander,application-update-reviewers

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/17587aa3ec61
https://hg.mozilla.org/mozilla-central/rev/5fdcaae015bf
https://hg.mozilla.org/mozilla-central/rev/6ea4d69aa5c6