[Autofill] Sort profiles and credit cards by frecency in PaymentRequest.

components/autofill/core/browser/personal_data_manager.cc

Index: components/autofill/core/browser/personal_data_manager.cc
diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc
index 564f23e74a5e38938a7ed23e0d3f10982a38b442..39e084ca978d8ceeebee1c6a174c8782fc58171c 100644
--- a/components/autofill/core/browser/personal_data_manager.cc
+++ b/components/autofill/core/browser/personal_data_manager.cc
@@ -686,6 +686,21 @@ void PersonalDataManager::Refresh() {
   LoadCreditCards();
 }
 
+const std::vector<AutofillProfile*> PersonalDataManager::GetProfilesToSuggest()
+    const {
+  std::vector<AutofillProfile*> profiles = GetProfiles(true);
+
+  // Rank the suggestions by frecency (see AutofillDataModel for details).
+  base::Time comparison_time = base::Time::Now();
+  std::sort(profiles.begin(), profiles.end(),
+            [comparison_time](const AutofillDataModel* a,
+                              const AutofillDataModel* b) {
+              return a->CompareFrecency(b, comparison_time);
+            });
+
+  return profiles;
+}
+
 std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
     const AutofillType& type,
     const base::string16& field_contents,
@@ -697,15 +712,8 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
   base::string16 field_contents_canon =
       AutofillProfile::CanonicalizeProfileString(field_contents);
 
-  std::vector<AutofillProfile*> profiles = GetProfiles(true);
-
-  // Rank the suggestions by frecency (see AutofillDataModel for details).
-  base::Time comparison_time = base::Time::Now();
-  std::sort(profiles.begin(), profiles.end(),
-            [comparison_time](const AutofillDataModel* a,
-                              const AutofillDataModel* b) {
-              return a->CompareFrecency(b, comparison_time);
-            });
+  // Get the profiles to suggest, which are already sorted.
+  std::vector<AutofillProfile*> profiles = GetProfilesToSuggest();
 
   std::vector<Suggestion> suggestions;
   // Match based on a prefix search.
@@ -790,31 +798,44 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
   return unique_suggestions;
 }
 
-std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
-    const AutofillType& type,
-    const base::string16& field_contents) {
-  if (IsInAutofillSuggestionsDisabledExperiment())
-    return std::vector<Suggestion>();
+// TODO(crbug.com/613187): Investigate if it would be more efficient to dedupe
+// with a vector instead of a list.
+const std::vector<CreditCard*> PersonalDataManager::GetCreditCardsToSuggest()
+    const {
+  std::vector<CreditCard*> credit_cards = GetCreditCards();
 
-  const std::vector<CreditCard*> credit_cards = GetCreditCards();
-  std::list<const CreditCard*> cards_to_suggest(credit_cards.begin(),
-                                                credit_cards.end());
+  std::list<CreditCard*> cards_to_dedupe(credit_cards.begin(),
+                                         credit_cards.end());
 
-  DedupeCreditCardToSuggest(&cards_to_suggest);
+  DedupeCreditCardToSuggest(&cards_to_dedupe);

[please use gerrit instead, 2016/05/24 20:21:48]

Can you check that this code path is not being used in for displaying cards in
chrome://settings/autofill and in the Android equivalent? Deduplication is good
for suggestions, but should not be happening in settings. Settings should
reflect exactly what's no disk, so the user can manager their own data.

For example, PersonalDataManager::getProfiles() (in Java) is used by Android
Autofill settings. We would not want deduplication there, if it was being used
for settings.

If you find that settings is performing deduping, then we would need two
separate code paths: one for settings without dedupe and one for
PaymentRequest/autofill with dedupe.

[sebsg, 2016/05/24 21:41:22]

The options page uses GetProfiles and GetCreditCards directly. These calls just
get the data without any processing. Dedupe was only called in
GetCreditCardSuggestions and now in GetCreditCardsToSuggest. 

When you get back we could also check if you could use our suggestions directly.

[please use gerrit instead, 2016/05/25 00:54:41]

What about getProfiles() and getCreditCards() in
https://codereview.chromium.org/1982623002/diff/220001/chrome/android/java/sr...
? That seems to be going through your new code path with de-dupe. I would
suggest that PersonalDataManager.java should have getProfilesForSettings(),
getProfilesToSuggest(), getCreditCardsForSettings(), and
getCreditCardsToSuggest(). The *ForSettings() variants should not have
deduplication. The *ToSuggest() variants should have deduplication and frecency
ordering. 

+
+  std::vector<CreditCard*> cards_to_suggest(
+      std::make_move_iterator(std::begin(cards_to_dedupe)),
+      std::make_move_iterator(std::end(cards_to_dedupe)));
 
   // Rank the cards by frecency (see AutofillDataModel for details). All expired
   // cards should be suggested last, also by frecency.
   base::Time comparison_time = base::Time::Now();
-  cards_to_suggest.sort(
-      [comparison_time](const CreditCard* a, const CreditCard* b) {
-        bool a_is_expired = a->IsExpired(comparison_time);
-        if (a_is_expired != b->IsExpired(comparison_time))
-          return !a_is_expired;
+  std::stable_sort(cards_to_suggest.begin(), cards_to_suggest.end(),
+                   [comparison_time](const CreditCard* a, const CreditCard* b) {
+                     bool a_is_expired = a->IsExpired(comparison_time);
+                     if (a_is_expired != b->IsExpired(comparison_time))
+                       return !a_is_expired;
+
+                     return a->CompareFrecency(b, comparison_time);
+                   });
 
-        return a->CompareFrecency(b, comparison_time);
-      });
+  return cards_to_suggest;
+}
+
+std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
+    const AutofillType& type,
+    const base::string16& field_contents) {
+  if (IsInAutofillSuggestionsDisabledExperiment())
+    return std::vector<Suggestion>();
 
-  return GetSuggestionsForCards(type, field_contents, cards_to_suggest);
+  return GetSuggestionsForCards(type, field_contents,
+                                GetCreditCardsToSuggest());
 }
 
 bool PersonalDataManager::IsAutofillEnabled() const {
@@ -964,7 +985,7 @@ const std::string& PersonalDataManager::GetDefaultCountryCodeForNewAddress()
 
 // static
 void PersonalDataManager::DedupeCreditCardToSuggest(
-    std::list<const CreditCard*>* cards_to_suggest) {
+    std::list<CreditCard*>* cards_to_suggest) {
   for (auto outer_it = cards_to_suggest->begin();
        outer_it != cards_to_suggest->end(); ++outer_it) {
     // If considering a full server card, look for local cards that are
@@ -1448,9 +1469,8 @@ const std::vector<AutofillProfile*>& PersonalDataManager::GetProfiles(
 std::vector<Suggestion> PersonalDataManager::GetSuggestionsForCards(
     const AutofillType& type,
     const base::string16& field_contents,
-    const std::list<const CreditCard*>& cards_to_suggest) const {
+    const std::vector<CreditCard*>& cards_to_suggest) const {
   std::vector<Suggestion> suggestions;
-  std::list<const CreditCard*> substring_matched_cards;
   base::string16 field_contents_lower = base::i18n::ToLower(field_contents);
   for (const CreditCard* credit_card : cards_to_suggest) {
     // The value of the stored data for this field type in the |credit_card|.