Refactor CredentialManagerImpl::Get. It should use PasswordStore::GetLogins().

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 e00b02c58efd074c946de03d72a4a01b2cc6ea40..8fbdbf807b5404be456c28ffb56487f13de04ed9 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
@@ -79,19 +79,18 @@ CredentialManagerPendingRequestTask::CredentialManagerPendingRequestTask(
     CredentialManagerPendingRequestTaskDelegate* delegate,
     const SendCredentialCallback& callback,
     bool request_zero_click_only,
-    const GURL& request_origin,
     bool include_passwords,
-    const std::vector<GURL>& request_federations,
-    const std::vector<std::string>& affiliated_realms)
+    const std::vector<GURL>& request_federations)
     : delegate_(delegate),
       send_callback_(callback),
       zero_click_only_(request_zero_click_only),
-      origin_(request_origin),
-      include_passwords_(include_passwords),
-      affiliated_realms_(affiliated_realms.begin(), affiliated_realms.end()) {
+      origin_(delegate_->GetOrigin()),
+      include_passwords_(include_passwords) {
   CHECK(!delegate_->client()->DidLastPageLoadEncounterSSLErrors());
+  federations_.reserve(request_federations.size());
   for (const GURL& federation : request_federations)
-    federations_.insert(url::Origin(federation.GetOrigin()).Serialize());
+    federations_.push_back(url::Origin(federation.GetOrigin()).Serialize());
+  std::sort(federations_.begin(), federations_.end());
 }
 
 CredentialManagerPendingRequestTask::~CredentialManagerPendingRequestTask() =
@@ -111,13 +110,14 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
   }
 
   std::vector<std::unique_ptr<autofill::PasswordForm>> local_results;
-  std::vector<std::unique_ptr<autofill::PasswordForm>> affiliated_results;
+  std::vector<std::unique_ptr<autofill::PasswordForm>> psl_results;

[vabr (Chromium), 2016/11/23 08:57:56]

The CL description says that the PSL results are filtered out, and indeed, the
code only pushes_back to |psl_results| but never uses what is stored there.

Why cannot we just keep those in |results| and execute continue; on line 132?
Alternatively, what about calling |psl_results| something like
|filtered_out_results| to make it clearer that it is a sink vector?

[vasilii, 2016/11/23 09:40:54]

We can but it's not a refactoring on its own. It's a preparation for fixing the
bug. In the next CL today or tomorrow |psl_results| is to be used below.

[vabr (Chromium), 2016/11/23 09:51:03]

Acknowledged.

   for (auto& form : results) {
     // Ensure that the form we're looking at matches the password and
     // federation filters provided.
     if (!((form->federation_origin.unique() && include_passwords_) ||
           (!form->federation_origin.unique() &&
-           federations_.count(form->federation_origin.Serialize())))) {
+           std::binary_search(federations_.begin(), federations_.end(),
+                              form->federation_origin.Serialize())))) {
       continue;
     }
 
@@ -125,22 +125,14 @@ void CredentialManagerPendingRequestTask::OnGetPasswordStoreResults(
     // PasswordForm definition: scheme, host, port and path.
     // GURL definition: scheme, host, and port.
     // So we can't compare them directly.
-    if (form->origin.GetOrigin() == origin_.GetOrigin()) {
+    if (form->is_affiliation_based_match ||
+        form->origin.GetOrigin() == origin_.GetOrigin()) {
       local_results.push_back(std::move(form));
-    } else if (affiliated_realms_.count(form->signon_realm) &&
-               AffiliatedMatchHelper::IsValidAndroidCredential(
-                   PasswordStore::FormDigest(*form))) {
-      form->is_affiliation_based_match = true;
-      affiliated_results.push_back(std::move(form));
+    } else if (form->is_public_suffix_match) {
+      psl_results.push_back(std::move(form));
     }
   }
 
-  if (!affiliated_results.empty()) {
-    password_manager_util::TrimUsernameOnlyCredentials(&affiliated_results);
-    std::move(affiliated_results.begin(), affiliated_results.end(),
-              std::back_inserter(local_results));
-  }
-
   // Remove empty usernames from the list.
   auto begin_empty =
       std::remove_if(local_results.begin(), local_results.end(),