Commit 4a7e4f9c authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Passwords] Support moving multiple password in PasswordManagerPresenter

This is preparation for adding the functionality of moving multiple
passwords from settings view.

Mocks are in the linked bug.

Bug: 1139263
Change-Id: I883e39bbab9bd20cdbfb67b43883886dea71badb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2479462Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819333}
parent 942e5f49
...@@ -435,7 +435,8 @@ void PasswordsPrivateDelegateImpl::MovePasswordToAccount( ...@@ -435,7 +435,8 @@ void PasswordsPrivateDelegateImpl::MovePasswordToAccount(
auto* client = ChromePasswordManagerClient::FromWebContents(web_contents); auto* client = ChromePasswordManagerClient::FromWebContents(web_contents);
DCHECK(client); DCHECK(client);
if (const std::string* sort_key = password_id_generator_.TryGetKey(id)) if (const std::string* sort_key = password_id_generator_.TryGetKey(id))
password_manager_presenter_->MovePasswordToAccountStore(*sort_key, client); password_manager_presenter_->MovePasswordToAccountStore({*sort_key},
client);
} }
void PasswordsPrivateDelegateImpl::ImportPasswords( void PasswordsPrivateDelegateImpl::ImportPasswords(
......
...@@ -377,7 +377,7 @@ void PasswordManagerPresenter::UndoRemoveSavedPasswordOrException() { ...@@ -377,7 +377,7 @@ void PasswordManagerPresenter::UndoRemoveSavedPasswordOrException() {
} }
void PasswordManagerPresenter::MovePasswordToAccountStore( void PasswordManagerPresenter::MovePasswordToAccountStore(
const std::string& sort_key, const std::vector<std::string>& sort_keys,
password_manager::PasswordManagerClient* client) { password_manager::PasswordManagerClient* client) {
if (!client->GetPasswordFeatureManager()->IsOptedInForAccountStorage() || if (!client->GetPasswordFeatureManager()->IsOptedInForAccountStorage() ||
ProfileSyncServiceFactory::GetForProfile(password_view_->GetProfile()) ProfileSyncServiceFactory::GetForProfile(password_view_->GetProfile())
...@@ -385,25 +385,27 @@ void PasswordManagerPresenter::MovePasswordToAccountStore( ...@@ -385,25 +385,27 @@ void PasswordManagerPresenter::MovePasswordToAccountStore(
return; return;
} }
auto it = password_map_.find(sort_key); for (const std::string& sort_key : sort_keys) {
if (it == password_map_.end()) auto it = password_map_.find(sort_key);
return; if (it == password_map_.end())
continue;
// MovePasswordToAccountStoreHelper takes care of moving the entire
// equivalence class, so passing the first element is fine. // MovePasswordToAccountStoreHelper takes care of moving the entire
const password_manager::PasswordForm& form = *(it->second[0]); // equivalence class, so passing the first element is fine.
const password_manager::PasswordForm& form = *(it->second[0]);
// Insert nullptr first to obtain the iterator passed to the callback.
MovePasswordToAccountStoreHelperList::iterator helper_it = // Insert nullptr first to obtain the iterator passed to the callback.
move_to_account_helpers_.insert(move_to_account_helpers_.begin(), MovePasswordToAccountStoreHelperList::iterator helper_it =
nullptr); move_to_account_helpers_.insert(move_to_account_helpers_.begin(),
// The presenter outlives the helper so it's safe to use base::Unretained. nullptr);
*helper_it = std::make_unique< // The presenter outlives the helper so it's safe to use base::Unretained.
PasswordManagerPresenter::MovePasswordToAccountStoreHelper>( *helper_it = std::make_unique<
form, client, PasswordManagerPresenter::MovePasswordToAccountStoreHelper>(
base::BindOnce( form, client,
&PasswordManagerPresenter::OnMovePasswordToAccountCompleted, base::BindOnce(
base::Unretained(this), helper_it)); &PasswordManagerPresenter::OnMovePasswordToAccountCompleted,
base::Unretained(this), helper_it));
}
} }
void PasswordManagerPresenter::OnMovePasswordToAccountCompleted( void PasswordManagerPresenter::OnMovePasswordToAccountCompleted(
......
...@@ -95,12 +95,13 @@ class PasswordManagerPresenter ...@@ -95,12 +95,13 @@ class PasswordManagerPresenter
// Undoes the last saved password or exception removal. // Undoes the last saved password or exception removal.
void UndoRemoveSavedPasswordOrException(); void UndoRemoveSavedPasswordOrException();
// Moves a password stored in the profile store to the account store. Results // Moves a list of passwords stored in the profile store to the account store.
// in a no-op if any of these is true: |sort_key| is invalid, |sort_key| // For each password to move, the result is a no-op if any of these is true:
// corresponds to a password already in the account store, or the user is not // |sort_key| is invalid, |sort_key| corresponds to a password already in the
// using the account-scoped password storage. // account store, or the user is not using the account-scoped password
// storage.
void MovePasswordToAccountStore( void MovePasswordToAccountStore(
const std::string& sort_key, const std::vector<std::string>& sort_keys,
password_manager::PasswordManagerClient* client); password_manager::PasswordManagerClient* client);
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
......
...@@ -503,20 +503,24 @@ TEST_F(PasswordManagerPresenterTestWithAccountStore, ...@@ -503,20 +503,24 @@ TEST_F(PasswordManagerPresenterTestWithAccountStore,
TestMovePasswordToAccountStore) { TestMovePasswordToAccountStore) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
// Fill the profile store with two entries in the same equivalence class. // Fill the profile store with 3 entries of which 2 are in the same
// equivalence class.
password_manager::PasswordForm password = password_manager::PasswordForm password =
AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword); AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
AddPasswordEntry(GURL(kExampleCom).Resolve("someOtherPath"), kUsername, AddPasswordEntry(GURL(kExampleCom).Resolve("someOtherPath"), kUsername,
kPassword); kPassword);
password_manager::PasswordForm password2 =
AddPasswordEntry(GURL(kExampleCom), kUsername2, kPassword);
// Since there are 2 stores, SetPasswordList() and SetPasswordExceptionList() // Since there are 2 stores, SetPasswordList() and SetPasswordExceptionList()
// are called twice. // are called twice.
EXPECT_CALL(GetUIController(), SetPasswordList).Times(2); EXPECT_CALL(GetUIController(), SetPasswordList).Times(2);
EXPECT_CALL(GetUIController(), SetPasswordExceptionList).Times(2); EXPECT_CALL(GetUIController(), SetPasswordExceptionList).Times(2);
UpdatePasswordLists(); UpdatePasswordLists();
ASSERT_THAT( ASSERT_THAT(GetUsernamesAndPasswords(
GetUsernamesAndPasswords( GetPasswordsInStoreForRealm(*profile_store(), kExampleCom)),
GetPasswordsInStoreForRealm(*profile_store(), kExampleCom)), UnorderedElementsAre(Pair(kUsername, kPassword),
ElementsAre(Pair(kUsername, kPassword), Pair(kUsername, kPassword))); Pair(kUsername, kPassword),
Pair(kUsername2, kPassword)));
ASSERT_THAT(GetUsernamesAndPasswords( ASSERT_THAT(GetUsernamesAndPasswords(
GetPasswordsInStoreForRealm(*account_store(), kExampleCom)), GetPasswordsInStoreForRealm(*account_store(), kExampleCom)),
IsEmpty()); IsEmpty());
...@@ -524,26 +528,29 @@ TEST_F(PasswordManagerPresenterTestWithAccountStore, ...@@ -524,26 +528,29 @@ TEST_F(PasswordManagerPresenterTestWithAccountStore,
// Move |password| to account and wait for stores to be updated. // Move |password| to account and wait for stores to be updated.
GetUIController().GetPasswordManagerPresenter()->MovePasswordToAccountStore( GetUIController().GetPasswordManagerPresenter()->MovePasswordToAccountStore(
password_manager::CreateSortKey(password), client()); {password_manager::CreateSortKey(password),
password_manager::CreateSortKey(password2)},
client());
PasswordStoreWaiter profile_store_waiter(profile_store()); PasswordStoreWaiter profile_store_waiter(profile_store());
PasswordStoreWaiter account_store_waiter(account_store()); PasswordStoreWaiter account_store_waiter(account_store());
// Both passwords should have moved. // All passwords should have moved.
EXPECT_CALL(GetUIController(), SetPasswordList).Times(2); EXPECT_CALL(GetUIController(), SetPasswordList).Times(2);
EXPECT_CALL(GetUIController(), SetPasswordExceptionList).Times(2); EXPECT_CALL(GetUIController(), SetPasswordExceptionList).Times(2);
UpdatePasswordLists(); UpdatePasswordLists();
EXPECT_THAT(GetPasswordsInStoreForRealm(*profile_store(), kExampleCom), EXPECT_THAT(GetPasswordsInStoreForRealm(*profile_store(), kExampleCom),
IsEmpty()); IsEmpty());
EXPECT_THAT( EXPECT_THAT(GetUsernamesAndPasswords(
GetUsernamesAndPasswords( GetPasswordsInStoreForRealm(*account_store(), kExampleCom)),
GetPasswordsInStoreForRealm(*account_store(), kExampleCom)), UnorderedElementsAre(Pair(kUsername, kPassword),
ElementsAre(Pair(kUsername, kPassword), Pair(kUsername, kPassword))); Pair(kUsername, kPassword),
Pair(kUsername2, kPassword)));
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
"PasswordManager.AccountStorage.MoveToAccountStoreFlowAccepted", "PasswordManager.AccountStorage.MoveToAccountStoreFlowAccepted",
password_manager::metrics_util::MoveToAccountStoreTrigger:: password_manager::metrics_util::MoveToAccountStoreTrigger::
kExplicitlyTriggeredInSettings, kExplicitlyTriggeredInSettings,
1); 2);
} }
} // namespace } // namespace
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment