Bug 1816197 - Part 2: Add an optional callback to MigratorBase.migrat…

…e to get migration updates. r=NeilDeakin,fluent-reviewers,flod

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

browser/components/migration/MigrationWizardParent.sys.mjs

@@ -18,7 +18,8 @@ XPCOMUtils.defineLazyGetter(lazy, "gFluentStrings", function() {
 ChromeUtils.defineESModuleGetters(lazy, {
   InternalTestingProfileMigrator:
     "resource:///modules/InternalTestingProfileMigrator.sys.mjs",
-  PromiseUtils: "resource://gre/modules/PromiseUtils.sys.mjs",
+  MigrationWizardConstants:
+    "chrome://browser/content/migration/migration-wizard-constants.mjs",
 });
 
 /**
@@ -112,66 +113,45 @@ export class MigrationWizardParent extends JSWindowActorParent {
 
     this.sendAsyncMessage("UpdateProgress", progress);
 
-    let observer = {
-      observe: (subject, topic, resourceType) => {
-        if (topic == "Migration:Ended") {
-          observer.migrationDefer.resolve();
-          return;
-        }
-
-        // Unfortunately, MigratorBase hands us the string representation
-        // of the numeric value of the MigrationUtils.resourceType from this
-        // observer. For now, we'll just do a look-up to map it to the right
-        // constant.
-
-        let resourceTypeNum = parseInt(resourceType, 10);
-        let foundResourceTypeName;
-        for (let resourceTypeName in MigrationUtils.resourceTypes) {
-          if (
-            MigrationUtils.resourceTypes[resourceTypeName] == resourceTypeNum
-          ) {
-            foundResourceTypeName = resourceTypeName;
-            break;
+    try {
+      await migrator.migrate(
+        resourceTypesToMigrate,
+        false,
+        profileObj,
+        async resourceTypeNum => {
+          // Unfortunately, MigratorBase hands us the the numeric value of the
+          // MigrationUtils.resourceType for this callback. For now, we'll just
+          // do a look-up to map it to the right constant.
+          let foundResourceTypeName;
+          for (let resourceTypeName in MigrationUtils.resourceTypes) {
+            if (
+              MigrationUtils.resourceTypes[resourceTypeName] == resourceTypeNum
+            ) {
+              foundResourceTypeName = resourceTypeName;
+              break;
+            }
           }
-        }
 
-        if (!foundResourceTypeName) {
-          console.error(
-            "Could not find a resource type for value: ",
-            resourceType
-          );
-        } else {
-          // For now, we ignore errors in migration, and simply display
-          // the success state.
-          progress[foundResourceTypeName] = {
-            inProgress: false,
-            message: "",
-          };
-          this.sendAsyncMessage("UpdateProgress", progress);
+          if (!foundResourceTypeName) {
+            console.error(
+              "Could not find a resource type for value: ",
+              resourceTypeNum
+            );
+          } else {
+            // For now, we ignore errors in migration, and simply display
+            // the success state.
+            progress[foundResourceTypeName] = {
+              inProgress: false,
+              message: await this.#getStringForImportQuantity(
+                foundResourceTypeName
+              ),
+            };
+            this.sendAsyncMessage("UpdateProgress", progress);
+          }
         }
-      },
-
-      migrationDefer: lazy.PromiseUtils.defer(),
-
-      QueryInterface: ChromeUtils.generateQI([
-        Ci.nsIObserver,
-        Ci.nsISupportsWeakReference,
-      ]),
-    };
-    Services.obs.addObserver(observer, "Migration:ItemAfterMigrate", true);
-    Services.obs.addObserver(observer, "Migration:ItemError", true);
-    Services.obs.addObserver(observer, "Migration:Ended", true);
-
-    try {
-      // The MigratorBase API is somewhat awkward - we must wait for an observer
-      // notification with topic Migration:Ended to know when the migration
-      // finishes.
-      migrator.migrate(resourceTypesToMigrate, false, profileObj);
-      await observer.migrationDefer.promise;
-    } finally {
-      Services.obs.removeObserver(observer, "Migration:ItemAfterMigrate");
-      Services.obs.removeObserver(observer, "Migration:ItemError");
-      Services.obs.removeObserver(observer, "Migration:Ended");
+      );
+    } catch (e) {
+      console.error(e);
     }
   }
 
@@ -281,4 +261,49 @@ export class MigrationWizardParent extends JSWindowActorParent {
       profile: profileObj,
     };
   }
+
+  /**
+   * Returns the "success" string for a particular resource type after
+   * migration has completed.
+   *
+   * @param {string} resourceTypeStr
+   *   A string mapping to one of the key values of
+   *   MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.
+   * @returns {Promise<string>}
+   *   The success string for the resource type after migration has completed.
+   */
+  #getStringForImportQuantity(resourceTypeStr) {
+    switch (resourceTypeStr) {
+      case lazy.MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.BOOKMARKS: {
+        let quantity = MigrationUtils.getImportedCount("bookmarks");
+        return lazy.gFluentStrings.formatValue(
+          "migration-wizard-progress-success-bookmarks",
+          {
+            quantity,
+          }
+        );
+      }
+      case lazy.MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.HISTORY: {
+        let quantity = MigrationUtils.getImportedCount("history");
+        return lazy.gFluentStrings.formatValue(
+          "migration-wizard-progress-success-history",
+          {
+            quantity,
+          }
+        );
+      }
+      case lazy.MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.PASSWORDS: {
+        let quantity = MigrationUtils.getImportedCount("logins");
+        return lazy.gFluentStrings.formatValue(
+          "migration-wizard-progress-success-passwords",
+          {
+            quantity,
+          }
+        );
+      }
+      default: {
+        return "";
+      }
+    }
+  }
 }

