Bug 1886614 - Write a marionette test for the BackupService.createBackup method. r=backup-reviewers,kpatenio

This test can be run via:

./mach marionette-test browser/components/backup/test/marionette/test_backup.py

This also makes it so that BackupResource.copySqliteDatabases does not throw if
one of the passed in database files doesn't exist - it'll just ignore it and
move on. This required me to update some tests in order to create some fake
SQLite databases to ensure that the fake Sqlite connection was made and the
backup stub was called for the SQLite databases being copied.

Finally, this also fixes something I noticed - some of our BackupResource's
weren't returning null or objects as their ManifestEntry. This fixes that
and adds tests to cover that case.

Differential Revision: https://phabricator.services.mozilla.com/D207811

browser/components/backup/BackupService.sys.mjs

@@ -146,6 +146,12 @@ export class BackupService {
     }
   }
 
+  /**
+   * @typedef {object} CreateBackupResult
+   * @property {string} stagingPath
+   *   The staging path for where the backup was created.
+   */
+
   /**
    * Create a backup of the user's profile.
    *
@@ -154,13 +160,15 @@ export class BackupService {
    * @param {string} [options.profilePath=PathUtils.profileDir]
    *   The path to the profile to backup. By default, this is the current
    *   profile.
-   * @returns {Promise<undefined>}
+   * @returns {Promise<CreateBackupResult|null>}
+   *   A promise that resolves to an object containing the path to the staging
+   *   folder where the backup was created, or null if the backup failed.
    */
   async createBackup({ profilePath = PathUtils.profileDir } = {}) {
     // createBackup does not allow re-entry or concurrent backups.
     if (this.#backupInProgress) {
       lazy.logConsole.warn("Backup attempt already in progress");
-      return;
+      return null;
     }
 
     this.#backupInProgress = true;
@@ -204,11 +212,19 @@ export class BackupService {
             resourcePath,
             profilePath
           );
+
+          if (manifestEntry === undefined) {
+            lazy.logConsole.error(
+              `Backup of resource with key ${resourceClass.key} returned undefined
+              as its ManifestEntry instead of null or an object`
+            );
+          } else {
             lazy.logConsole.debug(
               `Backup of resource with key ${resourceClass.key} completed`,
               manifestEntry
             );
             manifest.resources[resourceClass.key] = manifestEntry;
+          }
         } catch (e) {
           lazy.logConsole.error(
             `Failed to backup resource: ${resourceClass.key}`,
@@ -246,7 +262,12 @@ export class BackupService {
       );
       await IOUtils.writeJSON(manifestPath, manifest);
 
-      await this.#finalizeStagingFolder(stagingPath);
+      let renamedStagingPath = await this.#finalizeStagingFolder(stagingPath);
+      lazy.logConsole.log(
+        "Wrote backup to staging directory at ",
+        renamedStagingPath
+      );
+      return { stagingPath: renamedStagingPath };
     } finally {
       this.#backupInProgress = false;
     }
@@ -281,6 +302,9 @@ export class BackupService {
    *
    * @param {string} stagingPath
    *   The path to the populated staging folder.
+   * @returns {Promise<string|null>}
+   *   The path to the renamed staging folder, or null if the stagingPath was
+   *   not pointing to a valid folder.
    */
   async #finalizeStagingFolder(stagingPath) {
     if (!(await IOUtils.exists(stagingPath))) {
@@ -288,7 +312,7 @@ export class BackupService {
       lazy.logConsole.error(
         `Failed to finalize staging folder. Cannot find ${stagingPath}.`
       );
-      return;
+      return null;
     }
 
     try {
@@ -319,10 +343,12 @@ export class BackupService {
           });
         }
       }
+      return renamedBackupPath;
     } catch (e) {
       lazy.logConsole.error(
         `Something went wrong while finalizing the staging folder. ${e}`
       );
+      throw e;
     }
   }
 

browser/components/backup/moz.build

@@ -12,6 +12,7 @@ JAR_MANIFESTS += ["jar.mn"]
 SPHINX_TREES["docs"] = "docs"
 
 XPCSHELL_TESTS_MANIFESTS += ["tests/xpcshell/xpcshell.toml"]
