Commit 3f768a8a authored by Victor Hugo Vianna Silva's avatar Victor Hugo Vianna Silva Committed by Commit Bot

Refactor: Use matches directly in PasswordGenerationManager API

No behavior changed, this is a preparation to introduce B4P's conflict
resolution logic for password generation. We modify the API to take
matches instead of the form fetcher so that the multi-store save manager
can later pass only local matches to it in an overriden method.

Change-Id: Ib0eecb11b63fa23908e74a52116335433270468f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124455Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#754501}
parent 8da5beb0
......@@ -9,7 +9,6 @@
#include "base/callback.h"
#include "base/time/default_clock.h"
#include "components/password_manager/core/browser/form_fetcher.h"
#include "components/password_manager/core/browser/form_saver.h"
#include "components/password_manager/core/browser/password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_manager_client.h"
......@@ -200,19 +199,20 @@ std::unique_ptr<PasswordGenerationManager> PasswordGenerationManager::Clone()
void PasswordGenerationManager::GeneratedPasswordAccepted(
PasswordForm generated,
const FormFetcher& fetcher,
const std::vector<const PasswordForm*>& non_federated_matches,
const std::vector<const PasswordForm*>& federated_matches,
base::WeakPtr<PasswordManagerDriver> driver) {
// Clear the username value if there are already saved credentials with
// the same username in order to prevent overwriting.
std::vector<const PasswordForm*> matches = fetcher.GetNonFederatedMatches();
if (FindUsernameConflict(generated, matches)) {
if (FindUsernameConflict(generated, non_federated_matches)) {
generated.username_value.clear();
const PasswordForm* conflict = FindUsernameConflict(generated, matches);
const PasswordForm* conflict =
FindUsernameConflict(generated, non_federated_matches);
if (conflict) {
LogGenerationPresaveConflict(
GenerationPresaveConflict::kConflictWithEmptyUsername);
auto bubble_launcher = std::make_unique<PasswordDataForUI>(
std::move(generated), matches, fetcher.GetFederatedMatches(),
std::move(generated), non_federated_matches, federated_matches,
base::BindRepeating(&PasswordGenerationManager::OnPresaveBubbleResult,
weak_factory_.GetWeakPtr(), std::move(driver)));
client_->PromptUserToSaveOrUpdatePassword(std::move(bubble_launcher),
......
......@@ -15,7 +15,6 @@
namespace password_manager {
class FormFetcher;
class FormSaver;
class PasswordManagerClient;
class PasswordManagerDriver;
......@@ -37,19 +36,22 @@ class PasswordGenerationManager {
return presaved_->password_value;
}
// Called when user wants to start generation flow for |generated|. If there
// is no username conflict, the message is synchronously passed to |driver|.
// |fetcher| to fill that UI with correct data.
// Otherwise, the UI on the client is invoked to ask for overwrite permission.
// There is one corner case that is still not covered.
// The user had the current password saved with empty username.
// Called when user wants to start generation flow for |generated|.
// |non_federated_matches| and |federated_matches| are used to determine
// whether there is a username conflict. If there is none, the message is
// synchronously passed to |driver|. Otherwise, the UI on the client is
// invoked to ask for overwrite permission. There is one corner case that is
// still not covered. The user had the current password saved with empty
// username.
// - The change password form has no username.
// - The user generates a password and sees the bubble with an empty username.
// - The user clicks 'Update'.
// - The actual form submission doesn't succeed for some reason.
void GeneratedPasswordAccepted(autofill::PasswordForm generated,
const FormFetcher& fetcher,
base::WeakPtr<PasswordManagerDriver> driver);
void GeneratedPasswordAccepted(
autofill::PasswordForm generated,
const std::vector<const autofill::PasswordForm*>& non_federated_matches,
const std::vector<const autofill::PasswordForm*>& federated_matches,
base::WeakPtr<PasswordManagerDriver> driver);
// Called when generated password is accepted or changed by user.
void PresaveGeneratedPassword(
......
......@@ -167,8 +167,9 @@ PasswordGenerationManagerTest::SetUpOverwritingUI(
EXPECT_CALL(client_, PromptUserToSaveOrUpdatePasswordMock(true))
.WillOnce(testing::Return(true));
manager().GeneratedPasswordAccepted(std::move(generated), fetcher,
std::move(driver));
manager().GeneratedPasswordAccepted(
std::move(generated), fetcher.GetNonFederatedMatches(),
fetcher.GetFederatedMatches(), std::move(driver));
return client_.MoveForm();
}
......@@ -181,8 +182,9 @@ TEST_F(PasswordGenerationManagerTest, GeneratedPasswordAccepted_EmptyStore) {
FakeFormFetcher fetcher;
EXPECT_CALL(driver, GeneratedPasswordAccepted(generated.password_value));
manager().GeneratedPasswordAccepted(std::move(generated), fetcher,
driver.AsWeakPtr());
manager().GeneratedPasswordAccepted(
std::move(generated), fetcher.GetNonFederatedMatches(),
fetcher.GetFederatedMatches(), driver.AsWeakPtr());
EXPECT_FALSE(manager().HasGeneratedPassword());
histogram_tester.ExpectUniqueSample(
"PasswordGeneration.PresaveConflict",
......@@ -202,8 +204,9 @@ TEST_F(PasswordGenerationManagerTest, GeneratedPasswordAccepted_Conflict) {
fetcher.SetNonFederated({&saved});
EXPECT_CALL(driver, GeneratedPasswordAccepted(generated.password_value));
manager().GeneratedPasswordAccepted(std::move(generated), fetcher,
driver.AsWeakPtr());
manager().GeneratedPasswordAccepted(
std::move(generated), fetcher.GetNonFederatedMatches(),
fetcher.GetFederatedMatches(), driver.AsWeakPtr());
EXPECT_FALSE(manager().HasGeneratedPassword());
histogram_tester.ExpectUniqueSample(
"PasswordGeneration.PresaveConflict",
......
......@@ -301,8 +301,9 @@ void PasswordSaveManagerImpl::GeneratedPasswordAccepted(
PasswordForm parsed_form,
base::WeakPtr<PasswordManagerDriver> driver) {
generation_manager_ = std::make_unique<PasswordGenerationManager>(client_);
generation_manager_->GeneratedPasswordAccepted(std::move(parsed_form),
*form_fetcher_, driver);
generation_manager_->GeneratedPasswordAccepted(
std::move(parsed_form), form_fetcher_->GetNonFederatedMatches(),
form_fetcher_->GetFederatedMatches(), driver);
}
void PasswordSaveManagerImpl::PasswordNoLongerGenerated() {
......
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