commit 29023a6ade3da48075f4084350fd61067f405fbc
parent 84ad56baf96e2a6167ccb4e7d6ece2bde731a6db
Author: Harsheet <hsohaney@mozilla.com>
Date: Tue, 7 Oct 2025 20:21:47 +0000
Bug 1990517 - Show restore errors if any restore resource's fail. r=sthompson
Differential Revision: https://phabricator.services.mozilla.com/D266671
Diffstat:
6 files changed, 176 insertions(+), 116 deletions(-)
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -2684,6 +2684,7 @@ export class BackupService extends EventTarget {
} catch (_) {
lazy.logConsole.warn("Could not remove ", RECOVERY_FILE_DEST_PATH);
}
+
try {
// We're using a try/finally here to clean up the temporary OSKeyStore.
// We need to make sure that cleanup occurs _after_ the recovery has
@@ -2877,6 +2878,7 @@ export class BackupService extends EventTarget {
`Failed to recover resource: ${resourceKey}`,
e
);
+ throw e;
}
}
diff --git a/browser/components/backup/tests/xpcshell/head.js b/browser/components/backup/tests/xpcshell/head.js
@@ -4,41 +4,18 @@
"use strict";
-const { BackupService } = ChromeUtils.importESModule(
- "resource:///modules/backup/BackupService.sys.mjs"
-);
-
-const { BackupResource } = ChromeUtils.importESModule(
- "resource:///modules/backup/BackupResource.sys.mjs"
-);
-
-const { MeasurementUtils } = ChromeUtils.importESModule(
- "resource:///modules/backup/MeasurementUtils.sys.mjs"
-);
-
-const { TelemetryTestUtils } = ChromeUtils.importESModule(
- "resource://testing-common/TelemetryTestUtils.sys.mjs"
-);
-
-const { Sqlite } = ChromeUtils.importESModule(
- "resource://gre/modules/Sqlite.sys.mjs"
-);
-
-const { sinon } = ChromeUtils.importESModule(
- "resource://testing-common/Sinon.sys.mjs"
-);
-
-const { OSKeyStoreTestUtils } = ChromeUtils.importESModule(
- "resource://testing-common/OSKeyStoreTestUtils.sys.mjs"
-);
-
-const { MockRegistrar } = ChromeUtils.importESModule(
- "resource://testing-common/MockRegistrar.sys.mjs"
-);
-
-const { PrivateBrowsingUtils } = ChromeUtils.importESModule(
- "resource://gre/modules/PrivateBrowsingUtils.sys.mjs"
-);
+ChromeUtils.defineESModuleGetters(this, {
+ BackupService: "resource:///modules/backup/BackupService.sys.mjs",
+ BackupResource: "resource:///modules/backup/BackupResource.sys.mjs",
+ MeasurementUtils: "resource:///modules/backup/MeasurementUtils.sys.mjs",
+ TelemetryTestUtils: "resource://testing-common/TelemetryTestUtils.sys.mjs",
+ Sqlite: "resource://gre/modules/Sqlite.sys.mjs",
+ sinon: "resource://testing-common/Sinon.sys.mjs",
+ OSKeyStoreTestUtils: "resource://testing-common/OSKeyStoreTestUtils.sys.mjs",
+ MockRegistrar: "resource://testing-common/MockRegistrar.sys.mjs",
+ PrivateBrowsingUtils: "resource://gre/modules/PrivateBrowsingUtils.sys.mjs",
+ AppConstants: "resource://gre/modules/AppConstants.sys.mjs",
+});
const HISTORY_ENABLED_PREF = "places.history.enabled";
const SANITIZE_ON_SHUTDOWN_PREF = "privacy.sanitize.sanitizeOnShutdown";
@@ -349,6 +326,52 @@ function countHistogramMeasurements(histogram) {
return countsPerBucket.reduce((sum, count) => sum + count, 0);
}
+function setupProfile() {
+ // FOG needs to be initialized in order for data to flow.
+ Services.fog.initializeFOG();
+
+ // Much of this setup is copied from toolkit/profile/xpcshell/head.js. It is
+ // needed in order to put the xpcshell test environment into the state where
+ // it thinks its profile is the one pointed at by
+ // nsIToolkitProfileService.currentProfile.
+ let gProfD = do_get_profile();
+ let gDataHome = gProfD.clone();
+ gDataHome.append("data");
+ gDataHome.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+ let gDataHomeLocal = gProfD.clone();
+ gDataHomeLocal.append("local");
+ gDataHomeLocal.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+
+ let xreDirProvider = Cc["@mozilla.org/xre/directory-provider;1"].getService(
+ Ci.nsIXREDirProvider
+ );
+ xreDirProvider.setUserDataDirectory(gDataHome, false);
+ xreDirProvider.setUserDataDirectory(gDataHomeLocal, true);
+
+ let profileSvc = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
+ Ci.nsIToolkitProfileService
+ );
+
+ let createdProfile = {};
+ let didCreate = profileSvc.selectStartupProfile(
+ ["xpcshell"],
+ false,
+ AppConstants.UPDATE_CHANNEL,
+ "",
+ {},
+ {},
+ createdProfile
+ );
+ Assert.ok(didCreate, "Created a testing profile and set it to current.");
+ Assert.equal(
+ profileSvc.currentProfile,
+ createdProfile.value,
+ "Profile set to current"
+ );
+
+ return createdProfile.value;
+}
+
/**
* Asserts that a histogram received a certain number of measurements, regardless
* of the values of the measurements themselves.
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService.js b/browser/components/backup/tests/xpcshell/test_BackupService.js
@@ -28,49 +28,7 @@ const LAST_BACKUP_FILE_NAME_PREF_NAME =
let currentProfile;
add_setup(function () {
- // FOG needs to be initialized in order for data to flow.
- Services.fog.initializeFOG();
-
- // Much of this setup is copied from toolkit/profile/xpcshell/head.js. It is
- // needed in order to put the xpcshell test environment into the state where
- // it thinks its profile is the one pointed at by
- // nsIToolkitProfileService.currentProfile.
- let gProfD = do_get_profile();
- let gDataHome = gProfD.clone();
- gDataHome.append("data");
- gDataHome.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
- let gDataHomeLocal = gProfD.clone();
- gDataHomeLocal.append("local");
- gDataHomeLocal.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
-
- let xreDirProvider = Cc["@mozilla.org/xre/directory-provider;1"].getService(
- Ci.nsIXREDirProvider
- );
- xreDirProvider.setUserDataDirectory(gDataHome, false);
- xreDirProvider.setUserDataDirectory(gDataHomeLocal, true);
-
- let profileSvc = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
- Ci.nsIToolkitProfileService
- );
-
- let createdProfile = {};
- let didCreate = profileSvc.selectStartupProfile(
- ["xpcshell"],
- false,
- AppConstants.UPDATE_CHANNEL,
- "",
- {},
- {},
- createdProfile
- );
- Assert.ok(didCreate, "Created a testing profile and set it to current.");
- Assert.equal(
- profileSvc.currentProfile,
- createdProfile.value,
- "Profile set to current"
- );
-
- currentProfile = createdProfile.value;
+ currentProfile = setupProfile();
});
/**
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js b/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js
@@ -50,44 +50,7 @@ add_setup(async () => {
Services.prefs.setIntPref(BACKUP_RETRY_LIMIT_PREF_NAME, 2);
Services.prefs.setBoolPref(DISABLED_ON_IDLE_RETRY_PREF_NAME, false);
- // Much of this setup is copied from toolkit/profile/xpcshell/head.js. It is
- // needed in order to put the xpcshell test environment into the state where
- // it thinks its profile is the one pointed at by
- // nsIToolkitProfileService.currentProfile.
- let gProfD = do_get_profile();
- let gDataHome = gProfD.clone();
- gDataHome.append("data");
- gDataHome.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
- let gDataHomeLocal = gProfD.clone();
- gDataHomeLocal.append("local");
- gDataHomeLocal.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
-
- let xreDirProvider = Cc["@mozilla.org/xre/directory-provider;1"].getService(
- Ci.nsIXREDirProvider
- );
- xreDirProvider.setUserDataDirectory(gDataHome, false);
- xreDirProvider.setUserDataDirectory(gDataHomeLocal, true);
-
- let profileSvc = Cc["@mozilla.org/toolkit/profile-service;1"].getService(
- Ci.nsIToolkitProfileService
- );
-
- let createdProfile = {};
- let didCreate = profileSvc.selectStartupProfile(
- ["xpcshell"],
- false,
- AppConstants.UPDATE_CHANNEL,
- "",
- {},
- {},
- createdProfile
- );
- Assert.ok(didCreate, "Created a testing profile and set it to current.");
- Assert.equal(
- profileSvc.currentProfile,
- createdProfile.value,
- "Profile set to current"
- );
+ setupProfile();
registerCleanupFunction(async () => {
Services.prefs.clearUserPref(BACKUP_DEFAULT_LOCATION_PREF_NAME);
diff --git a/browser/components/backup/tests/xpcshell/test_ResourceFailures.js b/browser/components/backup/tests/xpcshell/test_ResourceFailures.js
@@ -0,0 +1,112 @@
+/* Any copyright is dedicated to the Public Domain.
+https://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+ChromeUtils.defineESModuleGetters(this, {
+ BackupError: "resource:///modules/backup/BackupError.mjs",
+ ERRORS: "chrome://browser/content/backup/backup-constants.mjs",
+ AppConstants: "resource://gre/modules/AppConstants.sys.mjs",
+});
+
+add_setup(function () {
+ setupProfile();
+});
+
+/**
+ * Test's the case where a resource fails to recover successfully. We expect
+ * the complete recover process to error out.
+ */
+add_task(async function testResourceFailure() {
+ let sandbox = sinon.createSandbox();
+ registerCleanupFunction(() => {
+ sandbox.restore();
+ });
+
+ let testBackupPath = await IOUtils.createUniqueDirectory(
+ PathUtils.tempDir,
+ "checkForErrorsTestBackup"
+ );
+
+ let fake1ManifestEntry = { fake1: "hello from 1" };
+ sandbox
+ .stub(FakeBackupResource1.prototype, "backup")
+ .resolves(fake1ManifestEntry);
+ sandbox.stub(FakeBackupResource1.prototype, "recover").resolves();
+
+ let fake2ManifestEntry = { fake1: "hello from 2" };
+ sandbox
+ .stub(FakeBackupResource2.prototype, "backup")
+ .resolves(fake2ManifestEntry);
+ sandbox
+ .stub(FakeBackupResource2.prototype, "recover")
+ .rejects(new BackupError("recovery failed", ERRORS.RECOVERY_FAILED));
+
+ let fake3ManifestEntry = { fake1: "hello from 3" };
+ sandbox
+ .stub(FakeBackupResource3.prototype, "backup")
+ .resolves(fake3ManifestEntry);
+ sandbox.stub(FakeBackupResource3.prototype, "recover").resolves();
+
+ let bs = new BackupService({
+ FakeBackupResource1,
+ FakeBackupResource2,
+ FakeBackupResource3,
+ });
+
+ let { manifest, archivePath: backupFilePath } = await bs.createBackup({
+ profilePath: testBackupPath,
+ });
+ Assert.ok(await IOUtils.exists(backupFilePath), "The backup file exists");
+
+ let archiveDateSuffix = bs.generateArchiveDateSuffix(
+ new Date(manifest.meta.date)
+ );
+
+ // We also expect the HTML file to have been written to the folder pointed
+ // at by browser.backups.location, within backupDirPath folder.
+ const EXPECTED_ARCHIVE_PATH = PathUtils.join(
+ bs.state.backupDirPath,
+ `${BackupService.BACKUP_FILE_NAME}_${manifest.meta.profileName}_${archiveDateSuffix}.html`
+ );
+ Assert.ok(
+ await IOUtils.exists(EXPECTED_ARCHIVE_PATH),
+ "Single-file backup archive was written."
+ );
+ Assert.equal(
+ backupFilePath,
+ EXPECTED_ARCHIVE_PATH,
+ "Backup was written to the configured destination folder"
+ );
+ Assert.deepEqual(
+ Object.keys(manifest.resources).sort(),
+ ["fake1", "fake2", "fake3"],
+ "Manifest contains all expected BackupResource keys"
+ );
+
+ let recoveredProfilePath = await IOUtils.createUniqueDirectory(
+ PathUtils.tempDir,
+ "createBackupTestRecoveredProfile"
+ );
+
+ await Assert.rejects(
+ bs.recoverFromBackupArchive(
+ backupFilePath,
+ null,
+ false,
+ testBackupPath,
+ recoveredProfilePath
+ ),
+ err => err.cause == ERRORS.RECOVERY_FAILED
+ );
+
+ // Since we fail on backupResource2, backupResource1 should never be called
+ sinon.assert.notCalled(FakeBackupResource1.prototype.recover);
+
+ // Ideally, we should be removing any left over data when we fail.
+ // The current behavior does not do this, so let's clear the paths
+ // manually in the test.
+ await maybeRemovePath(backupFilePath);
+ await maybeRemovePath(testBackupPath);
+ await maybeRemovePath(recoveredProfilePath);
+});
diff --git a/browser/components/backup/tests/xpcshell/xpcshell.toml b/browser/components/backup/tests/xpcshell/xpcshell.toml
@@ -77,6 +77,8 @@ run-sequentially = ["true"] # Mock Windows registry interferes with normal opera
["test_PreferencesBackupResource_searchEngines.js"]
+["test_ResourceFailures.js"]
+
["test_SessionStoreBackupResource.js"]
["test_backupService_findBackupsInWellKnownLocations.js"]
