Limit number of drawn Autofill suggestions to 10.

This fixes the issue where Chrome would hang if the user had a lot (tested with 5000) profiles.

After debugging, the following bottlenecks were identified (listed in order of execution):
- Finding matching profiles (impact small),
- Deduping matching profiles to create a unique set (impact medium),
- Calculating width of every rows (impact high),
- Eliding strings (impact high)

The approach this CL is taking to solving the issue is to limit the number of profiles that are shown to the user. This is done gradually to optimize perf while keeping the same precision.
Effectively, in the worst case, we are limiting our search to:
- Find the first 50 matching profiles,
- Dedupe matching profiles until you have 10 unique to show.

This way we are optimizing where perf cost is high, while keeping our results relevant since users will get other results not shown by default as they keep typing.

Metrics considered for choosing the numbers:
50 matching profiles:
- Very few users have more than 3 profiles, but recorded extreme cases go up to 53 (see UMA chart link),
- Since perf cost for this part of the code is not high, we can accommodate these extreme cases.

10 suggestions:
- Over 99% of our users will choose a suggestion that is within the first 10 suggestions.

Fixes: 753481
Change-Id: I59a9529f9ba4cf6da56a1c1167aa6bb7562d0f81
Reviewed-on: https://chromium-review.googlesource.com/c/1343609Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Commit-Queue: Sebastien Lalancette <seblalancette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611089}

components/autofill/core/browser/BUILD.gn

