Commit 859834d8 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot

PasswordManager: Remove remaining uses of "best_matches" map

The "best_matches" are now passed around in a vector instead of a map.
This CL updates the last two places that accepted a map:
- VotesUploader
- PasswordFormMetricsRecorder

Bug: 1011399
Change-Id: I42fa29549cfc02398f69b802a3d18cd1eb75e89a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1862448
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705949}
parent 60e70706
...@@ -145,14 +145,6 @@ bool FormContainsFieldWithName(const FormData& form, ...@@ -145,14 +145,6 @@ bool FormContainsFieldWithName(const FormData& form,
return false; return false;
} }
std::map<base::string16, const PasswordForm*> ByUsername(
const std::vector<const PasswordForm*> forms) {
std::map<base::string16, const PasswordForm*> by_username;
for (const auto* form : forms)
by_username[form->username_value] = form;
return by_username;
}
} // namespace } // namespace
PasswordFormManager::PasswordFormManager( PasswordFormManager::PasswordFormManager(
...@@ -324,8 +316,7 @@ void PasswordFormManager::Save() { ...@@ -324,8 +316,7 @@ void PasswordFormManager::Save() {
SanitizePossibleUsernames(&pending_credentials_); SanitizePossibleUsernames(&pending_credentials_);
pending_credentials_.date_created = base::Time::Now(); pending_credentials_.date_created = base::Time::Now();
votes_uploader_.SendVotesOnSave(observed_form_, *parsed_submitted_form_, votes_uploader_.SendVotesOnSave(observed_form_, *parsed_submitted_form_,
ByUsername(GetBestMatches()), GetBestMatches(), &pending_credentials_);
&pending_credentials_);
SavePendingToStore(false /*update*/); SavePendingToStore(false /*update*/);
} else { } else {
ProcessUpdate(); ProcessUpdate();
...@@ -904,7 +895,7 @@ void PasswordFormManager::CreatePendingCredentials() { ...@@ -904,7 +895,7 @@ void PasswordFormManager::CreatePendingCredentials() {
// Calculate the user's action based on existing matches and the submitted // Calculate the user's action based on existing matches and the submitted
// form. // form.
metrics_recorder_->CalculateUserAction(ByUsername(GetBestMatches()), metrics_recorder_->CalculateUserAction(GetBestMatches(),
*parsed_submitted_form_); *parsed_submitted_form_);
// This function might be called multiple times so set variables that are // This function might be called multiple times so set variables that are
...@@ -1055,9 +1046,8 @@ void PasswordFormManager::ProcessUpdate() { ...@@ -1055,9 +1046,8 @@ void PasswordFormManager::ProcessUpdate() {
} }
if (pending_credentials_.times_used == 1) { if (pending_credentials_.times_used == 1) {
votes_uploader_.UploadFirstLoginVotes(ByUsername(GetBestMatches()), votes_uploader_.UploadFirstLoginVotes(
pending_credentials_, GetBestMatches(), pending_credentials_, *parsed_submitted_form_);
*parsed_submitted_form_);
} }
} }
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "components/password_manager/core/browser/form_fetcher.h" #include "components/password_manager/core/browser/form_fetcher.h"
#include "components/password_manager/core/browser/password_bubble_experiment.h" #include "components/password_manager/core/browser/password_bubble_experiment.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/browser/password_manager_util.h"
#include "components/password_manager/core/browser/statistics_table.h" #include "components/password_manager/core/browser/statistics_table.h"
using autofill::FieldPropertiesFlags; using autofill::FieldPropertiesFlags;
...@@ -295,8 +296,8 @@ void PasswordFormMetricsRecorder::SetManagerAction( ...@@ -295,8 +296,8 @@ void PasswordFormMetricsRecorder::SetManagerAction(
} }
void PasswordFormMetricsRecorder::CalculateUserAction( void PasswordFormMetricsRecorder::CalculateUserAction(
const std::map<base::string16, const autofill::PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
const autofill::PasswordForm& submitted_form) { const PasswordForm& submitted_form) {
const base::string16& submitted_password = const base::string16& submitted_password =
!submitted_form.new_password_value.empty() !submitted_form.new_password_value.empty()
? submitted_form.new_password_value ? submitted_form.new_password_value
...@@ -306,8 +307,8 @@ void PasswordFormMetricsRecorder::CalculateUserAction( ...@@ -306,8 +307,8 @@ void PasswordFormMetricsRecorder::CalculateUserAction(
// In case the submitted form does not have a username field we do not // In case the submitted form does not have a username field we do not
// autofill. Thus the user either explicitly chose this credential from the // autofill. Thus the user either explicitly chose this credential from the
// dropdown, or created a new password. // dropdown, or created a new password.
for (const auto& match : best_matches) { for (const PasswordForm* match : best_matches) {
if (match.second->password_value == submitted_password) { if (match->password_value == submitted_password) {
user_action_ = UserAction::kChoose; user_action_ = UserAction::kChoose;
return; return;
} }
...@@ -320,14 +321,15 @@ void PasswordFormMetricsRecorder::CalculateUserAction( ...@@ -320,14 +321,15 @@ void PasswordFormMetricsRecorder::CalculateUserAction(
// In case the submitted form has a username value, check if there is an // In case the submitted form has a username value, check if there is an
// existing match with the same username. If not, the user created a new // existing match with the same username. If not, the user created a new
// credential. // credential.
auto found = best_matches.find(submitted_form.username_value); const PasswordForm* existing_match =
if (found == best_matches.end()) { password_manager_util::FindFormByUsername(best_matches,
submitted_form.username_value);
if (!existing_match) {
user_action_ = UserAction::kOverrideUsernameAndPassword; user_action_ = UserAction::kOverrideUsernameAndPassword;
return; return;
} }
// Otherwise check if the user changed the password. // Otherwise check if the user changed the password.
const autofill::PasswordForm* existing_match = found->second;
if (existing_match->password_value != submitted_password) { if (existing_match->password_value != submitted_password) {
user_action_ = UserAction::kOverridePassword; user_action_ = UserAction::kOverridePassword;
return; return;
......
...@@ -312,8 +312,7 @@ class PasswordFormMetricsRecorder ...@@ -312,8 +312,7 @@ class PasswordFormMetricsRecorder
// matches. Also inspects |manager_action_| to correctly detect if the // matches. Also inspects |manager_action_| to correctly detect if the
// user chose a credential. // user chose a credential.
void CalculateUserAction( void CalculateUserAction(
const std::map<base::string16, const autofill::PasswordForm*>& const std::vector<const autofill::PasswordForm*>& best_matches,
best_matches,
const autofill::PasswordForm& submitted_form); const autofill::PasswordForm& submitted_form);
// Allow tests to explicitly set a value for |user_action_|. // Allow tests to explicitly set a value for |user_action_|.
......
...@@ -60,18 +60,6 @@ bool IsBetterMatchUsingLastUsed(const PasswordForm* lhs, ...@@ -60,18 +60,6 @@ bool IsBetterMatchUsingLastUsed(const PasswordForm* lhs,
std::make_pair(!rhs->is_public_suffix_match, rhs->date_last_used); std::make_pair(!rhs->is_public_suffix_match, rhs->date_last_used);
} }
// Returns a form with the given |username_value| from |credentials|, or nullptr
// if none exists.
const PasswordForm* FindByUsername(
const std::vector<const PasswordForm*>& credentials,
const base::string16& username_value) {
for (const auto* form : credentials) {
if (form->username_value == username_value)
return form;
}
return nullptr;
}
} // namespace } // namespace
// Update |credential| to reflect usage. // Update |credential| to reflect usage.
...@@ -271,6 +259,16 @@ void FindBestMatches( ...@@ -271,6 +259,16 @@ void FindBestMatches(
*preferred_match = *non_federated_same_scheme->begin(); *preferred_match = *non_federated_same_scheme->begin();
} }
const PasswordForm* FindFormByUsername(
const std::vector<const PasswordForm*>& forms,
const base::string16& username_value) {
for (const PasswordForm* form : forms) {
if (form->username_value == username_value)
return form;
}
return nullptr;
}
const PasswordForm* GetMatchForUpdating( const PasswordForm* GetMatchForUpdating(
const PasswordForm& submitted_form, const PasswordForm& submitted_form,
const std::vector<const PasswordForm*>& credentials) { const std::vector<const PasswordForm*>& credentials) {
...@@ -282,7 +280,7 @@ const PasswordForm* GetMatchForUpdating( ...@@ -282,7 +280,7 @@ const PasswordForm* GetMatchForUpdating(
// Try to return form with matching |username_value|. // Try to return form with matching |username_value|.
const PasswordForm* username_match = const PasswordForm* username_match =
FindByUsername(credentials, submitted_form.username_value); FindFormByUsername(credentials, submitted_form.username_value);
if (username_match) { if (username_match) {
if (!username_match->is_public_suffix_match) if (!username_match->is_public_suffix_match)
return username_match; return username_match;
......
...@@ -115,6 +115,12 @@ void FindBestMatches( ...@@ -115,6 +115,12 @@ void FindBestMatches(
std::vector<const autofill::PasswordForm*>* best_matches, std::vector<const autofill::PasswordForm*>* best_matches,
const autofill::PasswordForm** preferred_match); const autofill::PasswordForm** preferred_match);
// Returns a form with the given |username_value| from |forms|, or nullptr if
// none exists. If multiple matches exist, returns the first one.
const autofill::PasswordForm* FindFormByUsername(
const std::vector<const autofill::PasswordForm*>& forms,
const base::string16& username_value);
// If the user submits a form, they may have used existing credentials, new // If the user submits a form, they may have used existing credentials, new
// credentials, or modified existing credentials that should be updated. // credentials, or modified existing credentials that should be updated.
// The function returns a form from |credentials| that is the best candidate to // The function returns a form from |credentials| that is the best candidate to
......
...@@ -34,6 +34,7 @@ using autofill::RandomizedEncoder; ...@@ -34,6 +34,7 @@ using autofill::RandomizedEncoder;
using autofill::ServerFieldType; using autofill::ServerFieldType;
using autofill::ServerFieldTypeSet; using autofill::ServerFieldTypeSet;
using autofill::ValueElementPair; using autofill::ValueElementPair;
using password_manager_util::FindFormByUsername;
using Logger = autofill::SavePasswordProgressLogger; using Logger = autofill::SavePasswordProgressLogger;
using StringID = autofill::SavePasswordProgressLogger::StringID; using StringID = autofill::SavePasswordProgressLogger::StringID;
...@@ -119,11 +120,12 @@ void LabelFields(const FieldTypeMap& field_types, ...@@ -119,11 +120,12 @@ void LabelFields(const FieldTypeMap& field_types,
// which doesn't have a username. // which doesn't have a username.
bool IsAddingUsernameToExistingMatch( bool IsAddingUsernameToExistingMatch(
const PasswordForm& credentials, const PasswordForm& credentials,
const std::map<base::string16, const PasswordForm*>& matches) { const std::vector<const PasswordForm*>& matches) {
const auto match = matches.find(base::string16()); if (credentials.username_value.empty())
return !credentials.username_value.empty() && match != matches.end() && return false;
!match->second->is_public_suffix_match && const PasswordForm* match = FindFormByUsername(matches, base::string16());
match->second->password_value == credentials.password_value; return match && !match->is_public_suffix_match &&
match->password_value == credentials.password_value;
} }
// Helper functions for character type classification. The built-in functions // Helper functions for character type classification. The built-in functions
...@@ -180,7 +182,7 @@ VotesUploader::~VotesUploader() = default; ...@@ -180,7 +182,7 @@ VotesUploader::~VotesUploader() = default;
void VotesUploader::SendVotesOnSave( void VotesUploader::SendVotesOnSave(
const FormData& observed, const FormData& observed,
const PasswordForm& submitted_form, const PasswordForm& submitted_form,
const std::map<base::string16, const PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
PasswordForm* pending_credentials) { PasswordForm* pending_credentials) {
if (pending_credentials->times_used == 1 || if (pending_credentials->times_used == 1 ||
IsAddingUsernameToExistingMatch(*pending_credentials, best_matches)) { IsAddingUsernameToExistingMatch(*pending_credentials, best_matches)) {
...@@ -366,7 +368,7 @@ bool VotesUploader::UploadPasswordVote( ...@@ -366,7 +368,7 @@ bool VotesUploader::UploadPasswordVote(
// TODO(crbug.com/840384): Share common code with UploadPasswordVote. // TODO(crbug.com/840384): Share common code with UploadPasswordVote.
void VotesUploader::UploadFirstLoginVotes( void VotesUploader::UploadFirstLoginVotes(
const std::map<base::string16, const PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
const PasswordForm& pending_credentials, const PasswordForm& pending_credentials,
const PasswordForm& form_to_upload) { const PasswordForm& form_to_upload) {
AutofillDownloadManager* download_manager = AutofillDownloadManager* download_manager =
...@@ -485,19 +487,20 @@ void VotesUploader::AddGeneratedVote(FormStructure* form_structure) { ...@@ -485,19 +487,20 @@ void VotesUploader::AddGeneratedVote(FormStructure* form_structure) {
void VotesUploader::SetKnownValueFlag( void VotesUploader::SetKnownValueFlag(
const PasswordForm& pending_credentials, const PasswordForm& pending_credentials,
const std::map<base::string16, const PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
FormStructure* form) { FormStructure* form) {
const base::string16& known_username = pending_credentials.username_value; const base::string16& known_username = pending_credentials.username_value;
base::string16 known_password; base::string16 known_password;
if (password_overridden_) { if (password_overridden_) {
// If we are updating a password, the known value should be the old // If we are updating a password, the known value should be the old
// password, not the new one. // password, not the new one.
auto it = best_matches.find(known_username); const PasswordForm* match =
if (it == best_matches.end()) { FindFormByUsername(best_matches, known_username);
if (!match) {
// Username was not found, do nothing. // Username was not found, do nothing.
return; return;
} }
known_password = it->second->password_value; known_password = match->password_value;
} else { } else {
known_password = pending_credentials.password_value; known_password = pending_credentials.password_value;
} }
......
...@@ -42,8 +42,7 @@ class VotesUploader { ...@@ -42,8 +42,7 @@ class VotesUploader {
void SendVotesOnSave( void SendVotesOnSave(
const autofill::FormData& observed, const autofill::FormData& observed,
const autofill::PasswordForm& submitted_form, const autofill::PasswordForm& submitted_form,
const std::map<base::string16, const autofill::PasswordForm*>& const std::vector<const autofill::PasswordForm*>& best_matches,
best_matches,
autofill::PasswordForm* pending_credentials); autofill::PasswordForm* pending_credentials);
// Check to see if |pending| corresponds to an account creation form. If we // Check to see if |pending| corresponds to an account creation form. If we
...@@ -65,8 +64,7 @@ class VotesUploader { ...@@ -65,8 +64,7 @@ class VotesUploader {
// Sends USERNAME and PASSWORD votes, when a credential is used to login for // Sends USERNAME and PASSWORD votes, when a credential is used to login for
// the first time. |form_to_upload| is the submitted login form. // the first time. |form_to_upload| is the submitted login form.
void UploadFirstLoginVotes( void UploadFirstLoginVotes(
const std::map<base::string16, const autofill::PasswordForm*>& const std::vector<const autofill::PasswordForm*>& best_matches,
best_matches,
const autofill::PasswordForm& pending_credentials, const autofill::PasswordForm& pending_credentials,
const autofill::PasswordForm& form_to_upload); const autofill::PasswordForm& form_to_upload);
...@@ -150,8 +148,7 @@ class VotesUploader { ...@@ -150,8 +148,7 @@ class VotesUploader {
// contained a previously stored credential on submission. // contained a previously stored credential on submission.
void SetKnownValueFlag( void SetKnownValueFlag(
const autofill::PasswordForm& pending_credentials, const autofill::PasswordForm& pending_credentials,
const std::map<base::string16, const autofill::PasswordForm*>& const std::vector<const autofill::PasswordForm*>& best_matches,
best_matches,
autofill::FormStructure* form_to_upload); autofill::FormStructure* form_to_upload);
// Searches for |username| in |all_possible_usernames| of |match|. If the // Searches for |username| in |all_possible_usernames| of |match|. If the
......
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