Backed out changeset ebbcdc3e642d (bug 1433593) for X failing in toolkit/components/passwordmgr/test/unit/test_maybeImportLogin.js on a CLOSED TREE
authorMargareta Eliza Balazs <ebalazs@mozilla.com>
Fri, 16 Feb 2018 04:50:42 +0200
changeset 404130 7194edc95a6e3fddb782154e757e5f6b01ca1ab1
parent 404129 eeae793339e7e909bfe3fbcc8cf9732552d02c5d
child 404131 0854548560aa2310ca47b53fe41d9b9651391db5
push id99938
push usercbrindusan@mozilla.com
push dateFri, 16 Feb 2018 09:57:26 +0000
treeherdermozilla-inbound@5f2344531e28 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1433593
milestone60.0a1
backs outebbcdc3e642dd03221d41dabd8fd398681a24a75
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
Backed out changeset ebbcdc3e642d (bug 1433593) for X failing in toolkit/components/passwordmgr/test/unit/test_maybeImportLogin.js on a CLOSED TREE
browser/components/migration/IEProfileMigrator.js
browser/components/migration/MSMigrationUtils.jsm
browser/components/migration/MigrationUtils.jsm
browser/components/migration/tests/unit/test_automigration.js
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/nsLoginManager.js
toolkit/components/passwordmgr/test/unit/test_maybeImportLogin.js
--- a/browser/components/migration/IEProfileMigrator.js
+++ b/browser/components/migration/IEProfileMigrator.js
@@ -117,41 +117,41 @@ IE7FormPasswords.prototype = {
       let count = key.valueCount;
       key.close();
       return count > 0;
     } catch (e) {
       return false;
     }
   },
 
