commit c9014d1918128ac900065d31c8f9f5b0e805ca9e
parent 4fd36a25bb8ca453b819ae0f3bf019ea7dece108
Author: Duncan McIntosh <dmcintosh@mozilla.com>
Date: Mon, 3 Nov 2025 21:53:03 +0000
Bug 1997389 - Part 1: Ensure that BackupService.#notifyStatusObservers is called, with correct 'this', when any disable reason changes. r=cdupuis
Differential Revision: https://phabricator.services.mozilla.com/D270896
Diffstat:
2 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -38,6 +38,7 @@ const DISABLED_ON_IDLE_RETRY_PREF_NAME =
const BACKUP_DEBUG_INFO_PREF_NAME = "browser.backup.backup-debug-info";
const MAXIMUM_NUMBER_OF_UNREMOVABLE_STAGING_ITEMS_PREF_NAME =
"browser.backup.max-num-unremovable-staging-items";
+const CREATED_MANAGED_PROFILES_PREF_NAME = "browser.profiles.created";
const SCHEMAS = Object.freeze({
BACKUP_MANIFEST: 1,
@@ -842,6 +843,12 @@ export class BackupService extends EventTarget {
#wasArchivePreviouslyDisabled = false;
/**
+ * Monitors prefs that are relevant to the status of the backup service.
+ * Unlike #observer, this does not wait for an idle tick.
+ */
+ #statusPrefObserver = null;
+
+ /**
* The path of the default parent directory for saving backups.
* The current default is the Documents directory.
*
@@ -988,6 +995,20 @@ export class BackupService extends EventTarget {
}
/**
+ * Prefs that should be monitored. When one of these prefs changes, the
+ * 'backup-service-status-changed' observers are notified and telemetry
+ * updates.
+ */
+ static get #STATUS_OBSERVER_PREFS() {
+ return [
+ BACKUP_ARCHIVE_ENABLED_PREF_NAME,
+ BACKUP_RESTORE_ENABLED_PREF_NAME,
+ "privacy.sanitize.sanitizeOnShutdown",
+ CREATED_MANAGED_PROFILES_PREF_NAME,
+ ];
+ }
+
+ /**
* Returns the schema for the schemaType for a given version.
*
* @param {number} schemaType
@@ -3873,35 +3894,28 @@ export class BackupService extends EventTarget {
#registerStatusObservers() {
// We don't use this.#observer since any changes to the prefs or nimbus should
- // immediately reflect across any observers, instead of waiting on idle
- Services.prefs.addObserver(
- BACKUP_ARCHIVE_ENABLED_PREF_NAME,
- this.#notifyStatusObservers
- );
- Services.prefs.addObserver(
- BACKUP_RESTORE_ENABLED_PREF_NAME,
- this.#notifyStatusObservers
- );
- Services.prefs.addObserver(
- "privacy.sanitize.sanitizeOnShutdown",
- this.#notifyStatusObservers
- );
- lazy.NimbusFeatures.backupService.onUpdate(this.#notifyStatusObservers);
+ // immediately reflect across any observers, instead of waiting on idle.
+ this.#statusPrefObserver = () => {
+ // Wrap in an arrow function so 'this' is preserved.
+ this.#notifyStatusObservers();
+ };
+
+ for (let pref of BackupService.#STATUS_OBSERVER_PREFS) {
+ Services.prefs.addObserver(pref, this.#statusPrefObserver);
+ }
+ lazy.NimbusFeatures.backupService.onUpdate(this.#statusPrefObserver);
}
#unregisterStatusObservers() {
- Services.prefs.removeObserver(
- BACKUP_ARCHIVE_ENABLED_PREF_NAME,
- this.#notifyStatusObservers
- );
- Services.prefs.removeObserver(
- BACKUP_RESTORE_ENABLED_PREF_NAME,
- this.#notifyStatusObservers
- );
- Services.prefs.removeObserver(
- "privacy.sanitize.sanitizeOnShutdown",
- this.#notifyStatusObservers
- );
+ if (this.#statusPrefObserver == null) {
+ return;
+ }
+
+ for (let pref of BackupService.#STATUS_OBSERVER_PREFS) {
+ Services.prefs.removeObserver(pref, this.#statusPrefObserver);
+ }
+ lazy.NimbusFeatures.backupService.offUpdate(this.#statusPrefObserver);
+ this.#statusPrefObserver = null;
}
/**
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_enabled.js b/browser/components/backup/tests/xpcshell/test_BackupService_enabled.js
@@ -154,7 +154,12 @@ add_task(async function test_restore_selectable_profiles() {
async function archiveTemplate({ internalReason, disable, enable }) {
const bs = BackupService.get();
+ 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");
Assert.ok(
!bs.archiveEnabledStatus.enabled,
@@ -181,6 +186,7 @@ async function archiveTemplate({ internalReason, disable, enable }) {
);
await enable();
+ Assert.equal(calledCount, 2, "Observers were notified on re-enable");
Assert.ok(
bs.archiveEnabledStatus.enabled,
@@ -205,6 +211,7 @@ async function archiveTemplate({ internalReason, disable, enable }) {
);
await IOUtils.remove(backup.archivePath);
+ Services.obs.removeObserver(callback, "backup-service-status-updated");
}
async function restoreTemplate({ internalReason, disable, enable }) {
@@ -216,7 +223,12 @@ async function restoreTemplate({ internalReason, disable, enable }) {
"Archive should have been created on disk."
);
+ 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");
const recoveryDir = await IOUtils.createUniqueDirectory(
PathUtils.profileDir,
@@ -246,6 +258,7 @@ async function restoreTemplate({ internalReason, disable, enable }) {
);
await enable();
+ Assert.equal(calledCount, 2, "Observers were notified on re-enable");
Assert.ok(
bs.restoreEnabledStatus.enabled,
@@ -267,4 +280,6 @@ async function restoreTemplate({ internalReason, disable, enable }) {
await IOUtils.exists(recoveredProfile.rootDir.path),
"Recovered profile directory should exist on disk."
);
+
+ Services.obs.removeObserver(callback, "backup-service-status-updated");
}
