Show PSL-matched credentials in the account chooser.

components/password_manager/core/browser/credential_manager_pending_request_task.cc

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;

[vabr (Chromium), 2016/11/23 18:49:11]

This did not change in this CL, but should we extend the meaning of
CREDENTIAL_MANAGER_GET_NONE_MANY_CREDENTIALS to also include the case of having
just 1 credential stored, but auto sign-in not being allowed? Or is there a
reason why we know that at this point !can_use_autosignin always means
local_results.size() > 1u?

[vasilii, 2016/11/24 12:44:10]

It's a very good question! Yes, we know that local_results.size() > 1u. The case
when zero_click_only_ == true and !delegate_->IsZeroClickAllowed() is handled in
CredentialManagerImpl::Get().

+    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());
   }
 }