Bug 1289229 - disable automigration undo if people create/change bookmarks/logins, r=mak
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 26 Jul 2016 12:48:37 +0100
changeset 346810 80afb86a5a171430c5d687f7fd697d809505fda3
parent 346809 96de0b18cace15865b2d013110e9c70aa7d9d6d8
child 346811 fe3a34f70dc005b1421dba2555eec53f26ada6e1
child 347108 4b767c8f023685e9e598f026453fbd906e0e8d1f
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1289229
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1289229 - disable automigration undo if people create/change bookmarks/logins, r=mak MozReview-Commit-ID: DAoNV9H71Yv
browser/app/profile/firefox.js
browser/components/migration/AutoMigrate.jsm
browser/components/migration/MigrationUtils.jsm
browser/components/migration/content/migration.js
browser/components/migration/tests/unit/test_automigration.js
browser/components/nsBrowserGlue.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1455,17 +1455,17 @@ pref("toolkit.pageThumbs.minHeight", 190
 
 // Enable speech synthesis
 pref("media.webspeech.synth.enabled", true);
 
 pref("browser.esedbreader.loglevel", "Error");
 
 pref("browser.laterrun.enabled", false);
 
-pref("browser.migration.automigrate", false);
+pref("browser.migrate.automigrate.enabled", false);
 
 // Enable browser frames for use on desktop.  Only exposed to chrome callers.
 pref("dom.mozBrowserFramesEnabled", true);
 
 pref("extensions.pocket.enabled", true);
 
 pref("signon.schemeUpgrades", true);
 
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -3,83 +3,129 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 this.EXPORTED_SYMBOLS = ["AutoMigrate"];
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
-const kAutoMigrateStartedPref = "browser.migrate.automigrate-started";
-const kAutoMigrateFinishedPref = "browser.migrate.automigrate-finished";
+const kAutoMigrateEnabledPref = "browser.migrate.automigrate.enabled";
+
+const kAutoMigrateStartedPref = "browser.migrate.automigrate.started";
+const kAutoMigrateFinishedPref = "browser.migrate.automigrate.finished";
+const kAutoMigrateBrowserPref = "browser.migrate.automigrate.browser";
+
+const kPasswordManagerTopic = "passwordmgr-storage-changed";
+const kPasswordManagerTopicTypes = new Set([
+  "addLogin",
+  "modifyLogin",
+]);
 
 Cu.import("resource:///modules/MigrationUtils.jsm");
+Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 const AutoMigrate = {
   get resourceTypesToUse() {
     let {BOOKMARKS, HISTORY, PASSWORDS} = Ci.nsIBrowserProfileMigrator;
     return BOOKMARKS | HISTORY | PASSWORDS;
   },
 
+  init() {
+    this.enabled = Preferences.get(kAutoMigrateEnabledPref, false);
+    if (this.enabled) {
+      this.maybeInitUndoObserver();
+    }
+  },
+
+  maybeInitUndoObserver() {
+    // Check synchronously (NB: canUndo is async) if we really need
+    // to do this:
+    if (!this.getUndoRange()) {
+      return;
+    }
+    // Now register places and password observers:
+    this.onItemAdded = this.onItemMoved = this.onItemChanged =
+      this.removeUndoOption;
+    PlacesUtils.addLazyBookmarkObserver(this, true);
+    Services.obs.addObserver(this, kPasswordManagerTopic, true);
+  },
+
+  observe(subject, topic, data) {
+    // As soon as any login gets added or modified, disable undo:
+    // (Note that this ignores logins being removed as that doesn't
+    //  impair the 'undo' functionality of the import.)
+    if (kPasswordManagerTopicTypes.has(data)) {
+      this.removeUndoOption();
+    }
+  },
+
   /**
    * Automatically pick a migrator and resources to migrate,
    * then migrate those and start up.
    *
    * @throws if automatically deciding on migrators/data
    *         failed for some reason.
    */
   migrate(profileStartup, migratorKey, profileToMigrate) {
-    let histogram = Services.telemetry.getHistogramById("FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_PROCESS_SUCCESS");
+    let histogram = Services.telemetry.getHistogramById(
+      "FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_PROCESS_SUCCESS");
     histogram.add(0);
-    let migrator = this.pickMigrator(migratorKey);
+    let {migrator, pickedKey} = this.pickMigrator(migratorKey);
     histogram.add(5);
 
     profileToMigrate = this.pickProfile(migrator, profileToMigrate);
     histogram.add(10);
 
     let resourceTypes = migrator.getMigrateData(profileToMigrate, profileStartup);
     if (!(resourceTypes & this.resourceTypesToUse)) {
       throw new Error("No usable resources were found for the selected browser!");
     }
     histogram.add(15);
 
     let sawErrors = false;
-    let migrationObserver = function(subject, topic, data) {
+    let migrationObserver = (subject, topic, data) => {
       if (topic == "Migration:ItemError") {
         sawErrors = true;
       } else if (topic == "Migration:Ended") {
         histogram.add(25);
         if (sawErrors) {
           histogram.add(26);
         }
         Services.obs.removeObserver(migrationObserver, "Migration:Ended");
         Services.obs.removeObserver(migrationObserver, "Migration:ItemError");
         Services.prefs.setCharPref(kAutoMigrateStartedPref, startTime.toString());
         Services.prefs.setCharPref(kAutoMigrateFinishedPref, Date.now().toString());
+        Services.prefs.setCharPref(kAutoMigrateBrowserPref, pickedKey);
+        // Need to manually start listening to new bookmarks/logins being created,
+        // because, when we were initialized, there wasn't the possibility to
+        // 'undo' anything.
+        this.maybeInitUndoObserver();
       }
     };
 
     Services.obs.addObserver(migrationObserver, "Migration:Ended", false);
     Services.obs.addObserver(migrationObserver, "Migration:ItemError", false);
     // We'll save this when the migration has finished, at which point the pref
     // service will be available.
     let startTime = Date.now();
     migrator.migrate(this.resourceTypesToUse, profileStartup, profileToMigrate);
     histogram.add(20);
   },
 
   /**
    * Pick and return a migrator to use for automatically migrating.
    *
    * @param {String} migratorKey   optional, a migrator key to prefer/pick.
-   * @returns                      the migrator to use for migrating.
+   * @returns {Object}             an object with the migrator to use for migrating, as
+   *                               well as the key we eventually ended up using to obtain it.
    */
   pickMigrator(migratorKey) {
     if (!migratorKey) {
       let defaultKey = MigrationUtils.getMigratorKeyForDefaultBrowser();
       if (!defaultKey) {
         throw new Error("Could not determine default browser key to migrate from");
       }
       migratorKey = defaultKey;
@@ -87,17 +133,17 @@ const AutoMigrate = {
     if (migratorKey == "firefox") {
       throw new Error("Can't automatically migrate from Firefox.");
     }
 
     let migrator = MigrationUtils.getMigrator(migratorKey);
     if (!migrator) {
       throw new Error("Migrator specified or a default was found, but the migrator object is not available.");
     }
-    return migrator;
+    return {migrator, pickedKey: migratorKey};
   },
 
   /**
    * 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 profile to migrate, or null if migrating
@@ -122,18 +168,18 @@ const AutoMigrate = {
       throw new Error("Don't know how to pick a profile when more than 1 profile is present.");
     }
     return profiles ? profiles[0] : null;
   },
 
   getUndoRange() {
     let start, finish;
     try {
-      start = parseInt(Services.prefs.getCharPref(kAutoMigrateStartedPref), 10);
-      finish = parseInt(Services.prefs.getCharPref(kAutoMigrateFinishedPref), 10);
+      start = parseInt(Preferences.get(kAutoMigrateStartedPref, "0"), 10);
+      finish = parseInt(Preferences.get(kAutoMigrateFinishedPref, "0"), 10);
     } catch (ex) {
       Cu.reportError(ex);
     }
     if (!finish || !start) {
       return null;
     }
     return [new Date(start), new Date(finish)];
   },
@@ -182,14 +228,32 @@ const AutoMigrate = {
     histogram.add(20);
 
     try {
       Services.logins.removeAllLogins();
     } catch (ex) {
       // ignore failure.
     }
     histogram.add(25);
-    Services.prefs.clearUserPref("browser.migrate.automigrate-started");
-    Services.prefs.clearUserPref("browser.migrate.automigrate-finished");
+    this.removeUndoOption();
     histogram.add(30);
   }),
+
+  removeUndoOption() {
+    // Remove observers, and ensure that exceptions doing so don't break
+    // removing the pref.
+    try {
+      Services.obs.removeObserver(this, kPasswordManagerTopic);
+    } catch (ex) {}
+    try {
+      PlacesUtils.removeLazyBookmarkObserver(this);
+    } catch (ex) {}
+    Services.prefs.clearUserPref(kAutoMigrateStartedPref);
+    Services.prefs.clearUserPref(kAutoMigrateFinishedPref);
+    Services.prefs.clearUserPref(kAutoMigrateBrowserPref);
+  },
+
+  QueryInterface: XPCOMUtils.generateQI(
+    [Ci.nsIObserver, Ci.nsINavBookmarkObserver, Ci.nsISupportsWeakReference]
+  ),
 };
 
+AutoMigrate.init();
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -447,16 +447,46 @@ this.MigrationUtils = Object.freeze({
     aKey = OVERRIDES[aKey] || aKey;
 
     if (aReplacements === undefined)
       return getMigrationBundle().GetStringFromName(aKey);
     return getMigrationBundle().formatStringFromName(
       aKey, aReplacements, aReplacements.length);
   },
 
+  _getLocalePropertyForBrowser(browserId) {
+    switch (browserId) {
+      case "edge":
+        return "sourceNameEdge";
+      case "ie":
+        return "sourceNameIE";
+      case "safari":
+        return "sourceNameSafari";
+      case "canary":
+        return "sourceNameCanary";
+      case "chrome":
+        return "sourceNameChrome";
+      case "chromium":
+        return "sourceNameChromium";
+      case "firefox":
+        return "sourceNameFirefox";
+      case "360se":
+        return "sourceName360se";
+    }
+    return null;
+  },
+
+  getBrowserName(browserId) {
+    let prop = this._getLocalePropertyForBrowser(browserId);
+    if (prop) {
+      return this.getLocalizedString(prop);
+    }
+    return null;
+  },
+
   /**
    * Helper for creating a folder for imported bookmarks from a particular
    * migration source.  The folder is created at the end of the given folder.
    *
    * @param sourceNameStr
    *        the source name (first letter capitalized).  This is used
    *        for reading the localized source name from the migration
    *        bundle (e.g. if aSourceNameStr is Mosaic, this will try to read
@@ -711,18 +741,17 @@ this.MigrationUtils = Object.freeze({
         this.finishMigration();
         return;
       }
     }
 
     let isRefresh = migrator && skipSourcePage &&
                     migratorKey == AppConstants.MOZ_APP_NAME;
 
-    if (!isRefresh &&
-        Services.prefs.getBoolPref("browser.migration.automigrate")) {
+    if (!isRefresh && AutoMigrate.enabled) {
       try {
         AutoMigrate.migrate(aProfileStartup, aMigratorKey, aProfileToMigrate);
         return;
       } catch (ex) {
         // If automigration failed, continue and show the dialog.
         Cu.reportError(ex);
       }
     }
--- a/browser/components/migration/content/migration.js
+++ b/browser/components/migration/content/migration.js
@@ -309,48 +309,24 @@ var MigrationWizard = {
     document.getElementById("homePageImportDesc").setAttribute("value", pageDesc);
 
     this._wiz._adjustWizardHeader();
 
     var singleStart = document.getElementById("homePageSingleStart");
     singleStart.setAttribute("label", mainStr);
     singleStart.setAttribute("value", "DEFAULT");
 
-    var source = null;
-    switch (this._source) {
-      case "ie":
-        source = "sourceNameIE";
-        break;
-      case "safari":
-        source = "sourceNameSafari";
-        break;
-      case "canary":
-        source = "sourceNameCanary";
-        break;
-      case "chrome":
-        source = "sourceNameChrome";
-        break;
-      case "chromium":
-        source = "sourceNameChromium";
-        break;
-      case "firefox":
-        source = "sourceNameFirefox";
-        break;
-      case "360se":
-        source = "sourceName360se";
-        break;
-    }
+    var appName = MigrationUtils.getBrowserName(this._source);
 
     // semi-wallpaper for crash when multiple profiles exist, since we haven't initialized mSourceProfile in places
     this._migrator.getMigrateData(this._selectedProfile, this._autoMigrate);
 
     var oldHomePageURL = this._migrator.sourceHomePageURL;
 
-    if (oldHomePageURL && source) {
-      var appName = MigrationUtils.getLocalizedString(source);
+    if (oldHomePageURL && appName) {
       var oldHomePageLabel =
         brandBundle.getFormattedString("homePageImport", [appName]);
       var oldHomePage = document.getElementById("oldHomePage");
       oldHomePage.setAttribute("label", oldHomePageLabel);
       oldHomePage.setAttribute("value", oldHomePageURL);
       oldHomePage.removeAttribute("hidden");
     }
     else {
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -1,10 +1,11 @@
 Cu.import("resource:///modules/MigrationUtils.jsm");
 Cu.import("resource://gre/modules/PlacesUtils.jsm");
+Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://testing-common/TestUtils.jsm");
 Cu.import("resource://testing-common/PlacesTestUtils.jsm");
 let AutoMigrateBackstage = Cu.import("resource:///modules/AutoMigrate.jsm");
 
 let gShimmedMigratorKeyPicker = null;
 let gShimmedMigrator = null;
 
 const kUsecPerMin = 60 * 1000000;
@@ -155,19 +156,19 @@ add_task(function* checkUndoPrecondition
                      "getMigrateData called with 'null' as a profile");
 
   let {BOOKMARKS, HISTORY, PASSWORDS} = Ci.nsIBrowserProfileMigrator;
   let expectedTypes = BOOKMARKS | HISTORY | PASSWORDS;
   Assert.deepEqual(gShimmedMigrator._migrateArgs, [expectedTypes, "startup", null],
                    "migrate called with 'null' as a profile");
 
   yield migrationFinishedPromise;
-  Assert.ok(Services.prefs.getPrefType("browser.migrate.automigrate-started"),
+  Assert.ok(Preferences.has("browser.migrate.automigrate.started"),
             "Should have set start time pref");
-  Assert.ok(Services.prefs.getPrefType("browser.migrate.automigrate-finished"),
+  Assert.ok(Preferences.has("browser.migrate.automigrate.finished"),
             "Should have set finish time pref");
   Assert.ok((yield AutoMigrate.canUndo()), "Should be able to undo migration");
 
   let [beginRange, endRange] = AutoMigrate.getUndoRange();
   let stringRange = `beginRange: ${beginRange}; endRange: ${endRange}`;
   Assert.ok(beginRange <= endRange,
             "Migration should have started before or when it ended " + stringRange);
 
@@ -175,17 +176,16 @@ add_task(function* checkUndoPrecondition
   Assert.ok(true, "Should be able to finish an undo cycle.");
 });
 
 /**
  * Fake a migration and then try to undo it to verify all data gets removed.
  */
 add_task(function* checkUndoRemoval() {
   let startTime = "" + Date.now();
-  Services.prefs.setCharPref("browser.migrate.automigrate-started", startTime);
 
   // Insert a login and check that that worked.
   let login = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
   login.init("www.mozilla.org", "http://www.mozilla.org", null, "user", "pass", "userEl", "passEl");
   Services.logins.addLogin(login);
   let storedLogins = Services.logins.findLogins({}, "www.mozilla.org",
                                                 "http://www.mozilla.org", null);
   Assert.equal(storedLogins.length, 1, "Should have 1 login");
@@ -217,17 +217,18 @@ add_task(function* checkUndoRemoval() {
   let visits = PlacesUtils.history.executeQuery(query, opts);
   visits.root.containerOpen = true;
   Assert.equal(visits.root.childCount, 2, "Should have 2 visits");
   // Clean up:
   visits.root.containerOpen = false;
 
   // Now set finished pref:
   let endTime = "" + Date.now();
-  Services.prefs.setCharPref("browser.migrate.automigrate-finished", endTime);
+  Preferences.set("browser.migrate.automigrate.started", startTime);
+  Preferences.set("browser.migrate.automigrate.finished", endTime);
 
   // Verify that we can undo, then undo:
   Assert.ok(yield AutoMigrate.canUndo(), "Should be possible to undo migration");
   yield AutoMigrate.undo();
 
   // Check that the undo removed the history visits:
   visits = PlacesUtils.history.executeQuery(query, opts);
   visits.root.containerOpen = true;
@@ -239,13 +240,49 @@ add_task(function* checkUndoRemoval() {
   Assert.ok(!bookmark, "Should have no bookmarks after undo");
 
   // Check that the undo removed the passwords:
   storedLogins = Services.logins.findLogins({}, "www.mozilla.org",
                                             "http://www.mozilla.org", null);
   Assert.equal(storedLogins.length, 0, "Should have no logins");
 
   // Finally check prefs got cleared:
-  Assert.ok(!Services.prefs.getPrefType("browser.migrate.automigrate-started"),
+  Assert.ok(!Preferences.has("browser.migrate.automigrate.started"),
             "Should no longer have pref for migration start time.");
-  Assert.ok(!Services.prefs.getPrefType("browser.migrate.automigrate-finished"),
+  Assert.ok(!Preferences.has("browser.migrate.automigrate.finished"),
             "Should no longer have pref for migration finish time.");
 });
+
+add_task(function* checkUndoDisablingByBookmarksAndPasswords() {
+  let startTime = "" + Date.now();
+  Services.prefs.setCharPref("browser.migrate.automigrate.started", startTime);
+  // Now set finished pref:
+  let endTime = "" + (Date.now() + 1000);
+  Services.prefs.setCharPref("browser.migrate.automigrate.finished", endTime);
+  AutoMigrate.maybeInitUndoObserver();
+
+  ok((yield AutoMigrate.canUndo()), "Should be able to undo.");
+
+  // Insert a login and check that that disabled undo.
+  let login = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
+  login.init("www.mozilla.org", "http://www.mozilla.org", null, "user", "pass", "userEl", "passEl");
+  Services.logins.addLogin(login);
+
+  ok(!(yield AutoMigrate.canUndo()), "Should no longer be able to undo.");
+  Services.prefs.setCharPref("browser.migrate.automigrate.started", startTime);
+  Services.prefs.setCharPref("browser.migrate.automigrate.finished", endTime);
+  ok((yield AutoMigrate.canUndo()), "Should be able to undo.");
+  AutoMigrate.maybeInitUndoObserver();
+
+  // Insert a bookmark and check that that disabled undo.
+  yield PlacesUtils.bookmarks.insert({
+    parentGuid: PlacesUtils.bookmarks.toolbarGuid,
+    url: "http://www.example.org/",
+    title: "Some example bookmark",
+  });
+  ok(!(yield AutoMigrate.canUndo()), "Should no longer be able to undo.");
+
+  try {
+    Services.logins.removeAllLogins();
+  } catch (ex) {}
+  yield PlacesUtils.bookmarks.eraseEverything();
+});
+
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -793,16 +793,19 @@ BrowserGlue.prototype = {
     Feeds.init();
     ContentPrefServiceParent.init();
 
     LoginManagerParent.init();
     ReaderParent.init();
 
     SelfSupportBackend.init();
 
+    // Ensure we keep track of places/pw-mananager undo by init'ing this early.
+    Cu.import("resource:///modules/AutoMigrate.jsm");
+
     if (!AppConstants.RELEASE_BUILD) {
       let themeName = gBrowserBundle.GetStringFromName("deveditionTheme.name");
       let vendorShortName = gBrandBundle.GetStringFromName("vendorShortName");
 
       LightweightThemeManager.addBuiltInTheme({
         id: "firefox-devedition@mozilla.org",
         name: themeName,
         headerURL: "resource:///chrome/browser/content/browser/defaultthemes/devedition.header.png",