Bug 1551657 part 1. Stop using [array] in getAllLogins. r=MattN
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 14 May 2019 19:28:51 +0000
changeset 532758 5a94b7075ddaa1a1958606ae59d3e683b659ee26
parent 532757 6eba5e15b3bc7e5ffdc7289ecd564dfa72b38167
child 532759 40d655af6177f18e6e30a4e0933846ca35600661
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1551657
milestone68.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 1551657 part 1. Stop using [array] in getAllLogins. r=MattN I audited all the callsites https://searchfox.org/mozilla-central/search?q=%5B%5EA-Za-z_%5D%5BGg%5DetAllLogins%5B%5EA-Za-z_%5D&case=true&regexp=true&path= brings up. Differential Revision: https://phabricator.services.mozilla.com/D31117
browser/components/migration/tests/unit/test_Chrome_passwords.js
browser/components/migration/tests/unit/test_IE7_passwords.js
services/sync/modules/engines/passwords.js
toolkit/components/passwordmgr/LoginManager.jsm
toolkit/components/passwordmgr/nsILoginManager.idl
toolkit/components/passwordmgr/nsILoginManagerStorage.idl
toolkit/components/passwordmgr/storage-json.js
toolkit/components/passwordmgr/storage-mozStorage.js
--- a/browser/components/migration/tests/unit/test_Chrome_passwords.js
+++ b/browser/components/migration/tests/unit/test_Chrome_passwords.js
@@ -153,59 +153,59 @@ add_task(async function setup() {
 add_task(async function test_importIntoEmptyDB() {
   for (let login of TEST_LOGINS) {
     await promiseSetPassword(login);
   }
 
   let migrator = await MigrationUtils.getMigrator("chrome");
   Assert.ok(await migrator.isSourceAvailable(), "Sanity check the source exists");
 
-  let logins = Services.logins.getAllLogins({});
+  let logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, 0, "There are no logins initially");
 
   // Migrate the logins.
   await promiseMigration(migrator, MigrationUtils.resourceTypes.PASSWORDS, PROFILE);
 
-  logins = Services.logins.getAllLogins({});
+  logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, TEST_LOGINS.length, "Check login count after importing the data");
   Assert.equal(logins.length, MigrationUtils._importQuantities.logins,
                "Check telemetry matches the actual import.");
 
   for (let i = 0; i < TEST_LOGINS.length; i++) {
     checkLoginsAreEqual(logins[i], TEST_LOGINS[i], i + 1);
   }
 });
 
 // Test that existing logins for the same primary key don't get overwritten
 add_task(async function test_importExistingLogins() {
   let migrator = await MigrationUtils.getMigrator("chrome");
   Assert.ok(await migrator.isSourceAvailable(), "Sanity check the source exists");
 
   Services.logins.removeAllLogins();
-  let logins = Services.logins.getAllLogins({});
+  let logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, 0, "There are no logins after removing all of them");
 
   let newLogins = [];
 
   // Create 3 new logins that are different but where the key properties are still the same.
   for (let i = 0; i < 3; i++) {
     newLogins.push(generateDifferentLogin(TEST_LOGINS[i]));
     Services.logins.addLogin(newLogins[i]);
   }
 
-  logins = Services.logins.getAllLogins({});
+  logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, newLogins.length, "Check login count after the insertion");
 
   for (let i = 0; i < newLogins.length; i++) {
     checkLoginsAreEqual(logins[i], newLogins[i], i + 1);
   }
   // Migrate the logins.
   await promiseMigration(migrator, MigrationUtils.resourceTypes.PASSWORDS, PROFILE);
 
-  logins = Services.logins.getAllLogins({});
+  logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, TEST_LOGINS.length,
                "Check there are still the same number of logins after re-importing the data");
   Assert.equal(logins.length, MigrationUtils._importQuantities.logins,
                "Check telemetry matches the actual import.");
 
   for (let i = 0; i < newLogins.length; i++) {
     checkLoginsAreEqual(logins[i], newLogins[i], i + 1);
   }
