1483399 - Add option in preferences to disable Background Update Agent

Status: RESOLVED FIXED

(Toolkit :: Application Update, enhancement, P2)

Description

[Kirk Steuber (he/him) [:bytesized]]

When the Background Update Agent ships, there should be an option in the update section of about:preferences that has a checkbox and reads something like "Install updates when Firefox is not running".

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: about:preferences on Windows

Comment 2

[Nick Alexander :nalexander [he/him]]

Attachment: about:preferences on macOS

Comment 3

[Nick Alexander :nalexander [he/him]]

Kirk and I had a long discussion about the following difficult problem. We want:

1. to have a toggle named app.update.background.enabled
2. to allow users to manage the toggle, preferably in about:preferences
3. to be able to control the toggle using a Firefox policy
4. to control the toggle using the existing Firefox mechanism for staged rollouts, namely Normandy
5. to have the toggle be per-installation on Windows in the manner of app.update.auto

Here I am using "toggle" to be clear that this is not (necessarily) a normal Firefox preference. I will refer to app.update.auto as a "per-installation (Firefox) pref(erence)".

A regular Firefox preference handles 1-4: that's the happy path. A per-installation pref like app.update.auto handles 1-3 and 5, but not 4. So the first question is, can we extend the per-installation pref mechanism to accommodate Normandy? After discussion with Kirk, I think we can.

First, let us recall how Normandy manages staged rollouts. Normandy leverages Firefox's multi-layer preference system whereby there is a "default" pref layer underneath the "user" pref layer. User prefs override default prefs. Normandy only every changes default prefs, allowing the user control. Thus we conclude that right now, Normandy cannot control the app.update.auto pref: any change to the default layer will not be reflected in the update-config.json file (technically, after any existing preference value has been migrated).

This suggests that we need to persist a parallel user-and-default preference structure for per-installation prefs. That is, we make app.update.background.enabled and DEFAULTS.app.update.background.enabled be per-installation prefs, and fallback appropriately. This will allow Normandy to control the underlying default while still giving the user control.

However, we will now expose multiple profiles and (potentially) multiple (Windows) OS-level users squabbling over the default per-installation pref. That is, Profile A may be enabled as a rollout is staged, while Profile B may not yet be enabled. (Two OS-level users may see the same situation.) Something must break ties; let us call the selection of the tie-breaking profile a "Normandy leader". There are many strategies for selecting a Normandy leader, but I propose that any profile that thinks itself the current default profile will control the per-installation default. This will result in two OS-level users fight over controlling the default pref, but that's acceptable: it means that if we need to stop a rollout, some OS-level user will witness the rollout change quickly.

This all seems achievable:

1. We make app.update.background.enabled and DEFAULTS.app.update.background.enabled be per-installation prefs;
2. Only a profile that matches defaultProfile will manage DEFAULTS.app.update.background.enabled.

Now, to UX (on Windows only at this point). Recall the screenshot in #c1. Right now we have:

Allow Nightly to:
( ) Automatically install updates
( ) Check for updates but let you choose to install them
(info) This setting will apply to all ...
--
[ ] Use a background service

Right now that final toggle is a regular pref and conceptually should be a per-installation pref, so let's remove it rather than fix it: Bug 1461755. I propose UX like:

Allow Nightly to:
( ) Automatically install updates
    [ ]  When Nightly is not running
( ) Check for updates but let you choose to install them
(info) This setting will apply to all ...

The radio button will control app.update.auto and the child checkbox will control app.update.background.enabled.

Finally, specific to app.update.background.enabled, there is another dimension that I will discuss here, although it's more related to Bug 1687777. The per-installation pref is not the only shared resource in play. The per-installation pref is managing a (Windows) Scheduled Task. With the approach we're taking (see Bug 1676296), these tasks are necessarily per-OS-level user. So we either share responsibility for the task among profiles, or we have the Normandy leader also exclusively manage the task. I'm strongly in favour of having the Normandy leader exclusively manage the task: this is a complicated piece of state that may depend on additional settings, say, logging levels and task interval cadences. If we make all such additional settings per-installation preferences we can't use about:config to configure them, leading to an explosion of compexity and UI. Instead, let's centralize such details in the selection of the Normandy leader (the current default profile). Very few users will ever see this.

Comment 4

[Nick Alexander :nalexander [he/him]]

bytesized: https://bugzilla.mozilla.org/show_bug.cgi?id=1483399#c3 is my summary (sort of) and conclusion (definitely) from our discussion Friday last. Does this accord with your recollection? Would you propose alternate solutions?

Comment 5

[Nick Alexander :nalexander [he/him]]

Now, to implementation. As soon as possible, I would like to add methods, similar to {get,set}AppUpdateAutoEnabled(...):

• {get,set}AppUpdateBackgroundEnabled(...)
• {get,set}DefaultBranchAppUpdateBackgroundEnabled(...)
  At first, these would be backed by regular per-profile preferences so that development of Bug 1687777 can proceed. Then this ticket would move them to use the same update-config.json mechanism as app.update.auto.

Comment 6

[Kirk Steuber (he/him) [:bytesized]]

