Bug 1289906 - part 1: use an observer to know when the user signs into sync after an undo, r?markh
MozReview-Commit-ID: BPjUVvYdsRG
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -18,16 +18,21 @@ const kAutoMigrateLastUndoPromptDateMsPr
const kAutoMigrateDaysToOfferUndoPref = "browser.migrate.automigrate.daysToOfferUndo";
const kPasswordManagerTopic = "passwordmgr-storage-changed";
const kPasswordManagerTopicTypes = new Set([
"addLogin",
"modifyLogin",
]);
+const kSyncTopics = new Set([
+ "weave:service:setup-complete",
+ "weave:service:login:finish",
+]);
+
const kNotificationId = "abouthome-automigration-undo";
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");
@@ -41,33 +46,39 @@ const AutoMigrate = {
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()) {
+ if (!this.canUndo()) {
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);
+ // And sync ones:
+ for (let topic of kSyncTopics) {
+ Services.obs.addObserver(this, topic, 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)) {
+ if (topic == kPasswordManagerTopic) {
+ // 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();
+ }
+ } else if (kSyncTopics.has(topic)) {
this.removeUndoOption();
}
},
/**
* Automatically pick a migrator and resources to migrate,
* then migrate those and start up.
*
@@ -185,34 +196,23 @@ const AutoMigrate = {
}
if (!finish || !start) {
return null;
}
return [new Date(start), new Date(finish)];
},
canUndo() {
- if (!this.getUndoRange()) {
- return Promise.resolve(false);
- }
- // Return a promise resolving to false if we're signed into sync, resolve
- // to true otherwise.
- let {fxAccounts} = Cu.import("resource://gre/modules/FxAccounts.jsm", {});
- return fxAccounts.getSignedInUser().then(user => {
- if (user) {
- Services.telemetry.getHistogramById("FX_STARTUP_MIGRATION_CANT_UNDO_BECAUSE_SYNC").add(true);
- }
- return !user;
- }, () => Promise.resolve(true));
+ return !!this.getUndoRange();
},
undo: Task.async(function* () {
let histogram = Services.telemetry.getHistogramById("FX_STARTUP_MIGRATION_AUTOMATED_IMPORT_UNDO");
histogram.add(0);
- if (!(yield this.canUndo())) {
+ if (!this.canUndo()) {
histogram.add(5);
throw new Error("Can't undo!");
}
histogram.add(10);
yield PlacesUtils.bookmarks.eraseEverything();
histogram.add(15);
@@ -241,16 +241,21 @@ const AutoMigrate = {
this.removeUndoOption();
histogram.add(30);
}),
removeUndoOption() {
// Remove observers, and ensure that exceptions doing so don't break
// removing the pref.
try {
+ for (let topic of kSyncTopics) {
+ Services.obs.removeObserver(this, kSyncTopics);
+ }
+ } catch (ex) {}
+ 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);
@@ -274,64 +279,63 @@ const AutoMigrate = {
let browserId = Services.prefs.getCharPref(kAutoMigrateBrowserPref);
if (browserId) {
return MigrationUtils.getBrowserName(browserId);
}
return null;
},
maybeShowUndoNotification(target) {
- this.canUndo().then(canUndo => {
- // The tab might have navigated since we requested the undo state:
- if (!canUndo || target.currentURI.spec != "about:home") {
- return;
- }
- let win = target.ownerGlobal;
- let notificationBox = win.gBrowser.getNotificationBox(target);
- if (!notificationBox || notificationBox.getNotificationWithValue("abouthome-automigration-undo")) {
- return;
- }
+ // The tab might have navigated since we requested the undo state:
+ if (!this.canUndo() || target.currentURI.spec != "about:home") {
+ return;
+ }
- // At this stage we're committed to show the prompt - unless we shouldn't,
- // in which case we remove the undo prefs (which will cause canUndo() to
- // return false from now on.):
- if (!this.shouldStillShowUndoPrompt()) {
- this.removeUndoOption();
- return;
- }
+ let win = target.ownerGlobal;
+ let notificationBox = win.gBrowser.getNotificationBox(target);
+ if (!notificationBox || notificationBox.getNotificationWithValue("abouthome-automigration-undo")) {
+ return;
+ }
+
+ // At this stage we're committed to show the prompt - unless we shouldn't,
+ // in which case we remove the undo prefs (which will cause canUndo() to
+ // return false from now on.):
+ if (!this.shouldStillShowUndoPrompt()) {
+ this.removeUndoOption();
+ return;
+ }
- let browserName = this.getBrowserUsedForMigration();
- let message;
- if (browserName) {
- message = MigrationUtils.getLocalizedString("automigration.undo.message",
- [browserName]);
- } else {
- message = MigrationUtils.getLocalizedString("automigration.undo.unknownBrowserMessage");
- }
+ let browserName = this.getBrowserUsedForMigration();
+ let message;
+ if (browserName) {
+ message = MigrationUtils.getLocalizedString("automigration.undo.message",
+ [browserName]);
+ } else {
+ message = MigrationUtils.getLocalizedString("automigration.undo.unknownBrowserMessage");
+ }
- let buttons = [
- {
- label: MigrationUtils.getLocalizedString("automigration.undo.keep.label"),
- accessKey: MigrationUtils.getLocalizedString("automigration.undo.keep.accesskey"),
- callback: () => {
- this.removeUndoOption();
- },
+ let buttons = [
+ {
+ label: MigrationUtils.getLocalizedString("automigration.undo.keep.label"),
+ accessKey: MigrationUtils.getLocalizedString("automigration.undo.keep.accesskey"),
+ callback: () => {
+ this.removeUndoOption();
},
- {
- label: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.label"),
- accessKey: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.accesskey"),
- callback: () => {
- this.undo();
- },
+ },
+ {
+ label: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.label"),
+ accessKey: MigrationUtils.getLocalizedString("automigration.undo.dontkeep.accesskey"),
+ callback: () => {
+ this.undo();
},
- ];
- notificationBox.appendNotification(
- message, kNotificationId, null, notificationBox.PRIORITY_INFO_HIGH, buttons
- );
- });
+ },
+ ];
+ notificationBox.appendNotification(
+ message, kNotificationId, null, notificationBox.PRIORITY_INFO_HIGH, buttons
+ );
},
shouldStillShowUndoPrompt() {
let today = new Date();
// Round down to midnight:
today = new Date(today.getFullYear(), today.getMonth(), today.getDate());
// We store the unix timestamp corresponding to midnight on the last day
// on which we prompted. Fetch that and compare it to today's date.
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -160,17 +160,17 @@ add_task(function* checkUndoPrecondition
Assert.deepEqual(gShimmedMigrator._migrateArgs, [expectedTypes, "startup", null],
"migrate called with 'null' as a profile");
yield migrationFinishedPromise;
Assert.ok(Preferences.has("browser.migrate.automigrate.started"),
"Should have set start time pref");
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");
+ Assert.ok(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);
yield AutoMigrate.undo();
Assert.ok(true, "Should be able to finish an undo cycle.");
@@ -221,17 +221,17 @@ add_task(function* checkUndoRemoval() {
visits.root.containerOpen = false;
// Now set finished pref:
let endTime = "" + Date.now();
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");
+ Assert.ok(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;
Assert.equal(visits.root.childCount, 0, "Should have no more visits");
visits.root.containerOpen = false;
@@ -254,35 +254,35 @@ add_task(function* checkUndoRemoval() {
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.");
+ ok(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.");
+ ok(!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.");
+ ok(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.");
+ ok(!AutoMigrate.canUndo(), "Should no longer be able to undo.");
try {
Services.logins.removeAllLogins();
} catch (ex) {}
yield PlacesUtils.bookmarks.eraseEverything();
});
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -4652,24 +4652,16 @@
"bug_numbers": [1283565],
"alert_emails": ["gijs@mozilla.com"],
"expires_in_version": "53",
"kind": "enumerated",
"n_values": 31,
"releaseChannelCollection": "opt-out",
"description": "Where undo of the automatic migration was attempted, indicates to what degree we succeeded to undo. 0 means we started to undo, 5 means we bailed out from the undo because it was not possible to complete it (there was nothing to undo or the user was signed in to sync). All higher values indicate progression through the undo sequence, with 30 indicating we finished the undo without exceptions in the middle."
},
- "FX_STARTUP_MIGRATION_CANT_UNDO_BECAUSE_SYNC": {
- "bug_numbers": [1283565],
- "alert_emails": ["gijs@mozilla.com"],
- "expires_in_version": "53",
- "kind": "boolean",
- "releaseChannelCollection": "opt-out",
- "description": "Where undo of the automatic migration was requested and denied because the user was logged into sync."
- },
"FX_STARTUP_MIGRATION_DATA_RECENCY": {
"bug_numbers": [1276694],
"alert_emails": ["gijs@mozilla.com"],
"expires_in_version": "53",
"keyed": true,
"kind": "exponential",
"n_buckets": 50,
"high": 8760,