+MARIONETTE_MANIFESTS += ["tests/marionette/manifest.toml"]
 
 EXTRA_JS_MODULES.backup += [
     "BackupResources.sys.mjs",

browser/components/backup/resources/AddonsBackupResource.sys.mjs

@@ -63,6 +63,8 @@ export class AddonsBackupResource extends BackupResource {
       stagingPath,
       databases
     );
+
+    return null;
   }
 
   async measure(profilePath = PathUtils.profileDir) {

browser/components/backup/resources/BackupResource.sys.mjs

@@ -153,7 +153,8 @@ export class BackupResource {
   /**
    * Copy a set of SQLite databases safely from a source directory to a
    * destination directory. A new read-only connection is opened for each
-   * database, and then a backup is created.
+   * database, and then a backup is created. If the source database does not
+   * exist, it is ignored.
    *
    * @param {string} sourcePath
    *   Path to the source directory of the SQLite databases.
@@ -167,6 +168,11 @@ export class BackupResource {
   static async copySqliteDatabases(sourcePath, destPath, sqliteDatabases) {
     for (let fileName of sqliteDatabases) {
       let sourceFilePath = PathUtils.join(sourcePath, fileName);
+
+      if (!(await IOUtils.exists(sourceFilePath))) {
+        continue;
+      }
+
       let destFilePath = PathUtils.join(destPath, fileName);
       let connection;
 

browser/components/backup/resources/CookiesBackupResource.sys.mjs

@@ -20,6 +20,7 @@ export class CookiesBackupResource extends BackupResource {
     await BackupResource.copySqliteDatabases(profilePath, stagingPath, [
       "cookies.sqlite",
     ]);
+    return null;
   }
 
   async measure(profilePath = PathUtils.profileDir) {

browser/components/backup/resources/FormHistoryBackupResource.sys.mjs

@@ -20,6 +20,8 @@ export class FormHistoryBackupResource extends BackupResource {
     await BackupResource.copySqliteDatabases(profilePath, stagingPath, [
       "formhistory.sqlite",
     ]);
+
+    return null;
   }
 
   async measure(profilePath = PathUtils.profileDir) {

browser/components/backup/resources/SessionStoreBackupResource.sys.mjs

@@ -41,6 +41,8 @@ export class SessionStoreBackupResource extends BackupResource {
     await BackupResource.copyFiles(profilePath, stagingPath, [
       "sessionstore-backups",
     ]);
+
+    return null;
   }
 
   async measure(profilePath = PathUtils.profileDir) {

browser/components/backup/tests/marionette/manifest.toml

@@ -0,0 +1,5 @@
+[DEFAULT]
+run-if = ["buildapp == 'browser'"]
+prefs = ["browser.backup.enabled=true", "browser.backup.log=true"]
+
+["test_backup.py"]

browser/components/backup/tests/marionette/test_backup.py

@@ -0,0 +1,98 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import json
+import os
+
+from marionette_harness import MarionetteTestCase
+
+
+class BackupTest(MarionetteTestCase):
+    def setUp(self):
+        MarionetteTestCase.setUp(self)
+        # We need to quit the browser and restart with the browser.backup.log
+        # pref already set to true in order for it to be displayed.
+        self.marionette.quit()
+        self.marionette.instance.prefs = {
+            "browser.backup.log": True,
+        }
+        # Now restart the browser.
+        self.marionette.instance.switch_profile()
+        self.marionette.start_session()
+
+    def test_backup(self):
+        self.marionette.set_context("chrome")
+
+        # In bug 1892105, we'll update this test to write some values to various
+        # datastores, like bookmarks, passwords, cookies, etc. Then we'll run
+        # a backup and ensure that the data can be recovered from the backup.
+        resourceKeys = self.marionette.execute_script(
+            """
+          const DefaultBackupResources = ChromeUtils.importESModule("resource:///modules/backup/BackupResources.sys.mjs");
+          let resourceKeys = [];
+          for (const resourceName in DefaultBackupResources) {
+            let resource = DefaultBackupResources[resourceName];
+            resourceKeys.push(resource.key);
+          }
+          return resourceKeys;
+        """
+        )
+
+        stagingPath = self.marionette.execute_async_script(
+            """
+          const { BackupService } = ChromeUtils.importESModule("resource:///modules/backup/BackupService.sys.mjs");
+          let bs = BackupService.init();
+          if (!bs) {
+            throw new Error("Could not get initialized BackupService.");
+          }
+
+          let [outerResolve] = arguments;
+          (async () => {
+            let { stagingPath } = await bs.createBackup();
+            if (!stagingPath) {
+              throw new Error("Could not create backup.");
+            }
+            return stagingPath;
+          })().then(outerResolve);
+        """
+        )
+
+        # First, ensure that the staging path exists
+        self.assertTrue(os.path.exists(stagingPath))
+        # Now, ensure that the backup-manifest.json file exists within it.
+        manifestPath = os.path.join(stagingPath, "backup-manifest.json")
+        self.assertTrue(os.path.exists(manifestPath))
+
+        # For now, we just do a cursory check to ensure that for the resources
+        # that are listed in the manifest as having been backed up, that we
+        # have at least one file in their respective staging directories.
+        # We don't check the contents of the files, just that they exist.
+
+        # Read the JSON manifest file
+        with open(manifestPath, "r") as f:
+            manifest = json.load(f)
+
+        # Ensure that the manifest has a "resources" key
+        self.assertIn("resources", manifest)
+        resources = manifest["resources"]
+        self.assertTrue(isinstance(resources, dict))
+        self.assertTrue(len(resources) > 0)
+
+        # We don't have encryption capabilities wired up yet, so we'll check
+        # that all default resources are represented in the manifest.
+        self.assertEqual(len(resources), len(resourceKeys))
+        for resourceKey in resourceKeys:
+            self.assertIn(resourceKey, resources)
+
+        # Iterate the resources dict keys
+        for resourceKey in resources:
+            print("Checking resource: %s" % resourceKey)
+            # Ensure that there are staging directories created for each
+            # resource that was backed up
+            resourceStagingDir = os.path.join(stagingPath, resourceKey)
+            self.assertTrue(os.path.exists(resourceStagingDir))
+
+        # In bug 1892105, we'll update this test to then recover from this
+        # staging directory and ensure that the recovered data is what we
+        # expect.

browser/components/backup/tests/xpcshell/test_AddonsBackupResource.js

@@ -302,7 +302,7 @@ add_task(async function test_backup() {
     "AddonsBackupResource-staging-test"
   );
 
-  const files = [
+  const simpleCopyFiles = [
     { path: "extensions.json" },
     { path: "extension-settings.json" },
     { path: "extension-preferences.json" },
@@ -316,20 +316,33 @@ add_task(async function test_backup() {
     { path: ["extension-store-permissions", "data.safe.bin"] },
     { path: ["extensions", "{11aa1234-f111-1234-abcd-a9b8c7654d32}.xpi"] },
   ];
-  await createTestFiles(sourcePath, files);
+  await createTestFiles(sourcePath, simpleCopyFiles);
 
   const junkFiles = [{ path: ["extensions", "junk"] }];
   await createTestFiles(sourcePath, junkFiles);
 
+  // Create a fake storage-sync-v2 database file. We don't expect this to
+  // be copied to the staging directory in this test due to our stubbing
+  // of the backup method, so we don't include it in `simpleCopyFiles`.
+  await createTestFiles(sourcePath, [{ path: "storage-sync-v2.sqlite" }]);
+
   let fakeConnection = {
     backup: sandbox.stub().resolves(true),
     close: sandbox.stub().resolves(true),
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await addonsBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await addonsBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "AddonsBackupResource.backup should return null as its ManifestEntry"
+  );
 
-  await assertFilesExist(stagingPath, files);
+  await assertFilesExist(stagingPath, simpleCopyFiles);
 
   let junkFile = PathUtils.join(stagingPath, "extensions", "junk");
   Assert.equal(

browser/components/backup/tests/xpcshell/test_BackupResource.js

@@ -101,6 +101,10 @@ add_task(async function test_copySqliteDatabases() {
     "BackupResource-dest-test"
   );
   let pretendDatabases = ["places.sqlite", "favicons.sqlite"];
+  await createTestFiles(
+    sourcePath,
+    pretendDatabases.map(f => ({ path: f }))
+  );
 
   let fakeConnection = {
     backup: sandbox.stub().resolves(true),

browser/components/backup/tests/xpcshell/test_CookiesBackupResource.js

@@ -61,6 +61,10 @@ add_task(async function test_backup() {
     "CookiesBackupResource-staging-test"
   );
 
+  // Make sure this file exists in the source directory, otherwise
+  // BackupResource will skip attempting to back it up.
+  await createTestFiles(sourcePath, [{ path: "cookies.sqlite" }]);
+
   // We have no need to test that Sqlite.sys.mjs's backup method is working -
   // this is something that is tested in Sqlite's own tests. We can just make
   // sure that it's being called using sinon. Unfortunately, we cannot do the
@@ -71,7 +75,15 @@ add_task(async function test_backup() {
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await cookiesBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await cookiesBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "CookiesBackupResource.backup should return null as its ManifestEntry"
+  );
 
   // Next, we'll make sure that the Sqlite connection had `backup` called on it
   // with the right arguments.

browser/components/backup/tests/xpcshell/test_CredentialsAndSecurityBackupResource.js

@@ -104,6 +104,15 @@ add_task(async function test_backup() {
   ];
   await createTestFiles(sourcePath, simpleCopyFiles);
 
+  // Create our fake database files. We don't expect these to be copied to the
+  // staging directory in this test due to our stubbing of the backup method, so
+  // we don't include it in `simpleCopyFiles`.
+  await createTestFiles(sourcePath, [
+    { path: "cert9.db" },
+    { path: "key4.db" },
+    { path: "credentialstate.sqlite" },
+  ]);
+
   // We have no need to test that Sqlite.sys.mjs's backup method is working -
   // this is something that is tested in Sqlite's own tests. We can just make
   // sure that it's being called using sinon. Unfortunately, we cannot do the
@@ -114,7 +123,16 @@ add_task(async function test_backup() {
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await credentialsAndSecurityBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await credentialsAndSecurityBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+
+  Assert.equal(
+    manifestEntry,
+    null,
+    "CredentialsAndSecurityBackupResource.backup should return null as its ManifestEntry"
+  );
 
   await assertFilesExist(stagingPath, simpleCopyFiles);
 

browser/components/backup/tests/xpcshell/test_FormHistoryBackupResource.js

@@ -65,6 +65,10 @@ add_task(async function test_backup() {
     "FormHistoryBackupResource-staging-test"
   );
 
+  // Make sure this file exists in the source directory, otherwise
+  // BackupResource will skip attempting to back it up.
+  await createTestFiles(sourcePath, [{ path: "formhistory.sqlite" }]);
+
   // We have no need to test that Sqlite.sys.mjs's backup method is working -
   // this is something that is tested in Sqlite's own tests. We can just make
   // sure that it's being called using sinon. Unfortunately, we cannot do the
@@ -75,7 +79,15 @@ add_task(async function test_backup() {
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await formHistoryBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await formHistoryBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "FormHistoryBackupResource.backup should return null as its ManifestEntry"
+  );
 
   // Next, we'll make sure that the Sqlite connection had `backup` called on it
   // with the right arguments.

browser/components/backup/tests/xpcshell/test_MiscDataBackupResource.js

@@ -79,6 +79,11 @@ add_task(async function test_backup() {
   ];
   await createTestFiles(sourcePath, simpleCopyFiles);
 
+  // Create our fake database files. We don't expect this to be copied to the
+  // staging directory in this test due to our stubbing of the backup method, so
+  // we don't include it in `simpleCopyFiles`.
+  await createTestFiles(sourcePath, [{ path: "protections.sqlite" }]);
+
   // We have no need to test that Sqlite.sys.mjs's backup method is working -
   // this is something that is tested in Sqlite's own tests. We can just make
   // sure that it's being called using sinon. Unfortunately, we cannot do the
@@ -101,7 +106,15 @@ add_task(async function test_backup() {
     .withArgs("snippets")
     .resolves(snippetsTableStub);
 
-  await miscDataBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await miscDataBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "MiscDataBackupResource.backup should return null as its ManifestEntry"
+  );
 
   await assertFilesExist(stagingPath, simpleCopyFiles);
 

browser/components/backup/tests/xpcshell/test_PlacesBackupResource.js

@@ -93,13 +93,28 @@ add_task(async function test_backup() {
     "PlacesBackupResource-staging-test"
   );
 
+  // Make sure these files exist in the source directory, otherwise
+  // BackupResource will skip attempting to back them up.
+  await createTestFiles(sourcePath, [
+    { path: "places.sqlite" },
+    { path: "favicons.sqlite" },
+  ]);
+
   let fakeConnection = {
     backup: sandbox.stub().resolves(true),
     close: sandbox.stub().resolves(true),
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await placesBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await placesBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "PlacesBackupResource.backup should return null as its ManifestEntry"
+  );
 
   Assert.ok(
     fakeConnection.backup.calledTwice,
@@ -154,7 +169,16 @@ add_task(async function test_backup_no_saved_history() {
   Services.prefs.setBoolPref(HISTORY_ENABLED_PREF, false);
   Services.prefs.setBoolPref(SANITIZE_ON_SHUTDOWN_PREF, false);
 
-  await placesBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await placesBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.deepEqual(
+    manifestEntry,
+    { bookmarksOnly: true },
+    "Should have gotten back a ManifestEntry indicating that we only copied " +
+      "bookmarks"
+  );
 
   Assert.ok(
     fakeConnection.backup.notCalled,
@@ -171,7 +195,13 @@ add_task(async function test_backup_no_saved_history() {
   Services.prefs.setBoolPref(SANITIZE_ON_SHUTDOWN_PREF, true);
 
   fakeConnection.backup.resetHistory();
-  await placesBackupResource.backup(stagingPath, sourcePath);
+  manifestEntry = await placesBackupResource.backup(stagingPath, sourcePath);
+  Assert.deepEqual(
+    manifestEntry,
+    { bookmarksOnly: true },
+    "Should have gotten back a ManifestEntry indicating that we only copied " +
+      "bookmarks"
+  );
 
   Assert.ok(
     fakeConnection.backup.notCalled,
@@ -211,7 +241,16 @@ add_task(async function test_backup_private_browsing() {
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
   sandbox.stub(PrivateBrowsingUtils, "permanentPrivateBrowsing").value(true);
 
-  await placesBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await placesBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.deepEqual(
+    manifestEntry,
+    { bookmarksOnly: true },
+    "Should have gotten back a ManifestEntry indicating that we only copied " +
+      "bookmarks"
+  );
 
   Assert.ok(
     fakeConnection.backup.notCalled,

browser/components/backup/tests/xpcshell/test_PreferencesBackupResource.js

@@ -86,6 +86,14 @@ add_task(async function test_backup() {
   ];
   await createTestFiles(sourcePath, simpleCopyFiles);
 
+  // Create our fake database files. We don't expect these to be copied to the
+  // staging directory in this test due to our stubbing of the backup method, so
+  // we don't include it in `simpleCopyFiles`.
+  await createTestFiles(sourcePath, [
+    { path: "permissions.sqlite" },
+    { path: "content-prefs.sqlite" },
+  ]);
+
   // We have no need to test that Sqlite.sys.mjs's backup method is working -
   // this is something that is tested in Sqlite's own tests. We can just make
   // sure that it's being called using sinon. Unfortunately, we cannot do the
@@ -96,7 +104,15 @@ add_task(async function test_backup() {
   };
   sandbox.stub(Sqlite, "openConnection").returns(fakeConnection);
 
-  await preferencesBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await preferencesBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "PreferencesBackupResource.backup should return null as its ManifestEntry"
+  );
 
   await assertFilesExist(stagingPath, simpleCopyFiles);
 

browser/components/backup/tests/xpcshell/test_SessionStoreBackupResource.js

@@ -93,7 +93,15 @@ add_task(async function test_backup() {
   await createTestFiles(sourcePath, simpleCopyFiles);
 
   let sessionStoreState = SessionStore.getCurrentState(true);
-  await sessionStoreBackupResource.backup(stagingPath, sourcePath);
+  let manifestEntry = await sessionStoreBackupResource.backup(
+    stagingPath,
+    sourcePath
+  );
+  Assert.equal(
+    manifestEntry,
+    null,
+    "SessionStoreBackupResource.backup should return null as its ManifestEntry"
+  );
 
   /**
    * We don't expect the actual file sessionstore.jsonlz4 to exist in the profile directory before calling the backup method.