@@ -172,6 +172,8 @@ jumbo_static_library("browser") {
     "subkey_requester.h",
     "suggestion.cc",
     "suggestion.h",
+    "suggestion_selection.cc",
+    "suggestion_selection.h",
     "ui/card_unmask_prompt_controller.h",
     "ui/card_unmask_prompt_controller_impl.cc",
     "ui/card_unmask_prompt_controller_impl.h",
@@ -510,6 +512,7 @@ source_set("unit_tests") {
     "search_field_unittest.cc",
     "strike_database_unittest.cc",
     "subkey_requester_unittest.cc",
+    "suggestion_selection_unittest.cc",
     "ui/card_unmask_prompt_controller_impl_unittest.cc",
     "validation_unittest.cc",
     "webdata/autocomplete_sync_bridge_unittest.cc",

components/autofill/core/browser/autofill_profile_comparator.h

@@ -113,6 +113,9 @@ class AutofillProfileComparator {
                       const AutofillProfile& p2,
                       Address* address) const;
 
+  // App locale used when this comparator instance was created.
+  const std::string app_locale() const { return app_locale_; }
+
  protected:
   // The result type returned by CompareTokens.
   enum CompareTokensResult {
@@ -215,8 +218,6 @@ class AutofillProfileComparator {
                      const AutofillProfile& p2,
                      NameInfo* info) const;
 
-  const std::string app_locale() const { return app_locale_; }
-
  private:
   l10n::CaseInsensitiveCompare case_insensitive_compare_;
   std::unique_ptr<icu::Transliterator> transliterator_;

components/autofill/core/browser/personal_data_manager.cc

@@ -37,6 +37,7 @@
 #include "components/autofill/core/browser/personal_data_manager_observer.h"
 #include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/phone_number_i18n.h"
+#include "components/autofill/core/browser/suggestion_selection.h"
 #include "components/autofill/core/browser/validation.h"
 #include "components/autofill/core/common/autofill_clock.h"
 #include "components/autofill/core/common/autofill_constants.h"
@@ -137,26 +138,6 @@ bool IsSyncEnabledFor(const syncer::SyncService* sync_service,
          sync_service->GetPreferredDataTypes().Has(model_type);
 }
 
-// In addition to just getting the values out of the autocomplete profile, this
-// function handles formatting of the street address into a single string.
-base::string16 GetInfoInOneLine(const AutofillProfile* profile,
-                                const AutofillType& type,
-                                const std::string app_locale) {
-  std::vector<base::string16> results;
-
-  AddressField address_field;
-  if (i18n::FieldForType(type.GetStorableType(), &address_field) &&
-      address_field == STREET_ADDRESS) {
-    std::string street_address_line;
-    GetStreetAddressLinesAsSingleLine(
-        *i18n::CreateAddressDataFromAutofillProfile(*profile, app_locale),
-        &street_address_line);
-    return base::UTF8ToUTF16(street_address_line);
-  }
-
-  return profile->GetInfo(type, app_locale);
-}
-
 // Receives the loaded profiles from the web data service and stores them in
 // |*dest|. The pending handle is the address of the pending handle
 // corresponding to this request type. This function is used to save both
@@ -180,52 +161,6 @@ static bool CompareVotes(const std::pair<std::string, int>& a,
   return a.second < b.second;
 }
 
-// Returns whether the |suggestion| is valid considering the
-// |field_contents_canon|, the |type| and |is_masked_server_card|. Assigns true
-// to |is_prefix_matched| if the |field_contents_canon| is a prefix to
-// |suggestion|, assigns false otherwise.
-bool IsValidSuggestionForFieldContents(base::string16 suggestion_canon,
-                                       base::string16 field_contents_canon,
-                                       const AutofillType& type,
-                                       bool is_masked_server_card,
-                                       bool* is_prefix_matched) {
-  *is_prefix_matched = true;
-
-  // Phones should do a substring match because they can be trimmed to remove
-  // the first parts (e.g. country code or prefix). It is still considered a
-  // prefix match in order to put it at the top of the suggestions.
-  if ((type.group() == PHONE_HOME || type.group() == PHONE_BILLING) &&
-      suggestion_canon.find(field_contents_canon) != base::string16::npos) {
-    return true;
-  }
-
-  // For card number fields, suggest the card if:
-  // - the number matches any part of the card, or
-  // - it's a masked card and there are 6 or fewer typed so far.
-  if (type.GetStorableType() == CREDIT_CARD_NUMBER) {
-    if (suggestion_canon.find(field_contents_canon) == base::string16::npos &&
-        (!is_masked_server_card || field_contents_canon.size() >= 6)) {
-      return false;
-    }
-    return true;
-  }
-
-  if (base::StartsWith(suggestion_canon, field_contents_canon,
-                       base::CompareCase::SENSITIVE)) {
-    return true;
-  }
-
-  if (IsFeatureSubstringMatchEnabled() &&
-      suggestion_canon.length() >= field_contents_canon.length() &&
-      GetTextSelectionStart(suggestion_canon, field_contents_canon, false) !=
-          base::string16::npos) {
-    *is_prefix_matched = false;
-    return true;
-  }
-
-  return false;
-}
-
 AutofillProfile CreateBasicTestAddress(const std::string& locale) {
   const base::Time use_date =
       AutofillClock::Now() - base::TimeDelta::FromDays(20);
@@ -1287,22 +1222,6 @@ std::vector<AutofillProfile*> PersonalDataManager::GetProfilesToSuggest()
   return profiles;
 }
 
-// static
-void PersonalDataManager::RemoveProfilesNotUsedSinceTimestamp(
-    base::Time min_last_used,
-    std::vector<AutofillProfile*>* profiles) {
-  const size_t original_size = profiles->size();
-  profiles->erase(
-      std::stable_partition(profiles->begin(), profiles->end(),
-                            [min_last_used](const AutofillDataModel* m) {
-                              return m->use_date() > min_last_used;
-                            }),
-      profiles->end());
-  const size_t num_profiles_supressed = original_size - profiles->size();
-  AutofillMetrics::LogNumberOfAddressesSuppressedForDisuse(
-      num_profiles_supressed);
-}
-
 // static
 void PersonalDataManager::MaybeRemoveInvalidSuggestions(
     const AutofillType& type,
@@ -1349,7 +1268,7 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
       comparator.NormalizeForComparison(field_contents);
 
   // Get the profiles to suggest, which are already sorted.
-  std::vector<AutofillProfile*> profiles = GetProfilesToSuggest();
+  std::vector<AutofillProfile*> sorted_profiles = GetProfilesToSuggest();
 
   // When suggesting with no prefix to match, consider suppressing disused
   // address suggestions as well as those based on invalid profile data.
@@ -1358,82 +1277,26 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
             features::kAutofillSuppressDisusedAddresses)) {
       const base::Time min_last_used =
           AutofillClock::Now() - kDisusedProfileTimeDelta;
-      RemoveProfilesNotUsedSinceTimestamp(min_last_used, &profiles);
+      suggestion_selection::RemoveProfilesNotUsedSinceTimestamp(
+          min_last_used, &sorted_profiles);
     }
     // We need the updated information on the validity states of the profiles.
-    UpdateProfilesValidityMapsIfNeeded(profiles);
-    MaybeRemoveInvalidSuggestions(type, &profiles);
+    UpdateProfilesValidityMapsIfNeeded(sorted_profiles);
+    MaybeRemoveInvalidSuggestions(type, &sorted_profiles);
   }
 
-  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;
-    base::string16 suggestion_canon = comparator.NormalizeForComparison(value);
-    if (IsValidSuggestionForFieldContents(
-            suggestion_canon, field_contents_canon, type,
-            /* is_masked_server_card= */ false, &prefix_matched_suggestion)) {
-      matched_profiles.push_back(profile);
-      suggestions.push_back(Suggestion(value));
-      suggestions.back().backend_id = profile->guid();
-      suggestions.back().match = prefix_matched_suggestion
-                                     ? Suggestion::PREFIX_MATCH
-                                     : Suggestion::SUBSTRING_MATCH;
-    }
-  }
-
-  // Prefix matches should precede other token matches.
-  if (IsFeatureSubstringMatchEnabled()) {
-    std::stable_sort(suggestions.begin(), suggestions.end(),
-                     [](const Suggestion& a, const Suggestion& b) {
-                       return a.match < b.match;
-                     });
-  }
+  std::vector<Suggestion> suggestions =
+      suggestion_selection::GetPrefixMatchedSuggestions(
+          type, field_contents_canon, comparator, sorted_profiles,
+          &matched_profiles);
 
   // Don't show two suggestions if one is a subset of the other.
   std::vector<AutofillProfile*> unique_matched_profiles;
