password_manager_util::FindBestMatches: return vector instead of map

FindBestMatches now returns the best matches in a vector, instead of in
a map keyed by username.
For now, FormFetcher[Impl] still sorts the matches by username; that'll
be addressed in followup CLs.

Bug: 1011399
Change-Id: I217aa7646bdc9eb9bf8cb2ff244f54f17b4f7a65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1844969
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703679}

components/password_manager/core/browser/fake_form_fetcher.cc

@@ -65,10 +65,15 @@ const PasswordForm* FakeFormFetcher::GetPreferredMatch() const {
 void FakeFormFetcher::SetNonFederated(
     const std::vector<const PasswordForm*>& non_federated) {
   non_federated_ = non_federated;
+  std::vector<const PasswordForm*> best_matches;
   password_manager_util::FindBestMatches(
       non_federated_, scheme_,
       /*sort_matches_by_date_last_used=*/false, &non_federated_same_scheme_,
-      &best_matches_, &preferred_match_);
+      &best_matches, &preferred_match_);
+  best_matches_.clear();
+  for (const auto* match : best_matches) {
+    best_matches_[match->username_value] = match;
+  }
 }
 
 void FakeFormFetcher::SetBlacklisted(bool is_blacklisted) {

components/password_manager/core/browser/form_fetcher_impl.cc

@@ -216,10 +216,15 @@ std::unique_ptr<FormFetcher> FormFetcherImpl::Clone() {
   result->non_federated_ = MakeCopies(non_federated_);
   result->federated_ = MakeCopies(federated_);
   result->is_blacklisted_ = is_blacklisted_;
+  std::vector<const PasswordForm*> best_matches;
   password_manager_util::FindBestMatches(
       MakeWeakCopies(result->non_federated_), form_digest_.scheme,
       sort_matches_by_date_last_used_, &result->non_federated_same_scheme_,
-      &result->best_matches_, &result->preferred_match_);
+      &best_matches, &result->preferred_match_);
+  result->best_matches_.clear();
+  for (const auto* match : best_matches) {
+    result->best_matches_[match->username_value] = match;
+  }
 
   result->interactions_stats_ = interactions_stats_;
   result->state_ = state_;
@@ -234,10 +239,15 @@ void FormFetcherImpl::ProcessPasswordStoreResults(
   state_ = State::NOT_WAITING;
   SplitResults(std::move(results));
 
+  std::vector<const PasswordForm*> best_matches;
   password_manager_util::FindBestMatches(
       MakeWeakCopies(non_federated_), form_digest_.scheme,
       sort_matches_by_date_last_used_, &non_federated_same_scheme_,
-      &best_matches_, &preferred_match_);
+      &best_matches, &preferred_match_);
+  best_matches_.clear();
+  for (const auto* match : best_matches) {
+    best_matches_[match->username_value] = match;
+  }
 
   for (auto* consumer : consumers_)
     consumer->OnFetchCompleted();

components/password_manager/core/browser/password_manager_util.cc

@@ -218,8 +218,8 @@ void FindBestMatches(
     const std::vector<const PasswordForm*>& non_federated_matches,
     PasswordForm::Scheme scheme,
     bool sort_matches_by_date_last_used,
-    std::vector<const autofill::PasswordForm*>* non_federated_same_scheme,
-    std::map<base::string16, const PasswordForm*>* best_matches,
+    std::vector<const PasswordForm*>* non_federated_same_scheme,
+    std::vector<const PasswordForm*>* best_matches,
     const PasswordForm** preferred_match) {
   DCHECK(std::all_of(
       non_federated_matches.begin(), non_federated_matches.end(),
@@ -246,11 +246,14 @@ void FindBestMatches(
             sort_matches_by_date_last_used ? IsBetterMatchUsingLastUsed
                                            : IsBetterMatch);
 
+  std::set<base::string16> usernames;
   for (const auto* match : *non_federated_same_scheme) {
     const base::string16& username = match->username_value;
     // The first match for |username| in the sorted array is best match.
-    if (best_matches->find(username) == best_matches->end())
-      best_matches->insert(std::make_pair(username, match));
+    if (!base::Contains(usernames, username)) {
+      usernames.insert(username);
+      best_matches->push_back(match);
+    }
   }
 
   *preferred_match = *non_federated_same_scheme->begin();

components/password_manager/core/browser/password_manager_util.h

@@ -103,17 +103,17 @@ base::StringPiece GetSignonRealmWithProtocolExcluded(
     const autofill::PasswordForm& form);
 
 // Given all non-blacklisted |non_federated_matches|, finds and populates
-// |best_matches| and |preferred_match_| accordingly. For comparing credentials
-// the following rule is used: non-psl match is better than psl match, preferred
-// match is better than non-preferred match. In case of tie, an arbitrary
-// credential from the tied ones is chosen for |best_matches| and
-// preferred_match.
+// |non_federated_same_scheme|, |best_matches|, and |preferred_match|
+// accordingly. For comparing credentials the following rule is used: non-psl
+// match is better than psl match, preferred match is better than non-preferred
+// match. In case of tie, an arbitrary credential from the tied ones is chosen
+// for |best_matches| and |preferred_match|.
 void FindBestMatches(
     const std::vector<const autofill::PasswordForm*>& non_federated_matches,
     autofill::PasswordForm::Scheme scheme,
     bool sort_matches_by_date_last_used,
     std::vector<const autofill::PasswordForm*>* non_federated_same_scheme,
-    std::map<base::string16, const autofill::PasswordForm*>* best_matches,
+    std::vector<const autofill::PasswordForm*>* best_matches,
     const autofill::PasswordForm** preferred_match);
 
 // If the user submits a form, they may have used existing credentials, new

components/password_manager/core/browser/password_manager_util_unittest.cc

@@ -197,7 +197,7 @@ TEST(PasswordManagerUtil, FindBestMatches) {
     for (const PasswordForm& match : owning_matches)
       matches.push_back(&match);
 
-    std::map<base::string16, const PasswordForm*> best_matches;
+    std::vector<const PasswordForm*> best_matches;
     const PasswordForm* preferred_match = nullptr;
 
     std::vector<const PasswordForm*> same_scheme_matches;
@@ -217,15 +217,14 @@ TEST(PasswordManagerUtil, FindBestMatches) {
       ASSERT_EQ(test_case.expected_best_matches_indices.size(),
                 best_matches.size());
 
-      for (const auto& username_match : best_matches) {
-        std::string username = base::UTF16ToUTF8(username_match.first);
+      for (const PasswordForm* match : best_matches) {
+        std::string username = base::UTF16ToUTF8(match->username_value);
         ASSERT_NE(test_case.expected_best_matches_indices.end(),
                   test_case.expected_best_matches_indices.find(username));
         size_t expected_index =
             test_case.expected_best_matches_indices.at(username);
         size_t actual_index = std::distance(
-            matches.begin(),
-            std::find(matches.begin(), matches.end(), username_match.second));
+            matches.begin(), std::find(matches.begin(), matches.end(), match));
         EXPECT_EQ(expected_index, actual_index);
       }
     }
@@ -352,7 +351,7 @@ TEST(PasswordManagerUtil, FindBestMatchesByUsageTime) {
     for (const PasswordForm& match : owning_matches)
       matches.push_back(&match);
 
-    std::map<base::string16, const PasswordForm*> best_matches;
+    std::vector<const PasswordForm*> best_matches;
     const PasswordForm* preferred_match = nullptr;
 
     std::vector<const PasswordForm*> same_scheme_matches;
@@ -372,15 +371,14 @@ TEST(PasswordManagerUtil, FindBestMatchesByUsageTime) {
       ASSERT_EQ(test_case.expected_best_matches_indices.size(),
                 best_matches.size());
 
-      for (const auto& username_match : best_matches) {
-        std::string username = base::UTF16ToUTF8(username_match.first);
+      for (const PasswordForm* match : best_matches) {
+        std::string username = base::UTF16ToUTF8(match->username_value);
         ASSERT_NE(test_case.expected_best_matches_indices.end(),
                   test_case.expected_best_matches_indices.find(username));
         size_t expected_index =
             test_case.expected_best_matches_indices.at(username);
         size_t actual_index = std::distance(
-            matches.begin(),
-            std::find(matches.begin(), matches.end(), username_match.second));
+            matches.begin(), std::find(matches.begin(), matches.end(), match));
         EXPECT_EQ(expected_index, actual_index);
       }
     }