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

components/autofill/core/browser/personal_data_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/personal_data_manager.h"
 
 #include <stddef.h>
 #include <algorithm>
 #include <list>
 #include <map>
@@ skipping 668 matching lines @@
 
 bool PersonalDataManager::HasServerData() const {
   return !server_credit_cards_.empty() || !server_profiles_.empty();
 }
 
 void PersonalDataManager::Refresh() {
   LoadProfiles();
   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,
     bool field_is_autofilled,
     const std::vector<ServerFieldType>& other_field_types) {
   if (IsInAutofillSuggestionsDisabledExperiment())
     return std::vector<Suggestion>();
 
   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.
   std::vector<AutofillProfile*> matched_profiles;
   for (AutofillProfile* profile : profiles) {
     base::string16 value = GetInfoInOneLine(profile, type, app_locale_);
     if (value.empty())
       continue;
 
     bool prefix_matched_suggestion;
@@ skipping 64 matching lines @@
   // trial group or SIZE_MAX if no limit is defined.
   std::string limit_str = variations::GetVariationParamValue(
       kFrecencyFieldTrialName, kFrecencyFieldTrialLimitParam);
   size_t limit = base::StringToSizeT(limit_str, &limit) ? limit : SIZE_MAX;
 
   unique_suggestions.resize(std::min(unique_suggestions.size(), limit));
 
   return unique_suggestions;
 }
 
+// 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();
+
+  std::list<CreditCard*> cards_to_dedupe(credit_cards.begin(),
+                                         credit_cards.end());
+
+  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();
+  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 cards_to_suggest;
+}
+
 std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
     const AutofillType& type,
     const base::string16& field_contents) {
   if (IsInAutofillSuggestionsDisabledExperiment())
     return std::vector<Suggestion>();
 
-  const std::vector<CreditCard*> credit_cards = GetCreditCards();
-  std::list<const CreditCard*> cards_to_suggest(credit_cards.begin(),
-                                                credit_cards.end());
-
-  DedupeCreditCardToSuggest(&cards_to_suggest);
-
-  // 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;
-
-        return a->CompareFrecency(b, comparison_time);
-      });
-
-  return GetSuggestionsForCards(type, field_contents, cards_to_suggest);
+  return GetSuggestionsForCards(type, field_contents,
+                                GetCreditCardsToSuggest());
 }
 
 bool PersonalDataManager::IsAutofillEnabled() const {
   return ::autofill::IsAutofillEnabled(pref_service_);
 }
 
 std::string PersonalDataManager::CountryCodeForCurrentTimezone() const {
   return base::CountryCodeForCurrentTimezone();
 }
 
@@ skipping 129 matching lines @@
 
   // Failing that, guess based on locale.
   if (default_country_code_.empty())
     default_country_code_ = AutofillCountry::CountryCodeForLocale(app_locale());
 
   return default_country_code_;
 }
 
 // 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
     // duplicates of it and remove them.
     if ((*outer_it)->record_type() == CreditCard::FULL_SERVER_CARD) {
       for (auto inner_it = cards_to_suggest->begin();
            inner_it != cards_to_suggest->end();) {
         auto inner_it_copy = inner_it++;
         if ((*inner_it_copy)->IsLocalDuplicateOfServerCard(**outer_it))
           cards_to_suggest->erase(inner_it_copy);
@@ skipping 463 matching lines @@
   if (pref_service_->GetBoolean(prefs::kAutofillWalletImportEnabled)) {
     profiles_.insert(
         profiles_.end(), server_profiles_.begin(), server_profiles_.end());
   }
   return profiles_;
 }
 
 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|.
     base::string16 creditcard_field_value =
         credit_card->GetInfo(type, app_locale_);
     if (creditcard_field_value.empty())
       continue;
     base::string16 creditcard_field_lower =
         base::i18n::ToLower(creditcard_field_value);
 
@@ skipping 44 matching lines @@
     std::stable_sort(suggestions.begin(), suggestions.end(),
                      [](const Suggestion& a, const Suggestion& b) {
                        return a.match < b.match;
                      });
   }
 
   return suggestions;
 }
 
 }  // namespace autofill