commit b7eaa282c99c508970c9337b100db2504da6347a
parent 3ad2b7912f2365d03ba19552358576abd59521df
Author: Cristina Horotan <chorotan@mozilla.com>
Date: Fri, 7 Nov 2025 20:18:23 +0200
Revert "Bug 1997090 - reverse order of delete and create backup r=hsohaney" for causing xpcshell failures on test_BackupService_regeneration.js
This reverts commit 59c2bad9f0fb05ef777ff974e7bbac6fd622bafb.
Diffstat:
3 files changed, 43 insertions(+), 58 deletions(-)
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -1195,10 +1195,16 @@ export class BackupService extends EventTarget {
this.#postRecoveryResolver = resolve;
this.#backupWriteAbortController = new AbortController();
this.#regenerationDebouncer = new lazy.DeferredTask(async () => {
- if (!this.#backupWriteAbortController.signal.aborted) {
- await this.createBackupOnIdleDispatch({
- reason: "user deleted some data",
- });
+ if (
+ !this.#backupWriteAbortController.signal.aborted &&
+ this.archiveEnabledStatus.enabled
+ ) {
+ await this.deleteLastBackup();
+ if (lazy.scheduledBackupsPref) {
+ await this.createBackupOnIdleDispatch({
+ reason: "user deleted some data",
+ });
+ }
}
}, BackupService.REGENERATION_DEBOUNCE_RATE_MS);
}
@@ -4040,7 +4046,7 @@ export class BackupService extends EventTarget {
* not been sent to the application for at least
* IDLE_THRESHOLD_SECONDS_PREF_NAME seconds.
*/
- async onIdle() {
+ onIdle() {
lazy.logConsole.debug("Saw idle callback");
if (!this.#takenMeasurements) {
this.takeMeasurements();
@@ -4086,19 +4092,12 @@ export class BackupService extends EventTarget {
// loop in the parent process isn't so busy with higher priority things.
let expectedBackupTime =
lastBackupDate + lazy.minimumTimeBetweenBackupsSeconds;
- try {
- await this.createBackupOnIdleDispatch({
- reason:
- expectedBackupTime < this._startupTimeUnixSeconds
- ? "missed"
- : "idle",
- });
- } catch (e) {
- lazy.logConsole.error(
- "createBackupOnIdleDispatch promise rejected",
- e
- );
- }
+ this.createBackupOnIdleDispatch({
+ reason:
+ expectedBackupTime < this._startupTimeUnixSeconds
+ ? "missed"
+ : "idle",
+ });
} else {
lazy.logConsole.debug(
"Last backup was too recent. Not creating one for now."
@@ -4122,11 +4121,10 @@ export class BackupService extends EventTarget {
* is not busy with higher priority events. This is intentionally broken out
* into its own method to make it easier to stub out in tests.
*
- * @param {object} [options]
- * @param {boolean} [options.deletePreviousBackup]
- * @param {string} [options.reason]
+ * @param {...*} args
+ * Arguments to pass through to createBackup.
*/
- createBackupOnIdleDispatch({ deletePreviousBackup = true, reason }) {
+ createBackupOnIdleDispatch(...args) {
let now = Math.floor(Date.now() / 1000);
let errorStateDebugInfo = Services.prefs.getStringPref(
BACKUP_DEBUG_INFO_PREF_NAME,
@@ -4145,27 +4143,15 @@ export class BackupService extends EventTarget {
lazy.logConsole.debug(
`We've already retried in the last ${lazy.minimumTimeBetweenBackupsSeconds}s. Waiting for next valid idleDispatch to try again.`
);
- return Promise.resolve();
+ return;
}
- // Determine path to old backup file
- const oldBackupFile = this.#_state.lastBackupFileName;
- const isScheduledBackupsEnabled = lazy.scheduledBackupsPref;
- let { backupPromise, resolve } = Promise.withResolvers();
- ChromeUtils.idleDispatch(async () => {
+ ChromeUtils.idleDispatch(() => {
lazy.logConsole.debug(
"idleDispatch fired. Attempting to create a backup."
);
- let oldBackupFilePath;
- if (await this.#infalliblePathExists(lazy.backupDirPref)) {
- oldBackupFilePath = PathUtils.join(lazy.backupDirPref, oldBackupFile);
- }
- try {
- if (isScheduledBackupsEnabled) {
- await this.createBackup({ reason });
- }
- } catch (e) {
+ this.createBackup(...args).catch(e => {
lazy.logConsole.debug(
`There was an error creating backup on idle dispatch: ${e}`
);
@@ -4176,22 +4162,8 @@ export class BackupService extends EventTarget {
Services.prefs.setBoolPref(DISABLED_ON_IDLE_RETRY_PREF_NAME, true);
Glean.browserBackup.backupThrottled.record();
}
- } finally {
- // Now delete the old backup file, if it exists
- if (deletePreviousBackup && oldBackupFilePath) {
- lazy.logConsole.log(
- "Attempting to delete last backup file at ",
- oldBackupFilePath
- );
- await IOUtils.remove(oldBackupFilePath, {
- ignoreAbsent: true,
- retryReadonly: true,
- });
- resolve();
- }
- }
+ });
});
- return backupPromise;
}
/**
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_regeneration.js b/browser/components/backup/tests/xpcshell/test_BackupService_regeneration.js
@@ -129,8 +129,16 @@ async function expectRegeneration(taskFn, msg) {
let bs = new BackupService();
- // Now we set up a stub on the BackupService to detect calls to
- // createbackupOnIdleDispatch
+ // Now we set up some stubs on the BackupService to detect calls to
+ // deleteLastBackup and createbackupOnIdleDispatch, which are both called
+ // on regeneration.
+ let deleteDeferred = Promise.withResolvers();
+ sandbox.stub(bs, "deleteLastBackup").callsFake(() => {
+ Assert.ok(true, "Saw deleteLastBackup call");
+ deleteDeferred.resolve();
+ return Promise.resolve();
+ });
+
let createBackupDeferred = Promise.withResolvers();
sandbox.stub(bs, "createBackupOnIdleDispatch").callsFake(options => {
Assert.ok(true, "Saw createBackupOnIdleDispatch call");
@@ -154,6 +162,11 @@ async function expectRegeneration(taskFn, msg) {
await taskFn();
+ let regenerationPromises = [
+ deleteDeferred.promise,
+ createBackupDeferred.promise,
+ ];
+
// We'll wait for 1 second before considering the regeneration a bust.
let timeoutPromise = new Promise((resolve, reject) =>
// eslint-disable-next-line mozilla/no-arbitrary-setTimeout
@@ -163,7 +176,7 @@ async function expectRegeneration(taskFn, msg) {
);
try {
- await Promise.race([createBackupDeferred.promise, timeoutPromise]);
+ await Promise.race([Promise.all(regenerationPromises), timeoutPromise]);
Assert.ok(true, msg);
} catch (e) {
Assert.ok(false, "Timed out waiting for regeneration.");
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js b/browser/components/backup/tests/xpcshell/test_BackupService_retryHeuristic.js
@@ -81,7 +81,7 @@ add_task(async function test_retry_limit() {
// ensure that there is no error code set
Services.prefs.setIntPref(BACKUP_ERROR_CODE_PREF_NAME, ERRORS.NONE);
- bs.createBackupOnIdleDispatch({});
+ bs.createBackupOnIdleDispatch();
// #backupInProgress is set to true
await bsInProgressStateUpdate(bs, true);
@@ -121,7 +121,7 @@ add_task(async function test_retry_limit() {
// check if it switched to no longer creating backups on idle
const previousCalls = bs.createBackup.callCount;
- bs.createBackupOnIdleDispatch({});
+ bs.createBackupOnIdleDispatch();
// wait a tick for the pref to update
await new Promise(executeSoon);
@@ -148,7 +148,7 @@ add_task(async function test_retry_limit() {
// eslint-disable-next-line mozilla/no-arbitrary-setTimeout
await new Promise(resolve => setTimeout(resolve, 10));
- bs.createBackupOnIdleDispatch({});
+ bs.createBackupOnIdleDispatch();
// #backupInProgress is set to true
await bsInProgressStateUpdate(bs, true);
