commit a47dfff9a41b8c0e730eb3d9895e5e51b26930e2
parent c9014d1918128ac900065d31c8f9f5b0e805ca9e
Author: Duncan McIntosh <dmcintosh@mozilla.com>
Date: Mon, 3 Nov 2025 21:53:03 +0000
Bug 1997389 - Part 2: Record new browser.backup.archiveEnabled and browser.backup.restoreEnabled metrics, along with restore reasons. r=bytesized,toolkit-telemetry-reviewers,cdupuis
Differential Revision: https://phabricator.services.mozilla.com/D270898
Diffstat:
4 files changed, 181 insertions(+), 62 deletions(-)
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -835,7 +835,7 @@ export class BackupService extends EventTarget {
/**
* Stores whether backing up has been disabled at some point during this
- * session. If it has been, the backupDisabledReason telemetry metric is set
+ * session. If it has been, the archiveDisabledReason telemetry metric is set
* on each backup. (It cannot be unset due to Glean limitations.)
*
* @type {boolean}
@@ -843,6 +843,15 @@ export class BackupService extends EventTarget {
#wasArchivePreviouslyDisabled = false;
/**
+ * Stores whether restoring up has been disabled at some point during this
+ * session. If it has been, the restoreDisabledReason telemetry metric is set
+ * on each backup. (It cannot be unset due to Glean limitations.)
+ *
+ * @type {boolean}
+ */
+ #wasRestorePreviouslyDisabled = false;
+
+ /**
* Monitors prefs that are relevant to the status of the backup service.
* Unlike #observer, this does not wait for an idle tick.
*/
@@ -1170,12 +1179,7 @@ export class BackupService extends EventTarget {
}
}, BackupService.REGENERATION_DEBOUNCE_RATE_MS);
- let backupStatus = this.archiveEnabledStatus;
- if (!backupStatus.enabled) {
- let internalReason = backupStatus.internalReason;
- this.#wasArchivePreviouslyDisabled = true;
- Glean.browserBackup.backupDisabledReason.set(internalReason);
- }
+ this.#notifyStatusObservers();
}
/**
@@ -1467,11 +1471,7 @@ export class BackupService extends EventTarget {
let status = this.archiveEnabledStatus;
if (!status.enabled) {
lazy.logConsole.debug(status.reason);
- this.#wasArchivePreviouslyDisabled = true;
- Glean.browserBackup.backupDisabledReason.set(status.internalReason);
return null;
- } else if (this.#wasArchivePreviouslyDisabled) {
- Glean.browserBackup.backupDisabledReason.set("reenabled");
}
// createBackup does not allow re-entry or concurrent backups.
@@ -3919,11 +3919,30 @@ export class BackupService extends EventTarget {
}
/**
- * Notify any listeners about the availability of the backup service.
+ * Notify any listeners about the availability of the backup service, then
+ * update relevant telemetry metrics.
*/
- #notifyStatusObservers = () => {
+ #notifyStatusObservers() {
Services.obs.notifyObservers(null, "backup-service-status-updated");
- };
+
+ let status = this.archiveEnabledStatus;
+ Glean.browserBackup.archiveEnabled.set(status.enabled);
+ if (!status.enabled) {
+ this.#wasArchivePreviouslyDisabled = true;
+ Glean.browserBackup.archiveDisabledReason.set(status.internalReason);
+ } else if (this.#wasArchivePreviouslyDisabled) {
+ Glean.browserBackup.archiveDisabledReason.set("reenabled");
+ }
+
+ status = this.restoreEnabledStatus;
+ Glean.browserBackup.restoreEnabled.set(status.enabled);
+ if (!status.enabled) {
+ this.#wasRestorePreviouslyDisabled = true;
+ Glean.browserBackup.restoreDisabledReason.set(status.internalReason);
+ } else if (this.#wasRestorePreviouslyDisabled) {
+ Glean.browserBackup.restoreDisabledReason.set("reenabled");
+ }
+ }
/**
* Called when the last known backup should be deleted and a new one
diff --git a/browser/components/backup/metrics.yaml b/browser/components/backup/metrics.yaml
@@ -14,7 +14,10 @@ browser.backup:
enabled:
type: boolean
description: >
- True if the BackupService is enabled by default.
+ True if the BackupService has initialized and reached idle. You may want
+ to use archive_enabled/restore_enabled to determine whether those
+ features are available, or use scheduler_enabled for whether automatic
+ backups are enabled.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1908727
data_reviews:
@@ -26,6 +29,38 @@ browser.backup:
expires: never
telemetry_mirror: BROWSER_BACKUP_ENABLED
+ archive_enabled:
+ type: boolean
+ description: >
+ True if the user can create backups, i.e. it has not been disabled by a
+ pref or otherwise deemed incompatible.
+ bugs:
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1997389
+ data_reviews:
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1997389
+ data_sensitivity:
+ - technical
+ notification_emails:
+ - drubino@mozilla.com
+ expires: never
+ telemetry_mirror: BROWSER_BACKUP_ARCHIVE_ENABLED
+
+ restore_enabled:
+ type: boolean
+ description: >
+ True if the user can restore backups, i.e. it has not been disabled by a
+ pref or otherwise deemed incompatible.
+ bugs:
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1997389
+ data_reviews:
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1997389
+ data_sensitivity:
+ - technical
+ notification_emails:
+ - drubino@mozilla.com
+ expires: never
+ telemetry_mirror: BROWSER_BACKUP_RESTORE_ENABLED
+
scheduler_enabled:
type: boolean
description: >
@@ -676,7 +711,23 @@ browser.backup:
- drubino@mozilla.com
expires: never
- backup_disabled_reason:
+ archive_disabled_reason:
+ type: string
+ description: >
+ Only set if `browser.backup.enabled` is `false`. Possible reasons are
+ "nimbus", "pref" (non-Nimbus), "policy", "sanitizeOnShutdown",
+ "selectable profiles".
+ bugs:
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1992808
+ data_reviews:
+ - https://phabricator.services.mozilla.com/D268774
+ data_sensitivity:
+ - technical
+ notification_emails:
+ - drubino@mozilla.com
+ expires: never
+
+ restore_disabled_reason:
type: string
description: >
Only set if `browser.backup.enabled` is `false`. Possible reasons are
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_enabled.js b/browser/components/backup/tests/xpcshell/test_BackupService_enabled.js
@@ -152,46 +152,34 @@ add_task(async function test_restore_selectable_profiles() {
});
async function archiveTemplate({ internalReason, disable, enable }) {
+ Services.telemetry.clearScalars();
+ Services.fog.testResetFOG();
+
const bs = BackupService.get();
+ Assert.equal(
+ bs.archiveEnabledStatus.enabled,
+ true,
+ "BackupService is enabled at the start"
+ );
+
let calledCount = 0;
let callback = () => calledCount++;
Services.obs.addObserver(callback, "backup-service-status-updated");
await disable();
Assert.equal(calledCount, 1, "Observers were notified on disable");
+ assertStatus("archive", bs.archiveEnabledStatus, false, internalReason);
- Assert.ok(
- !bs.archiveEnabledStatus.enabled,
- "The backup service should report that archiving is now disabled."
- );
-
- Assert.equal(
- bs.archiveEnabledStatus.internalReason,
- internalReason,
- "`archiveEnabledStatus` should report that it is disabled."
- );
-
- Services.fog.testResetFOG();
let backup = await bs.createBackup();
Assert.ok(
!backup,
"Creating a backup should fail when archiving is disabled."
);
- let telemetry = Glean.browserBackup.backupDisabledReason.testGetValue();
- Assert.equal(
- telemetry,
- internalReason,
- `Telemetry identifies the backup is disabled by ${internalReason}.`
- );
await enable();
Assert.equal(calledCount, 2, "Observers were notified on re-enable");
-
- Assert.ok(
- bs.archiveEnabledStatus.enabled,
- "The backup service should report that archiving is enabled once the archive killswitch experiment ends."
- );
+ assertStatus("archive", bs.archiveEnabledStatus, true, "reenabled");
backup = await bs.createBackup();
ok(
@@ -203,18 +191,14 @@ async function archiveTemplate({ internalReason, disable, enable }) {
"Archive file should exist on disk."
);
- telemetry = Glean.browserBackup.backupDisabledReason.testGetValue();
- Assert.equal(
- telemetry,
- "reenabled",
- "Telemetry identifies the backup was re-enabled."
- );
-
await IOUtils.remove(backup.archivePath);
Services.obs.removeObserver(callback, "backup-service-status-updated");
}
async function restoreTemplate({ internalReason, disable, enable }) {
+ Services.telemetry.clearScalars();
+ Services.fog.testResetFOG();
+
const bs = BackupService.get();
const backup = await bs.createBackup();
@@ -229,22 +213,13 @@ async function restoreTemplate({ internalReason, disable, enable }) {
await disable();
Assert.equal(calledCount, 1, "Observers were notified on disable");
+ assertStatus("restore", bs.restoreEnabledStatus, false, internalReason);
const recoveryDir = await IOUtils.createUniqueDirectory(
PathUtils.profileDir,
"recovered-profiles"
);
- Assert.ok(
- !bs.restoreEnabledStatus.enabled,
- "The backup service should report that restoring is now disabled."
- );
- Assert.equal(
- bs.restoreEnabledStatus.internalReason,
- internalReason,
- "`archiveEnabledStatus` should report that it is disabled."
- );
-
await Assert.rejects(
bs.recoverFromBackupArchive(
backup.archivePath,
@@ -259,11 +234,7 @@ async function restoreTemplate({ internalReason, disable, enable }) {
await enable();
Assert.equal(calledCount, 2, "Observers were notified on re-enable");
-
- Assert.ok(
- bs.restoreEnabledStatus.enabled,
- "The backup service should now report that restoring is enabled."
- );
+ assertStatus("restore", bs.restoreEnabledStatus, true, "reenabled");
let recoveredProfile = await bs.recoverFromBackupArchive(
backup.archivePath,
@@ -283,3 +254,48 @@ async function restoreTemplate({ internalReason, disable, enable }) {
Services.obs.removeObserver(callback, "backup-service-status-updated");
}
+
+/**
+ * Checks that the status object matches the expected values, and that the
+ * telemetry matches the object.
+ *
+ * @param {string} kind
+ * The kind of status object.
+ * @param {string} status
+ * The status object given.
+ * @param {boolean} enabled
+ * Whether the feature should be enabled or disabled.
+ * @param {string?} internalReason
+ * The internal reason that should be given.
+ */
+function assertStatus(kind, status, enabled, internalReason) {
+ Assert.equal(
+ status.enabled,
+ enabled,
+ `${kind} status is ${enabled ? "" : "not "}enabled.`
+ );
+ Assert.equal(
+ status.internalReason ?? null,
+ // 'reenabled' is only for telemetry, the status is null
+ internalReason == "reenabled" ? null : internalReason,
+ `${kind} status has the expected internal reason.`
+ );
+
+ Assert.equal(
+ Glean.browserBackup[kind + "Enabled"].testGetValue(),
+ enabled,
+ `Glean ${kind}_enabled metric should be ${enabled}.`
+ );
+ TelemetryTestUtils.assertScalar(
+ TelemetryTestUtils.getProcessScalars("parent", false, true),
+ `browser.backup.${kind}_enabled`,
+ enabled,
+ `Legacy ${kind}_enabled metric should be ${enabled}.`
+ );
+
+ Assert.equal(
+ Glean.browserBackup[kind + "DisabledReason"].testGetValue(),
+ internalReason,
+ `Glean ${kind}_disabled_reason metric is ${internalReason}.`
+ );
+}
diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml
@@ -377,7 +377,10 @@ browser.backup:
bug_numbers:
- 1908727
description: >
- True if the BackupService is enabled by default.
+ True if the BackupService has initialized and reached idle. You may want
+ to use archive_enabled/restore_enabled to determine whether those
+ features are available, or use scheduler_enabled for whether automatic
+ backups are enabled.
expires: never
kind: boolean
notification_emails:
@@ -387,6 +390,36 @@ browser.backup:
- 'firefox'
record_in_processes:
- 'main'
+ archive_enabled:
+ bug_numbers:
+ - 1908727
+ description: >
+ True if the user can create backups, i.e. it has not been disabled by a
+ pref or otherwise deemed incompatible.
+ expires: never
+ kind: boolean
+ notification_emails:
+ - drubino@mozilla.com
+ release_channel_collection: opt-out
+ products:
+ - 'firefox'
+ record_in_processes:
+ - 'main'
+ restore_enabled:
+ bug_numbers:
+ - 1908727
+ description: >
+ True if the user can restore backups, i.e. it has not been disabled by a
+ pref or otherwise deemed incompatible.
+ expires: never
+ kind: boolean
+ notification_emails:
+ - drubino@mozilla.com
+ release_channel_collection: opt-out
+ products:
+ - 'firefox'
+ record_in_processes:
+ - 'main'
scheduler_enabled:
bug_numbers:
- 1908727
