[Passwords] Introduce FormFetcher.GetAllRelevantMatches()

Change-Id: I8fa5b9a5c7046fb56ed97a08dbd8c369b8663978
Bug: 991510
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1783151
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693617}

components/password_manager/core/browser/fake_form_fetcher.cc

@@ -49,6 +49,11 @@ std::vector<const PasswordForm*> FakeFormFetcher::GetBlacklistedMatches()
   return blacklisted_;
 }
 
+const std::vector<const PasswordForm*>& FakeFormFetcher::GetAllRelevantMatches()
+    const {
+  return non_federated_same_scheme_;
+}
+
 const std::map<base::string16, const PasswordForm*>&
 FakeFormFetcher::GetBestMatches() const {
   return best_matches_;
@@ -62,6 +67,7 @@ void FakeFormFetcher::SetNonFederated(
     const std::vector<const PasswordForm*>& non_federated) {
   non_federated_ = non_federated;
   password_manager_util::FindBestMatches(non_federated_, scheme_,
+                                         &non_federated_same_scheme_,
                                          &best_matches_, &preferred_match_);
 }
 

components/password_manager/core/browser/fake_form_fetcher.h

@@ -60,6 +60,9 @@ class FakeFormFetcher : public FormFetcher {
   std::vector<const autofill::PasswordForm*> GetBlacklistedMatches()
       const override;
 
+  const std::vector<const autofill::PasswordForm*>& GetAllRelevantMatches()
+      const override;
+
   const std::map<base::string16, const autofill::PasswordForm*>&
   GetBestMatches() const override;
 
@@ -94,6 +97,7 @@ class FakeFormFetcher : public FormFetcher {
   std::vector<const autofill::PasswordForm*> non_federated_;
   std::vector<const autofill::PasswordForm*> federated_;
   std::vector<const autofill::PasswordForm*> blacklisted_;
+  std::vector<const autofill::PasswordForm*> non_federated_same_scheme_;
   std::map<base::string16, const autofill::PasswordForm*> best_matches_;
   const autofill::PasswordForm* preferred_match_ = nullptr;
 

components/password_manager/core/browser/form_fetcher.h

@@ -78,6 +78,11 @@ class FormFetcher {
   virtual std::vector<const autofill::PasswordForm*> GetBlacklistedMatches()
       const = 0;
 
+  // Non-federated matches obtained from the backend that have the same scheme
+  // of this form.
+  virtual const std::vector<const autofill::PasswordForm*>&
+  GetAllRelevantMatches() const = 0;
+
   // Nonblacklisted matches obtained from the backend.
   virtual const std::map<base::string16, const autofill::PasswordForm*>&
   GetBestMatches() const = 0;

components/password_manager/core/browser/form_fetcher_impl.cc

@@ -131,6 +131,11 @@ std::vector<const PasswordForm*> FormFetcherImpl::GetBlacklistedMatches()
   return MakeWeakCopies(blacklisted_);
 }
 
+const std::vector<const PasswordForm*>& FormFetcherImpl::GetAllRelevantMatches()
+    const {
+  return non_federated_same_scheme_;
+}
+
 const std::map<base::string16, const PasswordForm*>&
 FormFetcherImpl::GetBestMatches() const {
   return best_matches_;
@@ -233,7 +238,8 @@ std::unique_ptr<FormFetcher> FormFetcherImpl::Clone() {
   result->blacklisted_ = MakeCopies(blacklisted_);
   password_manager_util::FindBestMatches(
       MakeWeakCopies(result->non_federated_), form_digest_.scheme,
-      &result->best_matches_, &result->preferred_match_);
+      &result->non_federated_same_scheme_, &result->best_matches_,
+      &result->preferred_match_);
 
   result->interactions_stats_ = this->interactions_stats_;
   result->state_ = this->state_;
@@ -251,9 +257,9 @@ void FormFetcherImpl::ProcessPasswordStoreResults(
   non_federated_ = std::move(matches.non_federated);
   blacklisted_ = std::move(matches.blacklisted);
 
-  password_manager_util::FindBestMatches(MakeWeakCopies(non_federated_),
-                                         form_digest_.scheme, &best_matches_,
-                                         &preferred_match_);
+  password_manager_util::FindBestMatches(
+      MakeWeakCopies(non_federated_), form_digest_.scheme,
+      &non_federated_same_scheme_, &best_matches_, &preferred_match_);
 
   for (auto* consumer : consumers_)
     consumer->OnFetchCompleted();

components/password_manager/core/browser/form_fetcher_impl.h

@@ -47,6 +47,9 @@ class FormFetcherImpl : public FormFetcher,
   std::vector<const autofill::PasswordForm*> GetBlacklistedMatches()
       const override;
 
+  const std::vector<const autofill::PasswordForm*>& GetAllRelevantMatches()
+      const override;
+
   const std::map<base::string16, const autofill::PasswordForm*>&
   GetBestMatches() const override;
 
@@ -83,6 +86,9 @@ class FormFetcherImpl : public FormFetcher,
   // List of blacklisted credentials obtained form the password store.
   std::vector<std::unique_ptr<autofill::PasswordForm>> blacklisted_;
 
+  // Non-federated credentials of the same scheme as the observed form.
+  std::vector<const autofill::PasswordForm*> non_federated_same_scheme_;
+
   // Set of nonblacklisted PasswordForms from the password store that best match
   // the form being managed by |this|, indexed by username.
   std::map<base::string16, const autofill::PasswordForm*> best_matches_;

components/password_manager/core/browser/password_form_manager.cc

@@ -935,7 +935,8 @@ void PasswordFormManager::CreatePendingCredentials() {
     // Generate username correction votes.
     bool username_correction_found =
         votes_uploader_.FindCorrectedUsernameElement(
-            GetAllMatches(), parsed_submitted_form_->username_value,
+            form_fetcher_->GetAllRelevantMatches(),
+            parsed_submitted_form_->username_value,
             parsed_submitted_form_->password_value);
     UMA_HISTOGRAM_BOOLEAN("PasswordManager.UsernameCorrectionFound",
                           username_correction_found);
@@ -1105,8 +1106,8 @@ void PasswordFormManager::PresaveGeneratedPasswordInternal(
   // generated password is saved.
   parsed_form->password_value = generated_password;
 
-  generation_state_->PresaveGeneratedPassword(std::move(*parsed_form),
-                                              GetAllMatches());
+  generation_state_->PresaveGeneratedPassword(
+      std::move(*parsed_form), form_fetcher_->GetAllRelevantMatches());
 }
 
 void PasswordFormManager::CalculateFillingAssistanceMetric(
@@ -1132,16 +1133,6 @@ void PasswordFormManager::CalculateFillingAssistanceMetric(
 #endif
 }
 
-std::vector<const PasswordForm*> PasswordFormManager::GetAllMatches() const {
-  std::vector<const autofill::PasswordForm*> result =
-      form_fetcher_->GetNonFederatedMatches();
-  PasswordForm::Scheme observed_form_scheme = GetScheme();
-  base::EraseIf(result, [observed_form_scheme](const auto* form) {
-    return form->scheme != observed_form_scheme;
-  });
-  return result;
-}
-
 void PasswordFormManager::SavePendingToStore(bool update) {
   const PasswordForm* saved_form = password_manager_util::GetMatchForUpdating(
       *parsed_submitted_form_, best_matches_);
@@ -1152,12 +1143,15 @@ void PasswordFormManager::SavePendingToStore(bool update) {
   base::string16 old_password =
       saved_form ? saved_form->password_value : base::string16();
   if (HasGeneratedPassword()) {
-    generation_state_->CommitGeneratedPassword(pending_credentials_,
-                                               GetAllMatches(), old_password);
+    generation_state_->CommitGeneratedPassword(
+        pending_credentials_, form_fetcher_->GetAllRelevantMatches(),
+        old_password);
   } else if (update) {
-    form_saver_->Update(pending_credentials_, GetAllMatches(), old_password);
+    form_saver_->Update(pending_credentials_,
+                        form_fetcher_->GetAllRelevantMatches(), old_password);
   } else {
-    form_saver_->Save(pending_credentials_, GetAllMatches(), old_password);
+    form_saver_->Save(pending_credentials_,
+                      form_fetcher_->GetAllRelevantMatches(), old_password);
   }
 }
 

components/password_manager/core/browser/password_form_manager.h

@@ -287,9 +287,6 @@ class PasswordFormManager : public PasswordFormManagerInterface,
   void CalculateFillingAssistanceMetric(
       const autofill::FormData& submitted_form);
 
-  // Returns all the credentials for the origin.
-  std::vector<const autofill::PasswordForm*> GetAllMatches() const;
-
   // Save/update |pending_credentials_| to the password store.
   void SavePendingToStore(bool update);
 

components/password_manager/core/browser/password_manager_util.cc

@@ -228,37 +228,39 @@ base::StringPiece GetSignonRealmWithProtocolExcluded(const PasswordForm& form) {
 void FindBestMatches(
     const std::vector<const PasswordForm*>& non_federated_matches,
     PasswordForm::Scheme scheme,
+    std::vector<const autofill::PasswordForm*>* non_federated_same_scheme,
     std::map<base::string16, const PasswordForm*>* best_matches,
     const PasswordForm** preferred_match) {
   DCHECK(std::all_of(
       non_federated_matches.begin(), non_federated_matches.end(),
       [](const PasswordForm* match) { return !match->blacklisted_by_user; }));
+  DCHECK(non_federated_same_scheme);
   DCHECK(best_matches);
   DCHECK(preferred_match);
 
   *preferred_match = nullptr;
   best_matches->clear();
+  non_federated_same_scheme->clear();
 
-  std::vector<const PasswordForm*> same_scheme_non_federated_matches;
   for (auto* match : non_federated_matches) {
     if (match->scheme == scheme)
-      same_scheme_non_federated_matches.push_back(match);
+      non_federated_same_scheme->push_back(match);
   }
 
-  if (same_scheme_non_federated_matches.empty())
+  if (non_federated_same_scheme->empty())
     return;
 
   // Sort matches using IsBetterMatch predicate.
-  std::sort(same_scheme_non_federated_matches.begin(),
-            same_scheme_non_federated_matches.end(), IsBetterMatch);
-  for (const auto* match : same_scheme_non_federated_matches) {
+  std::sort(non_federated_same_scheme->begin(),
+            non_federated_same_scheme->end(), IsBetterMatch);
+  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));
   }
 
-  *preferred_match = *same_scheme_non_federated_matches.begin();
+  *preferred_match = *non_federated_same_scheme->begin();
 }
 
 const PasswordForm* GetMatchForUpdating(

components/password_manager/core/browser/password_manager_util.h

@@ -122,6 +122,7 @@ base::StringPiece GetSignonRealmWithProtocolExcluded(
 void FindBestMatches(
     const std::vector<const autofill::PasswordForm*>& non_federated_matches,
     autofill::PasswordForm::Scheme scheme,
+    std::vector<const autofill::PasswordForm*>* non_federated_same_scheme,
     std::map<base::string16, const autofill::PasswordForm*>* best_matches,
     const autofill::PasswordForm** preferred_match);
 

components/password_manager/core/browser/password_manager_util_unittest.cc

@@ -200,8 +200,9 @@ TEST(PasswordManagerUtil, FindBestMatches) {
     std::map<base::string16, const PasswordForm*> best_matches;
     const PasswordForm* preferred_match = nullptr;
 
-    FindBestMatches(matches, PasswordForm::Scheme::kHtml, &best_matches,
-                    &preferred_match);
+    std::vector<const PasswordForm*> same_scheme_matches;
+    FindBestMatches(matches, PasswordForm::Scheme::kHtml, &same_scheme_matches,
+                    &best_matches, &preferred_match);
 
     if (test_case.expected_preferred_match_index == kNotFound) {
       // Case of empty |matches|.