-  std::vector<Suggestion> unique_suggestions;
-  // If there are many profiles, subset checking will take a long time(easily
-  // seconds). We will only do this if the profiles count is reasonable.
-  if (matched_profiles.size() <= 15) {
-    ServerFieldTypeSet types(other_field_types.begin(),
-                             other_field_types.end());
-    for (size_t i = 0; i < matched_profiles.size(); ++i) {
-      bool include = true;
-      AutofillProfile* profile_a = matched_profiles[i];
-      for (size_t j = 0; j < matched_profiles.size(); ++j) {
-        AutofillProfile* profile_b = matched_profiles[j];
-        // Check if profile A is a subset of profile B. If not, continue.
-        if (i == j || suggestions[i].value != suggestions[j].value ||
-            !profile_a->IsSubsetOfForFieldSet(*profile_b, app_locale_, types)) {
-          continue;
-        }
-
-        // Check if profile B is also a subset of profile A. If so, the
-        // profiles are identical. Include the first one but not the second.
-        if (i < j &&
-            profile_b->IsSubsetOfForFieldSet(*profile_a, app_locale_, types)) {
-          continue;
-        }
-
-        // One-way subset. Don't include profile A.
-        include = false;
-        break;
-      }
-      if (include) {
-        unique_matched_profiles.push_back(matched_profiles[i]);
-        unique_suggestions.push_back(suggestions[i]);
-      }
-    }
-  } else {
-    unique_matched_profiles = matched_profiles;
-    unique_suggestions = suggestions;
-  }
+  std::vector<Suggestion> unique_suggestions =
+      suggestion_selection::GetUniqueSuggestions(other_field_types, app_locale_,
+                                                 matched_profiles, suggestions,
+                                                 &unique_matched_profiles);
 
   // Generate disambiguating labels based on the list of matches.
   std::vector<base::string16> labels;
