Bug 1283542 - fix profile detection code in AutoMigrate to deal with profile objects instead of identifiers, r?dolske draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 30 Jun 2016 17:57:33 +0100
changeset 382844 5529ea35f9a58402123c799810052597057f1897
parent 382843 7b4e8a8e4f0b3a13e0862d8b54f7e9967069b6d9
child 524309 45624d61b9e6b602abbb02aab36594d24ad6ac83
push id21844
push usergijskruitbosch@gmail.com
push dateThu, 30 Jun 2016 16:58:19 +0000
reviewersdolske
bugs1283542
milestone50.0a1
Bug 1283542 - fix profile detection code in AutoMigrate to deal with profile objects instead of identifiers, r?dolske MozReview-Commit-ID: KHjsOsxwzb7
browser/components/migration/AutoMigrate.jsm
browser/components/migration/tests/unit/test_automigration.js
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -95,38 +95,38 @@ const AutoMigrate = {
     return migrator;
   },
 
   /**
    * Pick a source profile (from the original browser) to use.
    *
    * @param {Migrator} migrator     the migrator object to use
    * @param {String}   suggestedId  the id of the profile to migrate, if pre-specified, or null
-   * @returns                       the id of the profile to migrate, or null if migrating
+   * @returns                       the profile to migrate, or null if migrating
    *                                from the default profile.
    */
   pickProfile(migrator, suggestedId) {
     let profiles = migrator.sourceProfiles;
     if (profiles && !profiles.length) {
       throw new Error("No profile data found to migrate.");
     }
     if (suggestedId) {
       if (!profiles) {
         throw new Error("Profile specified but only a default profile found.");
       }
       let suggestedProfile = profiles.find(profile => profile.id == suggestedId);
       if (!suggestedProfile) {
         throw new Error("Profile specified was not found.");
       }
-      return suggestedProfile.id;
+      return suggestedProfile;
     }
     if (profiles && profiles.length > 1) {
       throw new Error("Don't know how to pick a profile when more than 1 profile is present.");
     }
-    return profiles ? profiles[0].id : null;
+    return profiles ? profiles[0] : null;
   },
 
   getUndoRange() {
     let start, finish;
     try {
       start = parseInt(Services.prefs.getCharPref(kAutoMigrateStartedPref), 10);
       finish = parseInt(Services.prefs.getCharPref(kAutoMigrateFinishedPref), 10);
     } catch (ex) {
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -61,40 +61,42 @@ add_task(function* checkMigratorPicking(
 });
 
 
 /**
  * Test automatically picking a profile to migrate from
  */
 add_task(function* checkProfilePicking() {
   let fakeMigrator = {sourceProfiles: [{id: "a"}, {id: "b"}]};
+  let profB = fakeMigrator.sourceProfiles[1];
   Assert.throws(() => AutoMigrate.pickProfile(fakeMigrator),
                 /Don't know how to pick a profile when more/,
                 "Should throw when there are multiple profiles.");
   Assert.throws(() => AutoMigrate.pickProfile(fakeMigrator, "c"),
                 /Profile specified was not found/,
                 "Should throw when the profile supplied doesn't exist.");
   let profileToMigrate = AutoMigrate.pickProfile(fakeMigrator, "b");
-  Assert.equal(profileToMigrate, "b", "Should return profile supplied");
+  Assert.equal(profileToMigrate, profB, "Should return profile supplied");
 
   fakeMigrator.sourceProfiles = null;
   Assert.throws(() => AutoMigrate.pickProfile(fakeMigrator, "c"),
                 /Profile specified but only a default profile found./,
                 "Should throw when the profile supplied doesn't exist.");
   profileToMigrate = AutoMigrate.pickProfile(fakeMigrator);
   Assert.equal(profileToMigrate, null, "Should return default profile when that's the only one.");
 
   fakeMigrator.sourceProfiles = [];
   Assert.throws(() => AutoMigrate.pickProfile(fakeMigrator),
                 /No profile data found/,
                 "Should throw when no profile data is present.");
 
   fakeMigrator.sourceProfiles = [{id: "a"}];
+  let profA = fakeMigrator.sourceProfiles[0];
   profileToMigrate = AutoMigrate.pickProfile(fakeMigrator);
-  Assert.equal(profileToMigrate, "a", "Should return the only profile if only one is present.");
+  Assert.equal(profileToMigrate, profA, "Should return the only profile if only one is present.");
 });
 
 /**
  * Test the complete automatic process including browser and profile selection,
  * and actual migration (which implies startup)
  */
 add_task(function* checkIntegration() {
   gShimmedMigrator = {