browser/components/migration/MigratorBase.sys.mjs

@@ -217,8 +217,12 @@ export class MigratorBase {
    *   True if this migration is occurring during startup.
    * @param {object|string} aProfile
    *   The other browser profile that is being migrated from.
+   * @param {Function|null} aProgressCallback
+   *   An optional callback that will be fired once a resourceType has finished
+   *   migrating. The callback will be passed the numeric representation of the
+   *   resource type.
    */
-  async migrate(aItems, aStartup, aProfile) {
+  async migrate(aItems, aStartup, aProfile, aProgressCallback = () => {}) {
     let resources = await this.#getMaybeCachedResources(aProfile);
     if (!resources.length) {
       throw new Error("migrate called for a non-existent source");
@@ -357,6 +361,9 @@ export class MigratorBase {
                   : "Migration:ItemError",
                 migrationType
               );
+
+              aProgressCallback(migrationType);
+
               resourcesGroupedByItems.delete(migrationType);
 
               if (stopwatchHistogramId) {

browser/components/migration/tests/browser/browser_do_migration.js

@@ -8,6 +8,23 @@ const { InternalTestingProfileMigrator } = ChromeUtils.importESModule(
   "resource:///modules/InternalTestingProfileMigrator.sys.mjs"
 );
 
+/**
+ * These are the resource types that currently display their import success
+ * message with a quantity.
+ */
+const RESOURCE_TYPES_WITH_QUANTITIES = [
+  MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.BOOKMARKS,
+  MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.HISTORY,
+  MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.PASSWORDS,
+];
+
+/**
+ * We'll have this be our magic number of quantities of various imports.
+ * We will use Sinon to prepare MigrationUtils to presume that this was
+ * how many of each quantity-supported resource type was imported.
+ */
+const EXPECTED_QUANTITY = 123;
+
 /**
  * A helper function that prepares the InternalTestingProfileMigrator
  * with some set of fake available resources, and resolves a Promise
@@ -48,26 +65,42 @@ async function waitForTestMigration(
       );
     });
 
+  sandbox.stub(MigrationUtils, "_importQuantities").value({
+    bookmarks: EXPECTED_QUANTITY,
+    history: EXPECTED_QUANTITY,
+    logins: EXPECTED_QUANTITY,
+  });
+
   // Fake out the migrate method of the migrator and assert that the
   // next time it's called, its arguments match our expectations.
   return new Promise(resolve => {
     sandbox
       .stub(InternalTestingProfileMigrator.prototype, "migrate")
-      .callsFake((aResourceTypes, aStartup, aProfile) => {
+      .callsFake((aResourceTypes, aStartup, aProfile, aProgressCallback) => {
         Assert.ok(
           !aStartup,
           "Migrator should not have been called as a startup migration."
         );
+
+        let bitMask = 0;
+        for (let resourceType of expectedResourceTypes) {
+          bitMask |= resourceType;
+        }
+
         Assert.deepEqual(
           aResourceTypes,
-          expectedResourceTypes,
+          bitMask,
           "Got the expected resource types"
         );
         Assert.deepEqual(
           aProfile,
           expectedProfile,
           "Got the expected profile object"
         );
+
+        for (let resourceType of expectedResourceTypes) {
+          aProgressCallback(resourceType);
+        }
         Services.obs.notifyObservers(null, "Migration:Ended");
         resolve();
       });
@@ -121,6 +154,77 @@ function selectResourceTypesAndStartMigration(wizard, selectedResourceTypes) {
   importButton.click();
 }
 
+/**
+ * Assert that the resource types passed in expectedResourceTypes are
+ * showing a success state after a migration, and if they are part of
+ * the RESOURCE_TYPES_WITH_QUANTITIES group, that they're showing the
+ * EXPECTED_QUANTITY magic number in their success message. Otherwise,
+ * we (currently) check that they show the empty string.
+ *
+ * @param {Element} wizard
+ *   The MigrationWizard element.
+ * @param {string[]} expectedResourceTypes
+ *   An array of resource type strings from
+ *   MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.
+ */
+function assertQuantitiesShown(wizard, expectedResourceTypes) {
+  let shadow = wizard.openOrClosedShadowRoot;
+
+  // Make sure that we're showing the progress page first.
+  let deck = shadow.querySelector("#wizard-deck");
+  Assert.equal(
+    deck.selectedViewName,
+    `page-${MigrationWizardConstants.PAGES.PROGRESS}`
+  );
+
+  // Go through each displayed resource and make sure that only the
+  // ones that are expected are shown, and are showing the right
+  // success message.
+
+  let progressGroups = shadow.querySelectorAll(".resource-progress-group");
+  for (let progressGroup of progressGroups) {
+    if (expectedResourceTypes.includes(progressGroup.dataset.resourceType)) {
+      let progressIcon = progressGroup.querySelector(".progress-icon");
+      let successText = progressGroup.querySelector(".success-text")
+        .textContent;
+
+      Assert.ok(
+        progressIcon.classList.contains("completed"),
+        "Should be showing completed state."
+      );
+
+      if (
+        RESOURCE_TYPES_WITH_QUANTITIES.includes(
+          progressGroup.dataset.resourceType
+        )
+      ) {
+        Assert.notEqual(
+          successText.indexOf(EXPECTED_QUANTITY),
+          -1,
+          `Found expected quantity in success string: ${successText}`
+        );
+      } else {
+        // If you've found yourself here, and this is failing, it's probably because you've
+        // updated MigrationWizardParent.#getStringForImportQuantity to return a string for
+        // a resource type that's not in RESOURCE_TYPES_WITH_QUANTITIES, and you'll need
+        // to modify this function to check for that string.
+        Assert.equal(
+          successText,
+          "",
+          "Expected the empty string if the resource type " +
+            "isn't in RESOURCE_TYPES_WITH_QUANTITIES"
+        );
+      }
+    } else {
+      Assert.ok(
+        BrowserTestUtils.is_hidden(progressGroup),
+        `Resource progress group for ${progressGroup.dataset.resourceType}` +
+          ` should be hidden.`
+      );
+    }
+  }
+}
+
 /**
  * Tests that the MigrationWizard can be used to successfully migrate
  * using the InternalTestingProfileMigrator in a few scenarios.
@@ -145,6 +249,9 @@ add_task(async function test_successful_migrations() {
     ]);
     await migration;
     await wizardDone;
+    assertQuantitiesShown(wizard, [
+      MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.BOOKMARKS,
+    ]);
   });
 
   // Scenario 2: Several resource types are available, but only 1
@@ -170,5 +277,31 @@ add_task(async function test_successful_migrations() {
     ]);
     await migration;
     await wizardDone;
+    assertQuantitiesShown(wizard, [
+      MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES.PASSWORDS,
+    ]);
+  });
+
+  // Scenario 3: Several resource types are available, all are checked.
+  let allResourceTypeStrs = Object.values(
+    MigrationWizardConstants.DISPLAYED_RESOURCE_TYPES
+  );
+  let allResourceTypes = allResourceTypeStrs.map(resourceTypeStr => {
+    return MigrationUtils.resourceTypes[resourceTypeStr];
+  });
+
+  migration = waitForTestMigration(allResourceTypes, allResourceTypes, null);
+
+  await withMigrationWizardDialog(async prefsWin => {
+    let dialogBody = prefsWin.document.body;
+    let wizard = dialogBody.querySelector("migration-wizard");
+    let wizardDone = BrowserTestUtils.waitForEvent(
+      wizard,
+      "MigrationWizard:DoneMigration"
+    );
+    selectResourceTypesAndStartMigration(wizard, allResourceTypeStrs);
+    await migration;
+    await wizardDone;
+    assertQuantitiesShown(wizard, allResourceTypeStrs);
   });
 });