Commit fab8f314 authored by Vadym Doroshenko's avatar Vadym Doroshenko Committed by Commit Bot

Password Manager code clean-up.

It contains a number of tiny clean ups:
1.Using range based for
2.Dropping autofill:: namespace in favor of using
3.Using map lookup instead of range based for by map.
4.Old comment removal in password_manager.cc

Bug: 875768
Change-Id: I6d7c481139d59e8de35ec49ad58f9486de04359d
Reviewed-on: https://chromium-review.googlesource.com/1177743
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584439}
parent 703c17e1
...@@ -421,14 +421,11 @@ bool PasswordAutofillManager::GetPasswordAndRealmForUsername( ...@@ -421,14 +421,11 @@ bool PasswordAutofillManager::GetPasswordAndRealmForUsername(
} }
// Scan additional logins for a match. // Scan additional logins for a match.
for (autofill::PasswordFormFillData::LoginCollection::const_iterator iter = auto iter = fill_data.additional_logins.find(current_username);
fill_data.additional_logins.begin(); if (iter != fill_data.additional_logins.end()) {
iter != fill_data.additional_logins.end(); ++iter) {
if (iter->first == current_username) {
*password_and_realm = iter->second; *password_and_realm = iter->second;
return true; return true;
} }
}
return false; return false;
} }
......
...@@ -87,14 +87,14 @@ bool URLsEqualUpToHttpHttpsSubstitution(const GURL& a, const GURL& b) { ...@@ -87,14 +87,14 @@ bool URLsEqualUpToHttpHttpsSubstitution(const GURL& a, const GURL& b) {
// Since empty or unspecified form's action is automatically set to the page // Since empty or unspecified form's action is automatically set to the page
// origin, this function checks if a form's action is empty by comparing it to // origin, this function checks if a form's action is empty by comparing it to
// its origin. // its origin.
bool HasNonEmptyAction(const autofill::PasswordForm& form) { bool HasNonEmptyAction(const PasswordForm& form) {
return form.action != form.origin; return form.action != form.origin;
} }
// Checks if the observed form looks like the submitted one to handle "Invalid // Checks if the observed form looks like the submitted one to handle "Invalid
// password entered" case so we don't offer a password save when we shouldn't. // password entered" case so we don't offer a password save when we shouldn't.
bool IsPasswordFormReappeared(const autofill::PasswordForm& observed_form, bool IsPasswordFormReappeared(const PasswordForm& observed_form,
const autofill::PasswordForm& submitted_form) { const PasswordForm& submitted_form) {
if (observed_form.action.is_valid() && HasNonEmptyAction(observed_form) && if (observed_form.action.is_valid() && HasNonEmptyAction(observed_form) &&
HasNonEmptyAction(submitted_form) && HasNonEmptyAction(submitted_form) &&
URLsEqualUpToHttpHttpsSubstitution(submitted_form.action, URLsEqualUpToHttpHttpsSubstitution(submitted_form.action,
...@@ -191,7 +191,7 @@ bool IsPasswordUpdate(const PasswordFormManager& provisional_save_manager) { ...@@ -191,7 +191,7 @@ bool IsPasswordUpdate(const PasswordFormManager& provisional_save_manager) {
// Finds the matched form manager for |form| in |pending_login_managers|. // Finds the matched form manager for |form| in |pending_login_managers|.
PasswordFormManager* FindMatchedManager( PasswordFormManager* FindMatchedManager(
const autofill::PasswordForm& form, const PasswordForm& form,
const std::vector<std::unique_ptr<PasswordFormManager>>& const std::vector<std::unique_ptr<PasswordFormManager>>&
pending_login_managers, pending_login_managers,
const password_manager::PasswordManagerDriver* driver, const password_manager::PasswordManagerDriver* driver,
...@@ -335,8 +335,7 @@ void PasswordManager::GenerationAvailableForForm(const PasswordForm& form) { ...@@ -335,8 +335,7 @@ void PasswordManager::GenerationAvailableForForm(const PasswordForm& form) {
} }
} }
void PasswordManager::OnPresaveGeneratedPassword( void PasswordManager::OnPresaveGeneratedPassword(const PasswordForm& form) {
const autofill::PasswordForm& form) {
DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage()); DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage());
PasswordFormManager* form_manager = GetMatchingPendingManager(form); PasswordFormManager* form_manager = GetMatchingPendingManager(form);
if (form_manager) { if (form_manager) {
...@@ -361,7 +360,7 @@ void PasswordManager::OnPasswordNoLongerGenerated(const PasswordForm& form) { ...@@ -361,7 +360,7 @@ void PasswordManager::OnPasswordNoLongerGenerated(const PasswordForm& form) {
void PasswordManager::SetGenerationElementAndReasonForForm( void PasswordManager::SetGenerationElementAndReasonForForm(
password_manager::PasswordManagerDriver* driver, password_manager::PasswordManagerDriver* driver,
const autofill::PasswordForm& form, const PasswordForm& form,
const base::string16& generation_element, const base::string16& generation_element,
bool is_manually_triggered) { bool is_manually_triggered) {
DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage()); DCHECK(client_->IsSavingAndFillingEnabledForCurrentPage());
...@@ -522,7 +521,7 @@ void PasswordManager::OnPasswordFormSubmitted( ...@@ -522,7 +521,7 @@ void PasswordManager::OnPasswordFormSubmitted(
void PasswordManager::OnPasswordFormSubmittedNoChecks( void PasswordManager::OnPasswordFormSubmittedNoChecks(
password_manager::PasswordManagerDriver* driver, password_manager::PasswordManagerDriver* driver,
const autofill::PasswordForm& password_form) { const PasswordForm& password_form) {
if (password_manager_util::IsLoggingActive(client_)) { if (password_manager_util::IsLoggingActive(client_)) {
BrowserSavePasswordProgressLogger logger(client_->GetLogManager()); BrowserSavePasswordProgressLogger logger(client_->GetLogManager());
logger.LogMessage(Logger::STRING_ON_SAME_DOCUMENT_NAVIGATION); logger.LogMessage(Logger::STRING_ON_SAME_DOCUMENT_NAVIGATION);
...@@ -666,20 +665,19 @@ void PasswordManager::CreatePendingLoginManagers( ...@@ -666,20 +665,19 @@ void PasswordManager::CreatePendingLoginManagers(
pending_login_managers_.size()); pending_login_managers_.size());
} }
for (std::vector<PasswordForm>::const_iterator iter = forms.begin(); for (const PasswordForm& form : forms) {
iter != forms.end(); ++iter) {
// Don't involve the password manager if this form corresponds to // Don't involve the password manager if this form corresponds to
// SpdyProxy authentication, as indicated by the realm. // SpdyProxy authentication, as indicated by the realm.
if (base::EndsWith(iter->signon_realm, kSpdyProxyRealm, if (base::EndsWith(form.signon_realm, kSpdyProxyRealm,
base::CompareCase::SENSITIVE)) base::CompareCase::SENSITIVE))
continue; continue;
if (iter->is_gaia_with_skip_save_password_form) if (form.is_gaia_with_skip_save_password_form)
continue; continue;
bool old_manager_found = false; bool old_manager_found = false;
for (const auto& old_manager : pending_login_managers_) { for (const auto& old_manager : pending_login_managers_) {
if (old_manager->DoesManage(*iter, driver) != if (old_manager->DoesManage(form, driver) !=
PasswordFormManager::RESULT_COMPLETE_MATCH) { PasswordFormManager::RESULT_COMPLETE_MATCH) {
continue; continue;
} }
...@@ -692,23 +690,23 @@ void PasswordManager::CreatePendingLoginManagers( ...@@ -692,23 +690,23 @@ void PasswordManager::CreatePendingLoginManagers(
continue; // The current form is already managed. continue; // The current form is already managed.
UMA_HISTOGRAM_BOOLEAN("PasswordManager.EmptyUsernames.ParsedUsernameField", UMA_HISTOGRAM_BOOLEAN("PasswordManager.EmptyUsernames.ParsedUsernameField",
iter->username_element.empty()); form.username_element.empty());
// Out of the forms not containing a username field, determine how many // Out of the forms not containing a username field, determine how many
// are password change forms. // are password change forms.
if (iter->username_element.empty()) { if (form.username_element.empty()) {
UMA_HISTOGRAM_BOOLEAN( UMA_HISTOGRAM_BOOLEAN(
"PasswordManager.EmptyUsernames." "PasswordManager.EmptyUsernames."
"FormWithoutUsernameFieldIsPasswordChangeForm", "FormWithoutUsernameFieldIsPasswordChangeForm",
!iter->new_password_element.empty()); form.new_password_element.empty());
} }
if (logger) if (logger)
logger->LogFormSignatures(Logger::STRING_ADDING_SIGNATURE, *iter); logger->LogFormSignatures(Logger::STRING_ADDING_SIGNATURE, form);
auto manager = std::make_unique<PasswordFormManager>( auto manager = std::make_unique<PasswordFormManager>(
this, client_, this, client_,
(driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>()), (driver ? driver->AsWeakPtr() : base::WeakPtr<PasswordManagerDriver>()),
*iter, std::make_unique<FormSaverImpl>(client_->GetPasswordStore()), form, std::make_unique<FormSaverImpl>(client_->GetPasswordStore()),
nullptr); nullptr);
manager->Init(nullptr); manager->Init(nullptr);
pending_login_managers_.push_back(std::move(manager)); pending_login_managers_.push_back(std::move(manager));
...@@ -722,7 +720,7 @@ void PasswordManager::CreatePendingLoginManagers( ...@@ -722,7 +720,7 @@ void PasswordManager::CreatePendingLoginManagers(
void PasswordManager::CreateFormManagers( void PasswordManager::CreateFormManagers(
password_manager::PasswordManagerDriver* driver, password_manager::PasswordManagerDriver* driver,
const std::vector<autofill::PasswordForm>& forms) { const std::vector<PasswordForm>& forms) {
// Find new forms. // Find new forms.
std::vector<const PasswordForm*> new_forms; std::vector<const PasswordForm*> new_forms;
for (const PasswordForm& form : forms) { for (const PasswordForm& form : forms) {
...@@ -779,7 +777,7 @@ void PasswordManager::ProcessSubmittedForm( ...@@ -779,7 +777,7 @@ void PasswordManager::ProcessSubmittedForm(
} }
void PasswordManager::ReportSpecPriorityForGeneratedPassword( void PasswordManager::ReportSpecPriorityForGeneratedPassword(
const autofill::PasswordForm& password_form, const PasswordForm& password_form,
uint32_t spec_priority) { uint32_t spec_priority) {
PasswordFormManager* form_manager = GetMatchingPendingManager(password_form); PasswordFormManager* form_manager = GetMatchingPendingManager(password_form);
if (form_manager && form_manager->GetMetricsRecorder()) { if (form_manager && form_manager->GetMetricsRecorder()) {
...@@ -897,22 +895,17 @@ void PasswordManager::OnPasswordFormsRendered( ...@@ -897,22 +895,17 @@ void PasswordManager::OnPasswordFormsRendered(
if (did_stop_loading) { if (did_stop_loading) {
if (provisional_save_manager_->GetPendingCredentials().scheme == if (provisional_save_manager_->GetPendingCredentials().scheme ==
PasswordForm::SCHEME_HTML) { PasswordForm::SCHEME_HTML) {
for (size_t i = 0; i < all_visible_forms_.size(); ++i) { for (const PasswordForm& form : all_visible_forms_) {
// TODO(vabr): The similarity check is just action equality up to
// HTTP<->HTTPS substitution for now. If it becomes more complex, it may
// make sense to consider modifying and using
// PasswordFormManager::DoesManage for it.
if (IsPasswordFormReappeared( if (IsPasswordFormReappeared(
all_visible_forms_[i], form, provisional_save_manager_->GetPendingCredentials())) {
provisional_save_manager_->GetPendingCredentials())) {
if (provisional_save_manager_ if (provisional_save_manager_
->is_possible_change_password_form_without_username() && ->is_possible_change_password_form_without_username() &&
AreAllFieldsEmpty(all_visible_forms_[i])) AreAllFieldsEmpty(form))
continue; continue;
provisional_save_manager_->LogSubmitFailed(); provisional_save_manager_->LogSubmitFailed();
if (logger) { if (logger) {
logger->LogPasswordForm(Logger::STRING_PASSWORD_FORM_REAPPEARED, logger->LogPasswordForm(Logger::STRING_PASSWORD_FORM_REAPPEARED,
all_visible_forms_[i]); form);
logger->LogMessage(Logger::STRING_DECISION_DROP); logger->LogMessage(Logger::STRING_DECISION_DROP);
} }
provisional_save_manager_.reset(); provisional_save_manager_.reset();
...@@ -1027,7 +1020,7 @@ void PasswordManager::MaybeSavePasswordHash() { ...@@ -1027,7 +1020,7 @@ void PasswordManager::MaybeSavePasswordHash() {
if (!store) if (!store)
return; return;
const autofill::PasswordForm* password_form = const PasswordForm* password_form =
provisional_save_manager_->submitted_form(); provisional_save_manager_->submitted_form();
bool should_save_enterprise_pw = bool should_save_enterprise_pw =
......
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