-  async migrate(aCallback) {
+  migrate(aCallback) {
     let historyEnumerator = Cc["@mozilla.org/profile/migrator/iehistoryenumerator;1"].
                             createInstance(Ci.nsISimpleEnumerator);
     let uris = []; // the uris of the websites that are going to be migrated
     while (historyEnumerator.hasMoreElements()) {
       let entry = historyEnumerator.getNext().QueryInterface(Ci.nsIPropertyBag2);
       let uri = entry.get("uri").QueryInterface(Ci.nsIURI);
       // MSIE stores some types of URLs in its history that we don't handle, like HTMLHelp
       // and others. Since we are not going to import the logins that are performed in these URLs
       // we can just skip them.
       if (!["http", "https", "ftp"].includes(uri.scheme)) {
         continue;
       }
 
       uris.push(uri);
     }
-    await this._migrateURIs(uris);
+    this._migrateURIs(uris);
     aCallback(true);
   },
 
   /**
    * Migrate the logins that were saved for the uris arguments.
    * @param {nsIURI[]} uris - the uris that are going to be migrated.
    */
-  async _migrateURIs(uris) {
+  _migrateURIs(uris) {
     this.ctypesKernelHelpers = new MSMigrationUtils.CtypesKernelHelpers();
     this._crypto = new OSCrypto();
     let nsIWindowsRegKey = Ci.nsIWindowsRegKey;
     let key = Cc["@mozilla.org/windows-registry-key;1"].
               createInstance(nsIWindowsRegKey);
     key.open(nsIWindowsRegKey.ROOT_KEY_CURRENT_USER, kLoginsKey,
              nsIWindowsRegKey.ACCESS_READ);
 
@@ -161,17 +161,16 @@ IE7FormPasswords.prototype = {
     /* The logins are stored in the registry, where the key is a hashed URL and its
      * value contains the encrypted details for all logins for that URL.
      *
      * First iterate through IE history, hashing each URL and looking for a match. If
      * found, decrypt the value, using the URL as a salt. Finally add any found logins
      * to the Firefox password manager.
      */
 
-    let logins = [];
     for (let uri of uris) {
       try {
         // remove the query and the ref parts of the URL
         let urlObject = new URL(uri.spec);
         let url = urlObject.origin + urlObject.pathname;
         // if the current url is already processed, it should be skipped
         if (urlsSet.has(url)) {
           continue;
@@ -196,33 +195,21 @@ IE7FormPasswords.prototype = {
         }
         // extract the login details from the decrypted data
         let ieLogins = this._extractDetails(data, uri);
         // if at least a credential was found in the current data, successfullyDecryptedValues should
         // be incremented by one
         if (ieLogins.length) {
           successfullyDecryptedValues++;
         }
-        for (let ieLogin of ieLogins) {
-          logins.push({
-            username: ieLogin.username,
-            password: ieLogin.password,
-            hostname: ieLogin.url,
-            timeCreated: ieLogin.creation,
-          });
-        }
+        this._addLogins(ieLogins);
       } catch (e) {
         Cu.reportError("Error while importing logins for " + uri.spec + ": " + e);
       }
     }
-
-    if (logins.length > 0) {
-      await MigrationUtils.insertLoginsWrapper(logins);
-    }
-
     // if the number of the imported values is less than the number of values in the key, it means
     // that not all the values were imported and an error should be reported
     if (successfullyDecryptedValues < key.valueCount) {
       Cu.reportError("We failed to decrypt and import some logins. " +
                      "This is likely because we didn't find the URLs where these " +
                      "passwords were submitted in the IE history and which are needed to be used " +
                      "as keys in the decryption.");
     }
@@ -230,16 +217,37 @@ IE7FormPasswords.prototype = {
     key.close();
     this._crypto.finalize();
     this.ctypesKernelHelpers.finalize();
   },
 
   _crypto: null,
 
   /**
+   * Add the logins to the password manager.
+   * @param {Object[]} logins - array of the login details.
+   */
+  _addLogins(ieLogins) {
+    for (let ieLogin of ieLogins) {
+      try {
+        // create a new login
+        let login = {
+          username: ieLogin.username,
+          password: ieLogin.password,
+          hostname: ieLogin.url,
+          timeCreated: ieLogin.creation,
+        };
+        MigrationUtils.insertLoginWrapper(login);
+      } catch (e) {
+        Cu.reportError(e);
+      }
+    }
+  },
+
+  /**
    * Extract the details of one or more logins from the raw decrypted data.
    * @param {string} data - the decrypted data containing raw information.
    * @param {nsURI} uri - the nsURI of page where the login has occur.
    * @returns {Object[]} array of objects where each of them contains the username, password, URL,
    * and creation time representing all the logins found in the data arguments.
    */
   _extractDetails(data, uri) {
     // the structure of the header of the IE7 decrypted data for all the logins sharing the same URL
--- a/browser/components/migration/MSMigrationUtils.jsm
+++ b/browser/components/migration/MSMigrationUtils.jsm
@@ -743,17 +743,17 @@ WindowsVaultFormPasswords.prototype = {
    * Otherwise, check if there are passwords in the vault.
    * @param {function} aCallback - a callback called when the migration is done.
    * @param {boolean} [aOnlyCheckExists=false] - if aOnlyCheckExists is true, just check if there are some
    * passwords to migrate. Import the passwords from the vault and call aCallback otherwise.
    * @return true if there are passwords in the vault and aOnlyCheckExists is set to true,
    * false if there is no password in the vault and aOnlyCheckExists is set to true, undefined if
    * aOnlyCheckExists is set to false.
    */
-  async migrate(aCallback, aOnlyCheckExists = false) {
+  migrate(aCallback, aOnlyCheckExists = false) {
     // check if the vault item is an IE/Edge one
     function _isIEOrEdgePassword(id) {
       return id[0] == INTERNET_EXPLORER_EDGE_GUID[0] &&
              id[1] == INTERNET_EXPLORER_EDGE_GUID[1] &&
              id[2] == INTERNET_EXPLORER_EDGE_GUID[2] &&
              id[3] == INTERNET_EXPLORER_EDGE_GUID[3];
     }
 
@@ -780,18 +780,16 @@ WindowsVaultFormPasswords.prototype = {
       // enumerate all the available items. This api is going to return a table of all the
       // available items and item is going to point to the first element of this table.
       error = ctypesVaultHelpers._functions.VaultEnumerateItems(vault, VAULT_ENUMERATE_ALL_ITEMS,
                                                                 itemCount.address(),
                                                                 item.address());
       if (error != RESULT_SUCCESS) {
         throw new Error("Unable to enumerate Vault items: " + error);
       }
-
-      let logins = [];
       for (let j = 0; j < itemCount.value; j++) {
         try {
           // if it's not an ie/edge password, skip it
           if (!_isIEOrEdgePassword(item.contents.schemaId.id)) {
             continue;
           }
           let url = item.contents.pResourceElement.contents.itemValue.readString();
           let realURL;
@@ -827,39 +825,36 @@ WindowsVaultFormPasswords.prototype = {
             // to seconds since epoch and multiply to get milliseconds:
             creation = ctypesKernelHelpers.
                          fileTimeToSecondsSinceEpoch(item.contents.highLastModified,
                                                      item.contents.lowLastModified) * 1000;
           } catch (ex) {
             // Ignore exceptions in the dates and just create the login for right now.
           }
           // create a new login
-          logins.push({
+          let login = {
             username, password,
             hostname: realURL.prePath,
             timeCreated: creation,
-          });
+          };
+          MigrationUtils.insertLoginWrapper(login);
 
           // close current item
           error = ctypesVaultHelpers._functions.VaultFree(credential);
           if (error == FREE_CLOSE_FAILED) {
             throw new Error("Unable to free item: " + error);
           }
         } catch (e) {
           migrationSucceeded = false;
           Cu.reportError(e);
         } finally {
           // move to next item in the table returned by VaultEnumerateItems
           item = item.increment();
         }
       }
-
-      if (logins.length > 0) {
-        await MigrationUtils.insertLoginsWrapper(logins);
-      }
     } catch (e) {
       Cu.reportError(e);
       migrationSucceeded = false;
     } finally {
       if (successfulVaultOpen) {
         // close current vault
         error = ctypesVaultHelpers._functions.VaultCloseVault(vault);
         if (error == FREE_CLOSE_FAILED) {
--- a/browser/components/migration/MigrationUtils.jsm
+++ b/browser/components/migration/MigrationUtils.jsm
@@ -1029,16 +1029,29 @@ this.MigrationUtils = Object.freeze({
   insertVisitsWrapper(places, options) {
     this._importQuantities.history += places.length;
     if (gKeepUndoData) {
       this._updateHistoryUndo(places);
     }
     return PlacesUtils.asyncHistory.updatePlaces(places, options, true);
   },
 
+  insertLoginWrapper(login) {
+    this._importQuantities.logins++;
+    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});
+    }
+  },
+
   async insertLoginsWrapper(logins) {
     this._importQuantities.logins += logins.length;
     let inserted = await LoginHelper.maybeImportLogins(logins);
     // 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 (gKeepUndoData) {
--- a/browser/components/migration/tests/unit/test_automigration.js
+++ b/browser/components/migration/tests/unit/test_automigration.js
@@ -163,26 +163,26 @@ add_task(async function checkUndoPrecond
     getSourceProfiles() {
       info("Read sourceProfiles");
       return null;
     },
     getMigrateData(profileToMigrate) {
       this._getMigrateDataArgs = profileToMigrate;
       return Ci.nsIBrowserProfileMigrator.BOOKMARKS;
     },
-    async migrate(types, startup, profileToMigrate) {
+    migrate(types, startup, profileToMigrate) {
       this._migrateArgs = [types, startup, profileToMigrate];
       if (shouldAddData) {
         // Insert a login and check that that worked.
-        await MigrationUtils.insertLoginsWrapper([{
+        MigrationUtils.insertLoginWrapper({
           hostname: "www.mozilla.org",
           formSubmitURL: "http://www.mozilla.org",
           username: "user",
           password: "pass",
-        }]);
+        });
       }
       TestUtils.executeSoon(function() {
         Services.obs.notifyObservers(null, "Migration:Ended", undefined);
       });
     },
   };
 
   gShimmedMigratorKeyPicker = function() {
@@ -234,22 +234,22 @@ add_task(async function checkUndoPrecond
 
 /**
  * Fake a migration and then try to undo it to verify all data gets removed.
  */
 add_task(async function checkUndoRemoval() {
   MigrationUtils.initializeUndoData();
   Preferences.set("browser.migrate.automigrate.browser", "automationbrowser");
   // Insert a login and check that that worked.
-  await MigrationUtils.insertLoginsWrapper([{
+  MigrationUtils.insertLoginWrapper({
     hostname: "www.mozilla.org",
     formSubmitURL: "http://www.mozilla.org",
     username: "user",
     password: "pass",
-  }]);
+  });
   let storedLogins = Services.logins.findLogins({}, "www.mozilla.org",
                                                 "http://www.mozilla.org", null);
   Assert.equal(storedLogins.length, 1, "Should have 1 login");
 
   // Insert a bookmark and check that we have exactly 1 bookmark for that URI.
   await MigrationUtils.insertBookmarkWrapper({
     parentGuid: PlacesUtils.bookmarks.toolbarGuid,
     url: "http://www.example.org/",
@@ -436,56 +436,56 @@ add_task(async function testBookmarkRemo
     let dbgStr = dbResultForGuid && dbResultForGuid.title;
     Assert.equal(null, dbResultForGuid, "Should not be able to find items that should have been removed, but found " + dbgStr);
   }
   await PlacesUtils.bookmarks.eraseEverything();
 });
 
 add_task(async function checkUndoLoginsState() {
   MigrationUtils.initializeUndoData();
-  await MigrationUtils.insertLoginsWrapper([{
+  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 = (await MigrationUtils.stopAndRetrieveUndoData()).get("logins");
   Assert.deepEqual([{guid, timePasswordChanged}], undoLoginData);
   Services.logins.removeAllLogins();
 });
 
 add_task(async function testLoginsRemovalByUndo() {
   MigrationUtils.initializeUndoData();
-  await MigrationUtils.insertLoginsWrapper([{
+  MigrationUtils.insertLoginWrapper({
     username: "foo",
     password: "bar",
     hostname: "https://example.com",
     formSubmitURL: "https://example.com/",
     timeCreated: new Date(),
-  }]);
-  await MigrationUtils.insertLoginsWrapper([{
+  });
+  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
-  await LoginHelper.maybeImportLogins([{
+  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 = (await MigrationUtils.stopAndRetrieveUndoData()).get("logins");
 
   await 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,
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -203,16 +203,61 @@ this.LoginHelper = {
         // newURI will throw for some values e.g. chrome://FirefoxAccounts
         return false;
       }
     }
 
     return false;
   },
 
+  /**
+   * Helper to check if there are any duplicates of the existing login. If the
+   * duplicates need to have the password updated, this performs that update.
+   *
+   * @param {nsILoginInfo} aLogin - The login to search for.
+   * @return {boolean} - true if duplicates exist, otherwise false.
+   */
+  checkForDuplicatesAndMaybeUpdate(aLogin) {
+    // While here we're passing formSubmitURL and httpRealm, they could be empty/null and get
+    // ignored in that case, leading to multiple logins for the same username.
+    let existingLogins = Services.logins.findLogins({}, aLogin.hostname,
+                                                    aLogin.formSubmitURL,
+                                                    aLogin.httpRealm);
+    // Check for an existing login that matches *including* the password.
+    // If such a login exists, we do not need to add a new login.
+    if (existingLogins.some(l => aLogin.matches(l, false /* ignorePassword */))) {
+      return true;
+    }
+    // Now check for a login with the same username, where it may be that we have an
+    // updated password.
+    let foundMatchingLogin = false;
+    for (let existingLogin of existingLogins) {
+      if (aLogin.username == existingLogin.username) {
+        foundMatchingLogin = true;
+        existingLogin.QueryInterface(Ci.nsILoginMetaInfo);
+        if (aLogin.password != existingLogin.password &
+           aLogin.timePasswordChanged > existingLogin.timePasswordChanged) {
+          // if a login with the same username and different password already exists and it's older
+          // than the current one, update its password and timestamp.
+          let propBag = Cc["@mozilla.org/hash-property-bag;1"].
+                        createInstance(Ci.nsIWritablePropertyBag);
+          propBag.setProperty("password", aLogin.password);
+          propBag.setProperty("timePasswordChanged", aLogin.timePasswordChanged);
+          Services.logins.modifyLogin(existingLogin, propBag);
+        }
+      }
+    }
+    // if the new login is an update or is older than an exiting login, don't add it.
+    if (foundMatchingLogin) {
+      return true;
+    }
+
+    return false;
+  },
+
   doLoginsMatch(aLogin1, aLogin2, {
     ignorePassword = false,
     ignoreSchemes = false,
   }) {
     if (aLogin1.httpRealm != aLogin2.httpRealm ||
         aLogin1.username != aLogin2.username)
       return false;
 
@@ -541,19 +586,47 @@ this.LoginHelper = {
         fieldType == "tel" ||
         fieldType == "number") {
       return true;
     }
     return false;
   },
 
   /**
-   * For each login, add the login to the password manager if a similar one
-   * doesn't already exist. Merge it otherwise with the similar existing ones.
-   *
+   * Add the login to the password manager if a similar one doesn't already exist. Merge it
+   * otherwise with the similar existing ones.
+   * @param {Object} loginData - the data about the login that needs to be added.
+   * @returns {nsILoginInfo} the newly added login, or null if no login was added.
+   *                          Note that we will also return null if an existing login
+   *                          was modified.
+   */
+  maybeImportLogin(loginData) {
+    // create a new login
+    let login = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(Ci.nsILoginInfo);
+    login.init(loginData.hostname,
+               loginData.formSubmitURL || (typeof(loginData.httpRealm) == "string" ? null : ""),
+               typeof(loginData.httpRealm) == "string" ? loginData.httpRealm : null,
+               loginData.username,
+               loginData.password,
+               loginData.usernameElement || "",
+               loginData.passwordElement || "");
+
+    login.QueryInterface(Ci.nsILoginMetaInfo);
+    login.timeCreated = loginData.timeCreated;
+    login.timeLastUsed = loginData.timeLastUsed || loginData.timeCreated;
+    login.timePasswordChanged = loginData.timePasswordChanged || loginData.timeCreated;
+    login.timesUsed = loginData.timesUsed || 1;
+    if (this.checkForDuplicatesAndMaybeUpdate(login)) {
+      return null;
+    }
+    return Services.logins.addLogin(login);
+  },
+
+  /**
+   * Equivalent to maybeImportLogin, except asynchronous and for multiple logins.
    * @param {Object[]} loginDatas - For each login, the data that needs to be added.
    * @returns {nsILoginInfo[]} the newly added logins, filtered if no login was added.
    */
   async maybeImportLogins(loginDatas) {
     let loginsToAdd = [];
     let loginMap = new Map();
     for (let loginData of loginDatas) {
       // create a new login
@@ -577,18 +650,19 @@ this.LoginHelper = {
         // out from the bulk APIs below us.
         this.checkLoginValues(login);
       } catch (e) {
         Cu.reportError(e);
         continue;
       }
 
       // First, we need to check the logins that we've already decided to add, to
-      // see if this is a duplicate. This should mirror the logic below for
-      // existingLogins, but only for the array of logins we're adding.
+      // see if this is a duplicate. This should mirror the logic inside
+      // checkForDuplicatesAndMaybeUpdate, but only for the array of logins we're
+      // adding.
       let newLogins = loginMap.get(login.hostname) || [];
       if (!newLogins) {
         loginMap.set(login.hostname, newLogins);
       } else {
         if (newLogins.some(l => login.matches(l, false /* ignorePassword */))) {
           continue;
         }
         let foundMatchingNewLogin = false;
@@ -606,56 +680,23 @@ this.LoginHelper = {
           }
         }
 
         if (foundMatchingNewLogin) {
           continue;
         }
       }
 
-      // While here we're passing formSubmitURL and httpRealm, they could be empty/null and get
-      // ignored in that case, leading to multiple logins for the same username.
-      let existingLogins = Services.logins.findLogins({}, login.hostname,
-                                                      login.formSubmitURL,
-                                                      login.httpRealm);
-      // Check for an existing login that matches *including* the password.
-      // If such a login exists, we do not need to add a new login.
-      if (existingLogins.some(l => login.matches(l, false /* ignorePassword */))) {
-        continue;
-      }
-      // Now check for a login with the same username, where it may be that we have an
-      // updated password.
-      let foundMatchingLogin = false;
-      for (let existingLogin of existingLogins) {
-        if (login.username == existingLogin.username) {
-          foundMatchingLogin = true;
-          existingLogin.QueryInterface(Ci.nsILoginMetaInfo);
-          if (login.password != existingLogin.password &
-             login.timePasswordChanged > existingLogin.timePasswordChanged) {
-            // if a login with the same username and different password already exists and it's older
-            // than the current one, update its password and timestamp.
-            let propBag = Cc["@mozilla.org/hash-property-bag;1"].
-                          createInstance(Ci.nsIWritablePropertyBag);
-            propBag.setProperty("password", login.password);
-            propBag.setProperty("timePasswordChanged", login.timePasswordChanged);
-            Services.logins.modifyLogin(existingLogin, propBag);
-          }
-        }
-      }
-      // if the new login is an update or is older than an exiting login, don't add it.
-      if (foundMatchingLogin) {
+      if (this.checkForDuplicatesAndMaybeUpdate(login)) {
         continue;
       }
 
       newLogins.push(login);
       loginsToAdd.push(login);
     }
-    if (!loginsToAdd.length) {
-      return [];
-    }
     return Services.logins.addLogins(loginsToAdd);
   },
 
   /**
    * Convert an array of nsILoginInfo to vanilla JS objects suitable for
    * sending over IPC.
    *
    * NB: All members of nsILoginInfo and nsILoginMetaInfo are strings.
--- a/toolkit/components/passwordmgr/nsLoginManager.js
+++ b/toolkit/components/passwordmgr/nsLoginManager.js
@@ -307,31 +307,27 @@ LoginManager.prototype = {
 
   async addLogins(logins) {
     let crypto = Cc["@mozilla.org/login-manager/crypto/SDR;1"].
                  getService(Ci.nsILoginManagerCrypto);
     let plaintexts = logins.map(l => l.username).concat(logins.map(l => l.password));
     let ciphertexts = await crypto.encryptMany(plaintexts);
     let usernames = ciphertexts.slice(0, logins.length);
     let passwords = ciphertexts.slice(logins.length);
-    let resultLogins = new Array(logins.length);
     for (let i = 0; i < logins.length; i++) {
       let plaintextUsername = logins[i].username;
       let plaintextPassword = logins[i].password;
       logins[i].username = usernames[i];
       logins[i].password = passwords[i];
       log.debug("Adding login");
-      resultLogins[i] = this._storage.addLogin(logins[i], true);
+      this._storage.addLogin(logins[i], true);
       // Reset the username and password to keep the same guarantees as addLogin
       logins[i].username = plaintextUsername;
       logins[i].password = plaintextPassword;
-      resultLogins[i].username = plaintextUsername;
-      resultLogins[i].password = plaintextPassword;
     }
-    return resultLogins;
   },
 
   /**
    * Remove the specified login from the stored logins.
    */
   removeLogin(login) {
     log.debug("Removing login");
     return this._storage.removeLogin(login);
--- a/toolkit/components/passwordmgr/test/unit/test_maybeImportLogin.js
+++ b/toolkit/components/passwordmgr/test/unit/test_maybeImportLogin.js
@@ -8,162 +8,162 @@ const HOST2 = "https://www.mozilla.org/"
 
 const USER1 = "myuser";
 const USER2 = "anotheruser";
 
 const PASS1 = "mypass";
 const PASS2 = "anotherpass";
 const PASS3 = "yetanotherpass";
 
-add_task(async function test_new_logins() {
-  let [importedLogin] = await LoginHelper.maybeImportLogins([{
+add_task(function test_new_logins() {
+  let importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate imported login.");
   let matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should be 1 login for ${HOST1}`);
 
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST2,
     formSubmitURL: HOST2,
-  }]);
+  });
 
   Assert.ok(importedLogin, "Return value should indicate another imported login.");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should still be 1 login for ${HOST1}`);
 
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST2});
   Assert.equal(matchingLogins.length, 1, `There should also be 1 login for ${HOST2}`);
   Assert.equal(Services.logins.getAllLogins().length, 2, "There should be 2 logins in total");
   Services.logins.removeAllLogins();
 });
 
-add_task(async function test_duplicate_logins() {
-  let [importedLogin] = await LoginHelper.maybeImportLogins([{
+add_task(function test_duplicate_logins() {
+  let importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate imported login.");
   let matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should be 1 login for ${HOST1}`);
 
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(!importedLogin, "Return value should indicate no new login was imported.");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should still be 1 login for ${HOST1}`);
   Services.logins.removeAllLogins();
 });
 
-add_task(async function test_different_passwords() {
-  let [importedLogin] = await LoginHelper.maybeImportLogins([{
+add_task(function test_different_passwords() {
+  let importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
     timeCreated: new Date(Date.now() - 1000),
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate imported login.");
   let matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should be 1 login for ${HOST1}`);
 
   // This item will be newer, so its password should take precedence.
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS2,
     hostname: HOST1,
     formSubmitURL: HOST1,
     timeCreated: new Date(),
-  }]);
+  });
   Assert.ok(!importedLogin, "Return value should not indicate imported login (as we updated an existing one).");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should still be 1 login for ${HOST1}`);
   Assert.equal(matchingLogins[0].password, PASS2, "We should have updated the password for this login.");
 
   // Now try to update with an older password:
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS3,
     hostname: HOST1,
     formSubmitURL: HOST1,
     timeCreated: new Date(Date.now() - 1000000),
-  }]);
+  });
   Assert.ok(!importedLogin, "Return value should not indicate imported login (as we didn't update anything).");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should still be 1 login for ${HOST1}`);
   Assert.equal(matchingLogins[0].password, PASS2, "We should NOT have updated the password for this login.");
 
   Services.logins.removeAllLogins();
 });
 
-add_task(async function test_different_usernames() {
-  let [importedLogin] = await LoginHelper.maybeImportLogins([{
+add_task(function test_different_usernames() {
+  let importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate imported login.");
   let matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should be 1 login for ${HOST1}`);
 
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER2,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate another imported login.");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 2, `There should now be 2 logins for ${HOST1}`);
 
   Services.logins.removeAllLogins();
 });
 
-add_task(async function test_different_targets() {
-  let [importedLogin] = await LoginHelper.maybeImportLogins([{
+add_task(function test_different_targets() {
+  let importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     formSubmitURL: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate imported login.");
   let matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should be 1 login for ${HOST1}`);
 
   // Not passing either a formSubmitURL or a httpRealm should be treated as
   // the same as the previous login
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
-  }]);
+  });
   Assert.ok(!importedLogin, "Return value should NOT indicate imported login " +
     "(because a missing formSubmitURL and httpRealm should be duped to the existing login).");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 1, `There should still be 1 login for ${HOST1}`);
   Assert.equal(matchingLogins[0].formSubmitURL, HOST1, "The form submission URL should have been kept.");
 
-  [importedLogin] = await LoginHelper.maybeImportLogins([{
+  importedLogin = LoginHelper.maybeImportLogin({
     username: USER1,
     password: PASS1,
     hostname: HOST1,
     httpRealm: HOST1,
-  }]);
+  });
   Assert.ok(importedLogin, "Return value should indicate another imported login " +
     "as an httpRealm login shouldn't be duped.");
   matchingLogins = LoginHelper.searchLoginsWithObject({hostname: HOST1});
   Assert.equal(matchingLogins.length, 2, `There should now be 2 logins for ${HOST1}`);
 
   Services.logins.removeAllLogins();
 });