--- a/browser/components/migration/tests/unit/test_IE7_passwords.js
+++ b/browser/components/migration/tests/unit/test_IE7_passwords.js
@@ -316,59 +316,59 @@ add_task(async function setup() {
 
 add_task(async function test_passwordsNotAvailable() {
   if (AppConstants.isPlatformAndVersionAtLeast("win", "6.2")) {
     return;
   }
 
   let migrator = getFirstResourceOfType(MigrationUtils.resourceTypes.PASSWORDS);
   Assert.ok(migrator.exists, "The migrator has to exist");
-  let logins = Services.logins.getAllLogins({});
+  let logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, 0, "There are no logins at the beginning of the test");
 
   let uris = []; // the uris of the migrated logins
   for (let url of TESTED_URLS) {
     uris.push(makeURI(url));
      // in this test, there is no IE login data in the registry, so after the migration, the number
      // of logins in the store should be 0
     await migrator._migrateURIs(uris);
-    logins = Services.logins.getAllLogins({});
+    logins = Services.logins.getAllLogins();
     Assert.equal(logins.length, 0,
                  "There are no logins after doing the migration without adding values to the registry");
   }
 });
 
 add_task(async function test_passwordsAvailable() {
   if (AppConstants.isPlatformAndVersionAtLeast("win", "6.2")) {
     return;
   }
 
   let crypto = new OSCrypto();
   let hashes = []; // the hashes of all migrator websites, this is going to be used for the clean up
 
   registerCleanupFunction(() => {
     Services.logins.removeAllLogins();
-    logins = Services.logins.getAllLogins({});
+    logins = Services.logins.getAllLogins();
     Assert.equal(logins.length, 0, "There are no logins after the cleanup");
     // remove all the values created in this test from the registry
     removeAllValues(Storage2Key, hashes);
     // restore all backed up values
     restore(Storage2Key);
 
     // clean the dummy value
     if (Storage2Key.hasValue("dummy")) {
       Storage2Key.removeValue("dummy");
     }
     Storage2Key.close();
     crypto.finalize();
   });
 
   let migrator = getFirstResourceOfType(MigrationUtils.resourceTypes.PASSWORDS);
   Assert.ok(migrator.exists, "The migrator has to exist");
-  let logins = Services.logins.getAllLogins({});
+  let logins = Services.logins.getAllLogins();
   Assert.equal(logins.length, 0, "There are no logins at the beginning of the test");
 
   let uris = []; // the uris of the migrated logins
 
   let loginCount = 0;
   for (let current in TESTED_WEBSITES) {
     let website = TESTED_WEBSITES[current];
     // backup the current the registry value if it exists and replace the existing value/create a
@@ -376,17 +376,17 @@ add_task(async function test_passwordsAv
     backupAndStore(Storage2Key, website.hash,
                    crypto.encryptData(crypto.arrayToString(website.data),
                                       website.uri.spec));
     Assert.ok(migrator.exists, "The migrator has to exist");
     uris.push(website.uri);
     hashes.push(website.hash);
 
     await migrator._migrateURIs(uris);
-    logins = Services.logins.getAllLogins({});
+    logins = Services.logins.getAllLogins();
     // check that the number of logins in the password manager has increased as expected which means
     // that all the values for the current website were imported
     loginCount += website.logins.length;
     Assert.equal(logins.length, loginCount,
                  "The number of logins has increased after the migration");
     // NB: because telemetry records any login data passed to the login manager, it
     // also gets told about logins that are duplicates or invalid (for one reason
     // or another) and so its counts might exceed those of the login manager itself.
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -208,17 +208,17 @@ PasswordStore.prototype = {
     }
 
     this._log.trace("No items matching " + id + " found. Ignoring");
     return null;
   },
 
   async getAllIDs() {
     let items = {};
-    let logins = Services.logins.getAllLogins({});
+    let logins = Services.logins.getAllLogins();
 
     for (let i = 0; i < logins.length; i++) {
       // Skip over Weave password/passphrase entries.
       let metaInfo = logins[i].QueryInterface(Ci.nsILoginMetaInfo);
       if (Utils.getSyncCredentialsHosts().has(metaInfo.hostname)) {
         continue;
       }
 
@@ -397,17 +397,17 @@ class PasswordValidator extends Collecti
       "password",
       "passwordField",
       "username",
       "usernameField",
     ]);
   }
 
   getClientItems() {
-    let logins = Services.logins.getAllLogins({});
+    let logins = Services.logins.getAllLogins();
     let syncHosts = Utils.getSyncCredentialsHosts();
     let result = logins.map(l => l.QueryInterface(Ci.nsILoginMetaInfo))
                        .filter(l => !syncHosts.has(l.hostname));
     return Promise.resolve(result);
   }
 
   normalizeClientItem(item) {
     return {
--- a/toolkit/components/passwordmgr/LoginManager.jsm
+++ b/toolkit/components/passwordmgr/LoginManager.jsm
@@ -163,17 +163,17 @@ LoginManager.prototype = {
     clearAndGetHistogram("PWMGR_SAVING_ENABLED").add(LoginHelper.enabled);
     Services.obs.notifyObservers(null, "weave:telemetry:histogram", "PWMGR_SAVING_ENABLED");
 
     // Don't try to get logins if MP is enabled, since we don't want to show a MP prompt.
     if (!this.isLoggedIn) {
       return;
     }
 
-    let logins = this.getAllLogins({});
+    let logins = this.getAllLogins();
 
     let usernamePresentHistogram = clearAndGetHistogram("PWMGR_USERNAME_PRESENT");
     let loginLastUsedDaysHistogram = clearAndGetHistogram("PWMGR_LOGIN_LAST_USED_DAYS");
 
     let hostnameCount = new Map();
     for (let login of logins) {
       usernamePresentHistogram.add(!!login.username);
 
@@ -318,22 +318,21 @@ LoginManager.prototype = {
     log.debug("Modifying login");
     return this._storage.modifyLogin(oldLogin, newLogin);
   },
 
 
   /**
    * Get a dump of all stored logins. Used by the login manager UI.
    *
-   * @param count - only needed for XPCOM.
    * @return {nsILoginInfo[]} - If there are no logins, the array is empty.
    */
-  getAllLogins(count) {
+  getAllLogins() {
     log.debug("Getting a list of all logins");
-    return this._storage.getAllLogins(count);
+    return this._storage.getAllLogins();
   },
 
 
   /**
    * Remove all stored logins.
    */
   removeAllLogins() {
     log.debug("Removing all logins");
--- a/toolkit/components/passwordmgr/nsILoginManager.idl
+++ b/toolkit/components/passwordmgr/nsILoginManager.idl
@@ -92,28 +92,23 @@ interface nsILoginManager : nsISupports 
    */
   void removeAllLogins();
 
 
   /**
    * Fetch all logins in the login manager. An array is always returned;
    * if there are no logins the array is empty.
    *
-   * @param count
-   *        The number of elements in the array. JS callers can simply use
-   *        the array's .length property and omit this param.
-   * @param logins
-   *        An array of nsILoginInfo objects.
+   * @return An array of nsILoginInfo objects.
    *
    * NOTE: This can be called from JS as:
    *       var logins = pwmgr.getAllLogins();
    *       (|logins| is an array).
    */
-  void getAllLogins([optional] out unsigned long count,
-                    [retval, array, size_is(count)] out nsILoginInfo logins);
+  Array<nsILoginInfo> getAllLogins();
 
 
   /**
    * Obtain a list of all hosts for which password saving is disabled.
    *
    * @param count
    *        The number of elements in the array. JS callers can simply use
    *        the array's .length property and omit this param.
--- a/toolkit/components/passwordmgr/nsILoginManagerStorage.idl
+++ b/toolkit/components/passwordmgr/nsILoginManagerStorage.idl
@@ -104,28 +104,23 @@ interface nsILoginManagerStorage : nsISu
    */
   void removeAllLogins();
 
 
   /**
    * Fetch all logins in the login manager. An array is always returned;
    * if there are no logins the array is empty.
    *
-   * @param count
-   *        The number of elements in the array. JS callers can simply use
-   *        the array's .length property and omit this param.
-   * @param logins
-   *        An array of nsILoginInfo objects.
+   * @return An array of nsILoginInfo objects.
    *
    * NOTE: This can be called from JS as:
    *       var logins = pwmgr.getAllLogins();
    *       (|logins| is an array).
    */
-  void getAllLogins([optional] out unsigned long count,
-                    [retval, array, size_is(count)] out nsILoginInfo logins);
+  Array<nsILoginInfo> getAllLogins();
 
 
   /**
    * Search for logins in the login manager. An array is always returned;
    * if there are no logins the array is empty.
    *
    * @param count
    *        The number of elements in the array. JS callers can simply use
--- a/toolkit/components/passwordmgr/storage-json.js
+++ b/toolkit/components/passwordmgr/storage-json.js
@@ -243,26 +243,23 @@ this.LoginManagerStorage_json.prototype 
     }
 
     LoginHelper.notifyStorageChanged("modifyLogin", [oldStoredLogin, newLogin]);
   },
 
   /**
    * @return {nsILoginInfo[]}
    */
-  getAllLogins(count) {
+  getAllLogins() {
     let [logins, ids] = this._searchLogins({});
 
     // decrypt entries for caller.
     logins = this._decryptLogins(logins);
 
     this.log("_getAllLogins: returning", logins.length, "logins.");
-    if (count) {
-      count.value = logins.length;
-    } // needed for XPCOM
     return logins;
   },
 
   /**
    * Public wrapper around _searchLogins to convert the nsIPropertyBag to a
    * JavaScript object and decrypt the results.
    *
    * @return {nsILoginInfo[]} which are decrypted.
--- a/toolkit/components/passwordmgr/storage-mozStorage.js
+++ b/toolkit/components/passwordmgr/storage-mozStorage.js
@@ -391,26 +391,23 @@ LoginManagerStorage_mozStorage.prototype
 
     LoginHelper.notifyStorageChanged("modifyLogin", [oldStoredLogin, newLogin]);
   },
 
 
   /**
    * Returns an array of nsILoginInfo.
    */
-  getAllLogins(count) {
+  getAllLogins() {
     let [logins, ids] = this._searchLogins({});
 
     // decrypt entries for caller.
     logins = this._decryptLogins(logins);
 
     this.log("_getAllLogins: returning " + logins.length + " logins.");
-    if (count) {
-      count.value = logins.length;
-    } // needed for XPCOM
     return logins;
   },
 
 
   /**
    * Public wrapper around _searchLogins to convert the nsIPropertyBag to a
    * JavaScript object and decrypt the results.
    *