Bug 1285577 - part 2: keep track of added logins in an import and allow removing them, r=MattN
MozReview-Commit-ID: JZbOkmZ7ZZG
--- a/browser/components/migration/AutoMigrate.jsm
+++ b/browser/components/migration/AutoMigrate.jsm
@@ -28,16 +28,18 @@ const kSyncTopic = "fxaccounts:onlogin";
const kNotificationId = "abouthome-automigration-undo";
Cu.import("resource:///modules/MigrationUtils.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/Task.jsm");
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper",
+ "resource://gre/modules/LoginHelper.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm");
const AutoMigrate = {
get resourceTypesToUse() {
let {BOOKMARKS, HISTORY, PASSWORDS} = Ci.nsIBrowserProfileMigrator;
return BOOKMARKS | HISTORY | PASSWORDS;
},
@@ -433,14 +435,27 @@ const AutoMigrate = {
yield PlacesUtils.bookmarks.remove(guid, {preventRemovalOfNonEmptyFolders: true}).catch(err => {
if (err && err.message != "Cannot remove a non-empty folder.") {
Cu.reportError(err);
}
});
}
}),
+ _removeUnchangedLogins: Task.async(function* (logins) {
+ for (let login of logins) {
+ let foundLogins = LoginHelper.searchLoginsWithObject({guid: login.guid});
+ if (foundLogins.length) {
+ let foundLogin = foundLogins[0];
+ foundLogin.QueryInterface(Ci.nsILoginMetaInfo);
+ if (foundLogin.timePasswordChanged == login.timePasswordChanged) {
+ Services.logins.removeLogin(foundLogin);
+ }
+ }
+ }
+ }),
+
QueryInterface: XPCOMUtils.generateQI(
[Ci.nsIObserver, Ci.nsINavBookmarkObserver, Ci.nsISupportsWeakReference]
),
};
AutoMigrate.init();
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -969,17 +969,25 @@ this.MigrationUtils = Object.freeze({
insertVisitsWrapper(places, options) {
this._importQuantities.history += places.length;
return PlacesUtils.asyncHistory.updatePlaces(places, options);
},
insertLoginWrapper(login) {
this._importQuantities.logins++;
- return LoginHelper.maybeImportLogin(login);
+ let insertedLogin = LoginHelper.maybeImportLogin(login);
+ // Note that this means that if we import a login that has a newer password
+ // than we know about, we will update the login, and an undo of the import
+ // will not revert this. This seems preferable over removing the login
+ // outright or storing the old password in the undo file.
+ if (insertedLogin && gKeepUndoData) {
+ let {guid, timePasswordChanged} = insertedLogin;
+ gUndoData.get("logins").push({guid, timePasswordChanged});
+ }
},
initializeUndoData() {
gKeepUndoData = true;
gUndoData = new Map([["bookmarks", []], ["visits", new Map()], ["logins", []]]);
},
_postProcessUndoData: Task.async(function*(state) {
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -1,11 +1,12 @@
"use strict";
Cu.import("resource:///modules/MigrationUtils.jsm");
+Cu.import("resource://gre/modules/LoginHelper.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"); /* globals AutoMigrate */
let gShimmedMigratorKeyPicker = null;
let gShimmedMigrator = null;
@@ -379,8 +380,63 @@ add_task(function* testBookmarkRemovalBy
Assert.ok(itemFromDB, "Folder we inserted in migration is still there because of new kids.");
for (let removedGuid of itemsToRemove) {
let dbResultForGuid = yield PlacesUtils.bookmarks.fetch(removedGuid);
let dbgStr = dbResultForGuid && dbResultForGuid.title;
Assert.equal(null, dbResultForGuid, "Should not be able to find items that should have been removed, but found " + dbgStr);
}
yield PlacesUtils.bookmarks.eraseEverything();
});
+
+add_task(function* checkUndoLoginsState() {
+ MigrationUtils.initializeUndoData();
+ MigrationUtils.insertLoginWrapper({
+ username: "foo",
+ password: "bar",
+ hostname: "https://example.com",
+ formSubmitURL: "https://example.com/",
+ timeCreated: new Date(),
+ });
+ let storedLogins = Services.logins.findLogins({}, "https://example.com", "", "");
+ let storedLogin = storedLogins[0];
+ storedLogin.QueryInterface(Ci.nsILoginMetaInfo);
+ let {guid, timePasswordChanged} = storedLogin;
+ let undoLoginData = (yield MigrationUtils.stopAndRetrieveUndoData()).get("logins");
+ Assert.deepEqual([{guid, timePasswordChanged}], undoLoginData);
+ Services.logins.removeAllLogins();
+});
+
+add_task(function* testLoginsRemovalByUndo() {
+ MigrationUtils.initializeUndoData();
+ MigrationUtils.insertLoginWrapper({
+ username: "foo",
+ password: "bar",
+ hostname: "https://example.com",
+ formSubmitURL: "https://example.com/",
+ timeCreated: new Date(),
+ });
+ MigrationUtils.insertLoginWrapper({
+ username: "foo",
+ password: "bar",
+ hostname: "https://example.org",
+ formSubmitURL: "https://example.org/",
+ timeCreated: new Date(new Date().getTime() - 10000),
+ });
+ // This should update the existing login
+ LoginHelper.maybeImportLogin({
+ username: "foo",
+ password: "bazzy",
+ hostname: "https://example.org",
+ formSubmitURL: "https://example.org/",
+ timePasswordChanged: new Date(),
+ });
+ Assert.equal(1, LoginHelper.searchLoginsWithObject({hostname: "https://example.org", formSubmitURL: "https://example.org/"}).length,
+ "Should be only 1 login for example.org (that was updated)");
+ let undoLoginData = (yield MigrationUtils.stopAndRetrieveUndoData()).get("logins");
+
+ yield AutoMigrate._removeUnchangedLogins(undoLoginData);
+ Assert.equal(0, LoginHelper.searchLoginsWithObject({hostname: "https://example.com", formSubmitURL: "https://example.com/"}).length,
+ "unchanged example.com entry should have been removed.");
+ Assert.equal(1, LoginHelper.searchLoginsWithObject({hostname: "https://example.org", formSubmitURL: "https://example.org/"}).length,
+ "changed example.org entry should have persisted.");
+ Services.logins.removeAllLogins();
+});
+