Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(8513)

Unified Diff: components/password_manager/core/browser/credential_manager_pending_request_task.cc

Issue 2523593006: Show PSL-matched credentials in the account chooser. (Closed)
Patch Set: nit Created 4 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/password_manager/core/browser/credential_manager_pending_request_task.cc
diff --git a/components/password_manager/core/browser/credential_manager_pending_request_task.cc b/components/password_manager/core/browser/credential_manager_pending_request_task.cc
index ec9b8403adda279ee58ceffd7e975d9dbf30d8d6..8508bda272a96471a699a1c6f8f0f8147e65b72b 100644
--- a/components/password_manager/core/browser/credential_manager_pending_request_task.cc
+++ b/components/password_manager/core/browser/credential_manager_pending_request_task.cc
@@ -55,15 +55,21 @@ bool IsBetterMatch(const autofill::PasswordForm& form1,
void FilterDuplicates(
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) {
std::vector<std::unique_ptr<autofill::PasswordForm>> federated_forms;
- std::map<base::string16, std::unique_ptr<autofill::PasswordForm>> credentials;
+ // The key is [username, signon_realm]. signon_realm is used only for PSL
+ // matches because those entries have it in the UI.
+ std::map<std::pair<base::string16, std::string>,
+ std::unique_ptr<autofill::PasswordForm>>
+ credentials;
for (auto& form : *forms) {
if (!form->federation_origin.unique()) {
federated_forms.push_back(std::move(form));
} else {
- auto it = credentials.find(form->username_value);
- if (it == credentials.end() || IsBetterMatch(*form, *it->second)) {
- credentials[form->username_value] = std::move(form);
- }
+ auto key = std::make_pair(
+ form->username_value,
+ form->is_public_suffix_match ? form->signon_realm : std::string());
+ auto it = credentials.find(key);
+ if (it == credentials.end() || IsBetterMatch(*form, *it->second))
+ credentials[key] = std::move(form);
}
}
forms->clear();
@@ -73,6 +79,26 @@ void FilterDuplicates(
std::back_inserter(*forms));
}
+// Sift |forms| for the account chooser so it doesn't have empty usernames or
+// duplicates.
+void FilterDuplicatesAndEmptyUsername(
+ std::vector<std::unique_ptr<autofill::PasswordForm>>* forms,
+ bool* has_empty_username,
+ bool* has_duplicates) {
+ // Remove empty usernames from the list.
+ auto begin_empty =
+ std::remove_if(forms->begin(), forms->end(),
+ [](const std::unique_ptr<autofill::PasswordForm>& form) {
+ return form->username_value.empty();
+ });
+ *has_empty_username = (begin_empty != forms->end());
+ forms->erase(begin_empty, forms->end());
+
+ const size_t size_before = forms->size();
+ FilterDuplicates(forms);
+ *has_duplicates = (size_before != forms->size());
+}
+
} // namespace
CredentialManagerPendingRequestTask::CredentialManagerPendingRequestTask(
@@ -130,26 +156,10 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
}
}
- // Remove empty usernames from the list.
- auto begin_empty =
- std::remove_if(local_results.begin(), local_results.end(),
- [](const std::unique_ptr<autofill::PasswordForm>& form) {
- return form->username_value.empty();
- });
- const bool has_empty_username = (begin_empty != local_results.end());
- local_results.erase(begin_empty, local_results.end());
-
- const size_t local_results_size = local_results.size();
- FilterDuplicates(&local_results);
- const bool has_duplicates = (local_results_size != local_results.size());
-
- if (local_results.empty()) {
- LogCredentialManagerGetResult(
- metrics_util::CREDENTIAL_MANAGER_GET_NONE_EMPTY_STORE,
- mediation_status);
- delegate_->SendCredential(send_callback_, CredentialInfo());
- return;
- }
+ bool has_empty_username = false;
+ bool has_duplicates = false;
+ FilterDuplicatesAndEmptyUsername(&local_results, &has_empty_username,
+ &has_duplicates);
// We only perform zero-click sign-in when the result is completely
// unambigious. If there is one and only one entry, and zero-click is
@@ -157,8 +167,8 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
//
// Moreover, we only return such a credential if the user has opted-in via the
// first-run experience.
- bool can_use_autosignin = local_results.size() == 1u &&
- delegate_->IsZeroClickAllowed();
+ const bool can_use_autosignin =
+ local_results.size() == 1u && delegate_->IsZeroClickAllowed();
if (can_use_autosignin && !local_results[0]->skip_zero_click &&
!password_bubble_experiment::ShouldShowAutoSignInPromptFirstRunExperience(
delegate_->client()->GetPrefs())) {
@@ -175,41 +185,56 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
return;
}
- // Otherwise, return an empty credential if we're in zero-click-only mode
- // or if the user chooses not to return a credential, and the credential the
- // user chooses if they pick one.
- std::unique_ptr<autofill::PasswordForm> potential_autosignin_form(
- can_use_autosignin ? new autofill::PasswordForm(*local_results[0])
- : nullptr);
- if (!zero_click_only_)
- ReportAccountChooserUsabilityMetrics(has_duplicates, has_empty_username);
- if (zero_click_only_ ||
- !delegate_->client()->PromptUserToChooseCredentials(
- std::move(local_results), origin_,
- base::Bind(
- &CredentialManagerPendingRequestTaskDelegate::SendPasswordForm,
- base::Unretained(delegate_), send_callback_))) {
- metrics_util::CredentialManagerGetResult get_result =
- metrics_util::CREDENTIAL_MANAGER_GET_NONE;
- if (zero_click_only_) {
- if (!can_use_autosignin)
- get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_MANY_CREDENTIALS;
- else if (potential_autosignin_form->skip_zero_click)
- get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_SIGNED_OUT;
- else
- get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_FIRST_RUN;
- }
+ if (zero_click_only_) {
+ metrics_util::CredentialManagerGetResult get_result;
+ if (local_results.empty())
+ get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_EMPTY_STORE;
+ else if (!can_use_autosignin)
+ get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_MANY_CREDENTIALS;
+ else if (local_results[0]->skip_zero_click)
+ get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_SIGNED_OUT;
+ else
+ get_result = metrics_util::CREDENTIAL_MANAGER_GET_NONE_FIRST_RUN;
+
if (can_use_autosignin) {
// The user had credentials, but either chose not to share them with the
// site, or was prevented from doing so by lack of zero-click (or the
// first-run experience). So, notify the client that we could potentially
// have used zero-click.
delegate_->client()->NotifyUserCouldBeAutoSignedIn(
- std::move(potential_autosignin_form));
+ std::move(local_results[0]));
}
LogCredentialManagerGetResult(get_result, mediation_status);
delegate_->SendCredential(send_callback_, CredentialInfo());
+ return;
+ }
+
+ // Time to show the account chooser. If |local_results| is empty then it
+ // should list the PSL matches.
+ if (local_results.empty()) {
+ local_results = std::move(psl_results);
+ FilterDuplicatesAndEmptyUsername(&local_results, &has_empty_username,
+ &has_duplicates);
+ }
+
+ if (local_results.empty()) {
+ LogCredentialManagerGetResult(
+ metrics_util::CREDENTIAL_MANAGER_GET_NONE_EMPTY_STORE,
+ mediation_status);
+ delegate_->SendCredential(send_callback_, CredentialInfo());
+ return;
+ }
+
+ ReportAccountChooserUsabilityMetrics(has_duplicates, has_empty_username);
+ if (!delegate_->client()->PromptUserToChooseCredentials(
+ std::move(local_results), origin_,
+ base::Bind(
+ &CredentialManagerPendingRequestTaskDelegate::SendPasswordForm,
+ base::Unretained(delegate_), send_callback_))) {
+ LogCredentialManagerGetResult(metrics_util::CREDENTIAL_MANAGER_GET_NONE,
+ mediation_status);
+ delegate_->SendCredential(send_callback_, CredentialInfo());
}
}

Powered by Google App Engine
This is Rietveld 408576698