Yeah, this all sounds about right, though I do have a few questions:

I'm strongly in favour of having the Normandy leader exclusively manage the task

I'm not really sure I understand why. It seems like this could result in a user scheduling background updates, but the setting not taking effect until some other user logs in. I don't feel that strongly about it though, so if you think this is the way to do it, I'm fine with it. As long as the task checks the setting and potentially bails out, we won't get problems with the reverse, where the user doesn't want background updates but gets them. So I think that's ok.

I propose UX like:

Allow Nightly to:
( ) Automatically install updates
    [ ]  When Nightly is not running
( ) Check for updates but let you choose to install them
(info) This setting will apply to all ...

Are we going to do that info box on every OS? Currently, that box exists only on Windows.

At first, these would be backed by regular per-profile preferences so that development of Bug 1687777 can proceed.

During this time, will we be exposing the preference in about:preferences, or not?

Comment 7

[Kirk Steuber (he/him) [:bytesized]]

I could also use some clarification of what work we would like done as part of this bug.

Comment 8

[Nick Alexander :nalexander [he/him]]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #6)

Yeah, this all sounds about right, though I do have a few questions:

I'm strongly in favour of having the Normandy leader exclusively manage the task

I'm not really sure I understand why. It seems like this could result in a user scheduling background updates, but the setting not taking effect until some other user logs in. I don't feel that strongly about it though, so if you think this is the way to do it, I'm fine with it. As long as the task checks the setting and potentially bails out, we won't get problems with the reverse, where the user doesn't want background updates but gets them. So I think that's ok.

Ah, I see one point that we don't have alignment on. I've been saying "Normandy leader", but there's not really one -- there's one "Normandy leader" per-OS-level user. So it's not exactly as you say, but it's similarly confusing: profile NOTDEFAULT could flip the switch with no immediate effect. If they then never run with profile DEFAULT again, they'll never schedule the task. Sad. We could make this smoother by having a non-default profile schedule the task in some way, perhaps by invoking a backgroundtask that would then read the default profile's preferences... but I doubt it's worth the effort.

In any case, your over-arching concern should be addressed: the task will check the per-installation background update settings (app.update.{auto,background.enabled}) and bail if either are false, preventing accidental background updates.

I propose UX like:

Allow Nightly to:
( ) Automatically install updates
    [ ]  When Nightly is not running
( ) Check for updates but let you choose to install them
(info) This setting will apply to all ...

Are we going to do that info box on every OS? Currently, that box exists only on Windows.

At first, no, since we're only targeting Windows. Eventually, yes, I hope so.

At first, these would be backed by regular per-profile preferences so that development of Bug 1687777 can proceed.

During this time, will we be exposing the preference in about:preferences, or not?

I think we would do so only if it's helpful while working on the UI changes.

Comment 9

[Nick Alexander :nalexander [he/him]]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #7)

I could also use some clarification of what work we would like done as part of this bug.

I think there are two main parts to this ticket:

1. generalize the existing app.update.auto mechanism to handle more per-installation prefs. This might be https://bugzilla.mozilla.org/show_bug.cgi?id=1483399#c5 or it might be more general. More general would trade many getters and setters for more general getPerInstallation{Bool,Char,Int,Float}Pref methods, and might store the per-installation prefs as a single JSON object rather than a bunch of fields.

2. the actual UI work, to add the sub-checkbox, to listen for additional auto-update-config-change messages, etc.

Part 1) should probably be spun off into a separate ticket, which I'll do now.

Comment 10

[Kirk Steuber (he/him) [:bytesized]]

I'm going to go ahead and add this interface. In order to keep it from being publicly visible before the background update agent is actually ready, it will be pref'ed off by app.update.background.experimental, for now. Unless that pref is set to true, this UI will not be shown.

Comment 11

[Kirk Steuber (he/him) [:bytesized]]

Attachment: Bug 1483399 - Add a UI to about:preferences to control app.update.background.enabled r=nalexander!,#preferences-reviewers!

The UI is currently hidden unless app.update.background.experimental is set to true. The preferences page must be reloaded after setting this pref.

Comment 12

[Kirk Steuber (he/him) [:bytesized]]

Attachment: Bug 1483399 - String changes for app.update.background.enabled interface in about:preferences r=nalexander!,#fluent-reviewers!,#preferences-reviewers!

Depends on D108705

Comment 13

[Kirk Steuber (he/him) [:bytesized]]

Attachment: Bug 1483399 - Add tests for the about:preferences UI for app.update.background.enabled r=nalexander!

Depends on D108706

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e190a6a0080b
Add a UI to about:preferences to control app.update.background.enabled r=nalexander,preferences-reviewers,Gijs
https://hg.mozilla.org/integration/autoland/rev/7ee6f14e0193
String changes for app.update.background.enabled interface in about:preferences r=nalexander,flod
https://hg.mozilla.org/integration/autoland/rev/b62179c04725
Add tests for the about:preferences UI for app.update.background.enabled r=nalexander

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/e190a6a0080b
https://hg.mozilla.org/mozilla-central/rev/7ee6f14e0193
https://hg.mozilla.org/mozilla-central/rev/b62179c04725