@@ -2188,7 +2051,7 @@ std::vector<Suggestion> PersonalDataManager::GetSuggestionsForCards(
         base::i18n::ToLower(creditcard_field_value);
 
     bool prefix_matched_suggestion;
-    if (IsValidSuggestionForFieldContents(
+    if (suggestion_selection::IsValidSuggestionForFieldContents(
             creditcard_field_lower, field_contents_lower, type,
             credit_card->record_type() == CreditCard::MASKED_SERVER_CARD,
             &prefix_matched_suggestion)) {

components/autofill/core/browser/personal_data_manager.h

@@ -268,12 +268,6 @@ class PersonalDataManager : public KeyedService,
   // Returns the profiles to suggest to the user, ordered by frecency.
   std::vector<AutofillProfile*> GetProfilesToSuggest() const;
 
-  // Remove profiles that haven't been used after |min_last_used| from
-  // |profiles|. The relative ordering of |profiles| is maintained.
-  static void RemoveProfilesNotUsedSinceTimestamp(
-      base::Time min_last_used,
-      std::vector<AutofillProfile*>* profiles);
-
   // Remove profiles that whose |type| field is flagged as invalid, if Chrome
   // is configured to not make suggestions based on invalid data.
   static void MaybeRemoveInvalidSuggestions(

components/autofill/core/browser/suggestion_selection.cc

+// Copyright 2018 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/suggestion_selection.h"
+
+#include <string>
+#include <vector>
+
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
+#include "components/autofill/core/browser/address_i18n.h"
+#include "components/autofill/core/browser/autofill_metrics.h"
+#include "components/autofill/core/browser/autofill_profile.h"
+#include "components/autofill/core/browser/autofill_profile_comparator.h"
+#include "components/autofill/core/browser/suggestion.h"
+#include "components/autofill/core/common/autofill_clock.h"
+#include "components/autofill/core/common/autofill_features.h"
+#include "components/autofill/core/common/autofill_util.h"
+#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
+#include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_formatter.h"
+
+namespace autofill {
+namespace suggestion_selection {
+
+namespace {
+using ::i18n::addressinput::AddressField;
+using ::i18n::addressinput::GetStreetAddressLinesAsSingleLine;
+using ::i18n::addressinput::STREET_ADDRESS;
+
+// In addition to just getting the values out of the autocomplete profile, this
+// function handles formatting of the street address into a single string.
+base::string16 GetInfoInOneLine(const AutofillProfile* profile,
+                                const AutofillType& type,
+                                const std::string& app_locale) {
+  std::vector<base::string16> results;
+
+  AddressField address_field;
+  if (i18n::FieldForType(type.GetStorableType(), &address_field) &&
+      address_field == STREET_ADDRESS) {
+    std::string street_address_line;
+    GetStreetAddressLinesAsSingleLine(
+        *i18n::CreateAddressDataFromAutofillProfile(*profile, app_locale),
+        &street_address_line);
+    return base::UTF8ToUTF16(street_address_line);
+  }
+
+  return profile->GetInfo(type, app_locale);
+}
+}  // namespace
+
+// As of November 2018, 50 profiles should be more than enough to cover at least
+// 99% of all times the dropdown is shown.
+extern const size_t kMaxSuggestedProfilesCount = 50;
+
+// As of November 2018, displaying 10 suggestions cover at least 99% of the
+// indices clicked by our users. The suggestions will also refine as they type.
+extern const size_t kMaxUniqueSuggestionsCount = 10;
+
+std::vector<Suggestion> GetPrefixMatchedSuggestions(
+    const AutofillType& type,
+    const base::string16& field_contents_canon,
+    const AutofillProfileComparator& comparator,
+    const std::vector<AutofillProfile*>& profiles,
+    std::vector<AutofillProfile*>* matched_profiles) {
+  std::vector<Suggestion> suggestions;
+
+  for (size_t i = 0; i < profiles.size() &&
+                     matched_profiles->size() < kMaxSuggestedProfilesCount;
+       i++) {
+    AutofillProfile* profile = profiles[i];
+    base::string16 value =
+        GetInfoInOneLine(profile, type, comparator.app_locale());
+    if (value.empty())
+      continue;
+
+    bool prefix_matched_suggestion;
+    base::string16 suggestion_canon = comparator.NormalizeForComparison(value);
+    if (IsValidSuggestionForFieldContents(
+            suggestion_canon, field_contents_canon, type,
+            /* is_masked_server_card= */ false, &prefix_matched_suggestion)) {
+      matched_profiles->push_back(profile);
+      suggestions.push_back(Suggestion(value));
+      suggestions.back().backend_id = profile->guid();
+      suggestions.back().match = prefix_matched_suggestion
+                                     ? Suggestion::PREFIX_MATCH
+                                     : Suggestion::SUBSTRING_MATCH;
+    }
+  }
+
+  // Prefix matches should precede other token matches.
+  if (IsFeatureSubstringMatchEnabled()) {
+    std::stable_sort(suggestions.begin(), suggestions.end(),
+                     [](const Suggestion& a, const Suggestion& b) {
+                       return a.match < b.match;
+                     });
+  }
+
+  return suggestions;
+}
+
+std::vector<Suggestion> GetUniqueSuggestions(
+    const std::vector<ServerFieldType>& other_field_types,
+    const std::string app_locale,
+    const std::vector<AutofillProfile*> matched_profiles,
+    const std::vector<Suggestion>& suggestions,
+    std::vector<AutofillProfile*>* unique_matched_profiles) {
+  std::vector<Suggestion> unique_suggestions;
+
+  // Limit number of unique profiles as having too many makes the browser hang
+  // due to drawing calculations (and is also not very useful for the user).
+  ServerFieldTypeSet types(other_field_types.begin(), other_field_types.end());
+  for (size_t i = 0; i < matched_profiles.size() &&
+                     unique_suggestions.size() < kMaxUniqueSuggestionsCount;
+       ++i) {
+    bool include = true;
+    AutofillProfile* profile_a = matched_profiles[i];
+    for (size_t j = 0; j < matched_profiles.size(); ++j) {
+      AutofillProfile* profile_b = matched_profiles[j];
+      // Check if profile A is a subset of profile B. If not, continue.
+      if (i == j || suggestions[i].value != suggestions[j].value ||
+          !profile_a->IsSubsetOfForFieldSet(*profile_b, app_locale, types)) {
+        continue;
+      }
+
+      // Check if profile B is also a subset of profile A. If so, the
+      // profiles are identical. Include the first one but not the second.
+      if (i < j &&
+          profile_b->IsSubsetOfForFieldSet(*profile_a, app_locale, types)) {
+        continue;
+      }
+
+      // One-way subset. Don't include profile A.
+      include = false;
+      break;
+    }
+    if (include) {
+      unique_matched_profiles->push_back(matched_profiles[i]);
+      unique_suggestions.push_back(suggestions[i]);
+    }
+  }
+
+  return unique_suggestions;
+}
+
+bool IsValidSuggestionForFieldContents(base::string16 suggestion_canon,
+                                       base::string16 field_contents_canon,
+                                       const AutofillType& type,
+                                       bool is_masked_server_card,
+                                       bool* is_prefix_matched) {
+  *is_prefix_matched = true;
+
+  // Phones should do a substring match because they can be trimmed to remove
+  // the first parts (e.g. country code or prefix). It is still considered a
+  // prefix match in order to put it at the top of the suggestions.
+  if ((type.group() == PHONE_HOME || type.group() == PHONE_BILLING) &&
+      suggestion_canon.find(field_contents_canon) != base::string16::npos) {
+    return true;
+  }
+
+  // For card number fields, suggest the card if:
+  // - the number matches any part of the card, or
+  // - it's a masked card and there are 6 or fewer typed so far.
+  if (type.GetStorableType() == CREDIT_CARD_NUMBER) {
+    if (suggestion_canon.find(field_contents_canon) == base::string16::npos &&
+        (!is_masked_server_card || field_contents_canon.size() >= 6)) {
+      return false;
+    }
+    return true;
+  }
+
+  if (base::StartsWith(suggestion_canon, field_contents_canon,
+                       base::CompareCase::SENSITIVE)) {
+    return true;
+  }
+
+  if (IsFeatureSubstringMatchEnabled() &&
+      suggestion_canon.length() >= field_contents_canon.length() &&
+      GetTextSelectionStart(suggestion_canon, field_contents_canon, false) !=
+          base::string16::npos) {
+    *is_prefix_matched = false;
+    return true;
+  }
+
+  return false;
+}
+
+void RemoveProfilesNotUsedSinceTimestamp(
+    base::Time min_last_used,
+    std::vector<AutofillProfile*>* profiles) {
+  const size_t original_size = profiles->size();
+  profiles->erase(
+      std::stable_partition(profiles->begin(), profiles->end(),
+                            [min_last_used](const AutofillDataModel* m) {
+                              return m->use_date() > min_last_used;
+                            }),
+      profiles->end());
+  const size_t num_profiles_supressed = original_size - profiles->size();
+  AutofillMetrics::LogNumberOfAddressesSuppressedForDisuse(
+      num_profiles_supressed);
+}
+
+}  // namespace suggestion_selection
+}  // namespace autofill
\ No newline at end of file

components/autofill/core/browser/suggestion_selection.h

+// Copyright 2018 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.
+#ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_SUGGESTION_SELECTION_H_
+#define COMPONENTS_AUTOFILL_CORE_BROWSER_SUGGESTION_SELECTION_H_
+
+#include <vector>
+
+#include "components/autofill/core/browser/field_types.h"
+#include "components/autofill/core/browser/suggestion.h"
+
+namespace autofill {
+
+class AutofillProfile;
+class AutofillProfileComparator;
+class AutofillType;
+
+namespace suggestion_selection {
+
+extern const size_t kMaxSuggestedProfilesCount;
+extern const size_t kMaxUniqueSuggestionsCount;
+
+// Matches based on prefix search, and limits number of profiles.
+// Returns the top matching suggestions based on prefix search, and adds the
+// corresponding profiles to |matched_profiles|. At most
+// |kMaxSuggestedProfilesCount| are returned.
+std::vector<Suggestion> GetPrefixMatchedSuggestions(
+    const AutofillType& type,
+    const base::string16& field_contents_canon,
+    const AutofillProfileComparator& comparator,
+    const std::vector<AutofillProfile*>& profiles,
+    std::vector<AutofillProfile*>* matched_profiles);
+
+// Dedupes given suggestions based on if one is a subset of the other.
+// Returns unique_suggestions, and adds the corresponding profiles to
+// |unique_matched_profiles|. At most |kMaxUniqueSuggestionsCount| are returned.
+std::vector<Suggestion> GetUniqueSuggestions(
+    const std::vector<ServerFieldType>& other_field_types,
+    const std::string app_locale,
+    const std::vector<AutofillProfile*> matched_profiles,
+    const std::vector<Suggestion>& suggestions,
+    std::vector<AutofillProfile*>* unique_matched_profiles);
+
+// Returns whether the |suggestion_canon| is valid considering the
+// |field_contents_canon|, the |type| and |is_masked_server_card|. Assigns true
+// to |is_prefix_matched| if the |field_contents_canon| is a prefix to
+// |suggestion_canon|, assigns false otherwise.
+bool IsValidSuggestionForFieldContents(base::string16 suggestion_canon,
+                                       base::string16 field_contents_canon,
+                                       const AutofillType& type,
+                                       bool is_masked_server_card,
+                                       bool* is_prefix_matched);
+
+// Removes profiles that haven't been used after |min_last_used| from
+// |profiles|. The relative ordering of |profiles| is maintained.
+void RemoveProfilesNotUsedSinceTimestamp(
+    base::Time min_last_used,
+    std::vector<AutofillProfile*>* profiles);
+
+}  // namespace suggestion_selection
+}  // namespace autofill
+
+#endif  // COMPONENTS_AUTOFILL_CORE_BROWSER_SUGGESTION_SELECTION_H_