Skip to content

Commit

Permalink
Bug 1807618, replace all usage of getAllLogins with the asynchronous …
Browse files Browse the repository at this point in the history
…version, removing the syncronous version, r=credential-management-reviewers,sync-reviewers,sgalich,skhamis

Differential Revision: https://phabricator.services.mozilla.com/D178406
  • Loading branch information
EnnDeakin2 committed Jul 16, 2023
1 parent d56d7e3 commit 2483a0c
Show file tree
Hide file tree
Showing 47 changed files with 275 additions and 331 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,10 @@ def checkPassword(self):
self.assertEqual(loginInfo[0]["username"], self._username)
self.assertEqual(loginInfo[0]["password"], self._password)

loginCount = self.marionette.execute_script(
loginCount = self.runAsyncCode(
"""
return Services.logins.getAllLogins().length;
let resolve = arguments[arguments.length - 1];
Services.logins.getAllLogins().then(logins => resolve(logins.length));
"""
)
# Note that we expect 2 logins - one from us, one from sync.
Expand Down
10 changes: 5 additions & 5 deletions browser/components/migration/tests/unit/test_Chrome_passwords.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ add_task(async function test_importIntoEmptyDB() {
"Sanity check the source exists"
);

let logins = Services.logins.getAllLogins();
let logins = await Services.logins.getAllLogins();
Assert.equal(logins.length, 0, "There are no logins initially");

// Migrate the logins.
Expand All @@ -296,7 +296,7 @@ add_task(async function test_importIntoEmptyDB() {
true
);

logins = Services.logins.getAllLogins();
logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
TEST_LOGINS.length,
Expand All @@ -322,7 +322,7 @@ add_task(async function test_importExistingLogins() {
);

Services.logins.removeAllUserFacingLogins();
let logins = Services.logins.getAllLogins();
let logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
0,
Expand All @@ -337,7 +337,7 @@ add_task(async function test_importExistingLogins() {
await Services.logins.addLoginAsync(newLogins[i]);
}

logins = Services.logins.getAllLogins();
logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
newLogins.length,
Expand All @@ -355,7 +355,7 @@ add_task(async function test_importExistingLogins() {
true
);

logins = Services.logins.getAllLogins();
logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
TEST_LOGINS.length,
Expand Down
12 changes: 6 additions & 6 deletions browser/components/migration/tests/unit/test_IE7_passwords.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ add_task(async function test_passwordsNotAvailable() {
MigrationUtils.resourceTypes.PASSWORDS
);
Assert.ok(migrator.exists, "The migrator has to exist");
let logins = Services.logins.getAllLogins();
let logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
0,
Expand All @@ -397,7 +397,7 @@ add_task(async function test_passwordsNotAvailable() {
// 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 = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
0,
Expand All @@ -414,9 +414,9 @@ add_task(async function test_passwordsAvailable() {
let crypto = new OSCrypto();
let hashes = []; // the hashes of all migrator websites, this is going to be used for the clean up

registerCleanupFunction(() => {
registerCleanupFunction(async () => {
Services.logins.removeAllUserFacingLogins();
logins = Services.logins.getAllLogins();
logins = await 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);
Expand All @@ -435,7 +435,7 @@ add_task(async function test_passwordsAvailable() {
MigrationUtils.resourceTypes.PASSWORDS
);
Assert.ok(migrator.exists, "The migrator has to exist");
let logins = Services.logins.getAllLogins();
let logins = await Services.logins.getAllLogins();
Assert.equal(
logins.length,
0,
Expand All @@ -459,7 +459,7 @@ add_task(async function test_passwordsAvailable() {
hashes.push(website.hash);

await migrator._migrateURIs(uris);
logins = Services.logins.getAllLogins();
logins = await 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;
Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/LoginDetectionService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void LoginDetectionService::FetchLogins() {
return;
}

Unused << loginManager->GetAllLoginsWithCallbackAsync(this);
Unused << loginManager->GetAllLoginsWithCallback(this);
}

void LoginDetectionService::UnregisterObserver() {
Expand Down
8 changes: 4 additions & 4 deletions services/sync/modules/engines/passwords.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ PasswordEngine.prototype = {
async _getChangedIDs(getAll) {
let changes = {};

let logins = await this._store.storage.getAllLoginsAsync(true);
let logins = await this._store.storage.getAllLogins(true);
for (let login of logins) {
if (getAll || login.syncCounter > 0) {
if (Utils.getSyncCredentialsHosts().has(login.origin)) {
Expand Down Expand Up @@ -306,7 +306,7 @@ PasswordStore.prototype = {

async getAllIDs() {
let items = {};
let logins = await this.storage.getAllLoginsAsync(true);
let logins = await this.storage.getAllLogins(true);

for (let i = 0; i < logins.length; i++) {
// Skip over Weave password/passphrase entries.
Expand Down Expand Up @@ -496,8 +496,8 @@ export class PasswordValidator extends CollectionValidator {
]);
}

getClientItems() {
let logins = Services.logins.getAllLogins();
async getClientItems() {
let logins = await Services.logins.getAllLogins();
let syncHosts = Utils.getSyncCredentialsHosts();
let result = logins
.map(l => l.QueryInterface(Ci.nsILoginMetaInfo))
Expand Down
6 changes: 2 additions & 4 deletions services/sync/tests/unit/test_password_engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,7 @@ add_task(async function test_sync_outgoing() {
equal(collection.count(), 1);
equal(Services.logins.countLogins("", "", ""), 2);
equal(Services.logins.findLogins("", "", "").length, 2);
equal(Services.logins.getAllLogins().length, 2);
equal((await Services.logins.getAllLoginsAsync()).length, 2);
equal((await Services.logins.getAllLogins()).length, 2);
ok(await engine._store.itemExists(guid));

ok((await engine._store.getAllIDs())[guid]);
Expand Down Expand Up @@ -403,8 +402,7 @@ add_task(async function test_sync_outgoing() {
// All of these should not include the deleted login. Only the FxA password should exist.
equal(Services.logins.countLogins("", "", ""), 1);
equal(Services.logins.findLogins("", "", "").length, 1);
equal(Services.logins.getAllLogins().length, 1);
equal((await Services.logins.getAllLoginsAsync()).length, 1);
equal((await Services.logins.getAllLogins()).length, 1);
ok(!(await engine._store.itemExists(guid)));

// getAllIDs includes deleted items but skips the FxA login.
Expand Down
2 changes: 1 addition & 1 deletion services/sync/tests/unit/test_password_tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ add_task(async function test_removeAllLogins() {
Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2);

if (syncBeforeRemove) {
let logins = await Services.logins.getAllLoginsAsync(true);
let logins = await Services.logins.getAllLogins();
for (let login of logins) {
engine.markSynced(login.guid);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ var nsLoginInfo = new Components.Constructor(
"init"
);

export var DumpPasswords = function TPS__Passwords__DumpPasswords() {
let logins = Services.logins.getAllLogins();
export var DumpPasswords = async function TPS__Passwords__DumpPasswords() {
let logins = await Services.logins.getAllLogins();
Logger.logInfo("\ndumping password list\n", true);
for (var i = 0; i < logins.length; i++) {
Logger.logInfo(
Expand Down
2 changes: 1 addition & 1 deletion services/sync/tps/extensions/tps/resource/tps.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ export var TPS = {
"executing action " + action.toUpperCase() + " on passwords"
);
} catch (e) {
lazy.DumpPasswords();
await lazy.DumpPasswords();
throw e;
}
},
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/cleardata/ClearDataService.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ const PasswordsCleaner = {

async _deleteInternal(aCb) {
try {
let logins = Services.logins.getAllLogins();
let logins = await Services.logins.getAllLogins();
for (let login of logins) {
if (aCb(login)) {
Services.logins.removeLogin(login);
Expand Down
12 changes: 6 additions & 6 deletions toolkit/components/cleardata/tests/unit/test_passwords.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ add_task(async function test_principal_downloads() {
});
await Services.logins.addLoginAsync(loginInfo);

Assert.equal(countLogins(URL), 1);
Assert.equal(await countLogins(URL), 1);

let uri = Services.io.newURI(URL);
let principal = Services.scriptSecurityManager.createContentPrincipal(
Expand All @@ -42,7 +42,7 @@ add_task(async function test_principal_downloads() {
);
});

Assert.equal(countLogins(URL), 0);
Assert.equal(await countLogins(URL), 0);

LoginTestUtils.clearData();
});
Expand All @@ -59,7 +59,7 @@ add_task(async function test_all() {
});
await Services.logins.addLoginAsync(loginInfo);

Assert.equal(countLogins(URL), 1);
Assert.equal(await countLogins(URL), 1);

await new Promise(resolve => {
Services.clearData.deleteData(
Expand All @@ -71,14 +71,14 @@ add_task(async function test_all() {
);
});

Assert.equal(countLogins(URL), 0);
Assert.equal(await countLogins(URL), 0);

LoginTestUtils.clearData();
});

function countLogins(origin) {
async function countLogins(origin) {
let count = 0;
const logins = Services.logins.getAllLogins();
const logins = await Services.logins.getAllLogins();
for (const login of logins) {
if (login.origin == origin) {
++count;
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/passwordmgr/LoginExport.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class LoginExport {
*/
static async exportAsCSV(path, logins = null) {
if (!logins) {
logins = await Services.logins.getAllLoginsAsync();
logins = await Services.logins.getAllLogins();
}
let columns = [
"origin",
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/passwordmgr/LoginHelper.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1734,7 +1734,7 @@ export const LoginHelper = {

async getAllUserFacingLogins() {
try {
let logins = await Services.logins.getAllLoginsAsync();
let logins = await Services.logins.getAllLogins();
return logins.filter(this.isUserFacingLogin);
} catch (e) {
if (e.result == Cr.NS_ERROR_ABORT) {
Expand Down
20 changes: 5 additions & 15 deletions toolkit/components/passwordmgr/LoginManager.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ LoginManager.prototype = {
return;
}

let logins = await this.getAllLoginsAsync();
let logins = await this.getAllLogins();

let usernamePresentHistogram = clearAndGetHistogram(
"PWMGR_USERNAME_PRESENT"
Expand Down Expand Up @@ -387,32 +387,22 @@ LoginManager.prototype = {
);
},

/**
* Get a dump of all stored logins. Used by the login manager UI.
*
* @return {nsILoginInfo[]} - If there are no logins, the array is empty.
*/
getAllLogins() {
lazy.log.debug("Getting a list of all logins.");
return this._storage.getAllLogins();
},

/**
* Get a dump of all stored logins asynchronously. Used by the login manager UI.
*
* @return {nsILoginInfo[]} - If there are no logins, the array is empty.
*/
async getAllLoginsAsync() {
async getAllLogins() {
lazy.log.debug("Getting a list of all logins asynchronously.");
return this._storage.getAllLoginsAsync();
return this._storage.getAllLogins();
},

/**
* Get a dump of all stored logins asynchronously. Used by the login detection service.
*/
getAllLoginsWithCallbackAsync(aCallback) {
getAllLoginsWithCallback(aCallback) {
lazy.log.debug("Searching a list of all logins asynchronously.");
this._storage.getAllLoginsAsync().then(logins => {
this._storage.getAllLogins().then(logins => {
aCallback.onSearchComplete(logins);
});
},
Expand Down
19 changes: 5 additions & 14 deletions toolkit/components/passwordmgr/nsILoginManager.idl
Original file line number Diff line number Diff line change
Expand Up @@ -147,30 +147,21 @@ interface nsILoginManager : nsISupports {

/**
* Fetch all logins in the login manager. An array is always returned;
* if there are no logins the array is empty.
*
* @deprecated Use `getAllLoginsAsync` instead.
*
* @return An array of nsILoginInfo objects.
*/
Array<nsILoginInfo> getAllLogins();

/**
* Like getAllLogins, but asynchronous. This method is faster when large
* amounts of logins are present since it will handle decryption in one batch.
* if there are no logins the array is empty. Decryption is handled in
* one batch.
*
* @return A promise which resolves with a JS Array of nsILoginInfo objects.
*/
Promise getAllLoginsAsync();
Promise getAllLogins();

/**
* Like getAllLoginsAsync, but with a callback returning the search results.
* Like getAllLogins, but with a callback returning the search results.
*
* @param {nsILoginSearchCallback} aCallback
* The interface to notify when the search is complete.
*
*/
void getAllLoginsWithCallbackAsync(in nsILoginSearchCallback aCallback);
void getAllLoginsWithCallback(in nsILoginSearchCallback aCallback);

/**
* Obtain a list of all origins for which password saving is disabled.
Expand Down
6 changes: 1 addition & 5 deletions toolkit/components/passwordmgr/storage-geckoview.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,12 @@ export class LoginManagerStorage extends LoginManagerStorage_json {
);
}

getAllLogins() {
throw Components.Exception("", Cr.NS_ERROR_NOT_IMPLEMENTED);
}

/**
* Returns a promise resolving to an array of all saved logins that can be decrypted.
*
* @resolve {nsILoginInfo[]}
*/
getAllLoginsAsync(includeDeleted) {
getAllLogins(includeDeleted) {
return this._getLoginsAsync({}, includeDeleted);
}

Expand Down
17 changes: 1 addition & 16 deletions toolkit/components/passwordmgr/storage-json.sys.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -460,29 +460,14 @@ export class LoginManagerStorage_json {
return this._store._data.dismissedBreachAlertsByLoginGUID;
}

/**
* @return {nsILoginInfo[]}
*/
getAllLogins() {
this._store.ensureDataReady();

let [logins] = this._searchLogins({});

// decrypt entries for caller.
logins = this._decryptLogins(logins);

this.log(`Returning ${logins.length} logins.`);
return logins;
}

/**
* Returns an array of nsILoginInfo. If decryption of a login
* fails due to a corrupt entry, the login is not included in
* the resulting array.
*
* @resolve {nsILoginInfo[]}
*/
async getAllLoginsAsync(includeDeleted) {
async getAllLogins(includeDeleted) {
this._store.ensureDataReady();

let [logins] = this._searchLogins({}, includeDeleted);
Expand Down
Loading

0 comments on commit 2483a0c

Please sign in to comment.