[Autofill] Discards duplicate suggestions.

Bug: 966830
Change-Id: I738b691847869439dd38df6d6fa7595e87af9688
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628042
Commit-Queue: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664010}

components/autofill/core/browser/personal_data_manager.cc

@@ -1189,8 +1189,6 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
                                           type.GetStorableType(), 1,
                                           app_locale_, &labels);
   }
-  suggestion_selection::PrepareSuggestions(formatter != nullptr, labels,
-                                           &unique_suggestions);
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   if (use_improved_label_disambiguation && !unique_suggestions.empty()) {
@@ -1199,6 +1197,9 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
   }
 #endif  // #if !defined(OS_ANDROID) && !defined(OS_IOS)
 
+  suggestion_selection::PrepareSuggestions(formatter != nullptr, labels,
+                                           &unique_suggestions, comparator);
+
   return unique_suggestions;
 }
 

components/autofill/core/browser/ui/suggestion_selection.cc

@@ -4,6 +4,7 @@
 #include "components/autofill/core/browser/ui/suggestion_selection.h"
 
 #include <string>
+#include <unordered_set>
 #include <vector>
 
 #include "base/strings/string_util.h"
@@ -247,18 +248,56 @@ void RemoveProfilesNotUsedSinceTimestamp(
 
 void PrepareSuggestions(bool add_profile_icon,
                         const std::vector<base::string16>& labels,
-                        std::vector<Suggestion>* suggestions) {
+                        std::vector<Suggestion>* suggestions,
+                        const AutofillProfileComparator& comparator) {
   DCHECK_EQ(suggestions->size(), labels.size());
 
+  // This set is used to determine whether duplicate Suggestions exist. For
+  // example, a Suggestion with the value "John" and the label "400 Oak Rd" has
+  // the normalized text "john400oakrd". This text can only be added to the set
+  // once.
+  std::unordered_set<base::string16> suggestion_text;
+  size_t index_to_add_suggestion = 0;
+
+  // Dedupes Suggestions to show in the dropdown once values and labels have
+  // been created. This is useful when LabelFormatters make Suggestions' labels.
+  //
+  // Suppose profile A has the data John, 400 Oak Rd, and (617) 544-7411 and
+  // profile B has the data John, 400 Oak Rd, (508) 957-5009. If a formatter
+  // puts only 400 Oak Rd in the label, then there will be two Suggestions with
+  // the normalized text "john400oakrd", and the Suggestion with the lower
+  // ranking should be discarded.
   for (size_t i = 0; i < labels.size(); ++i) {
-    (*suggestions)[i].additional_label = labels[i];
-    (*suggestions)[i].label = labels[i];
+    base::string16 label = labels[i];
+
+    bool text_inserted =
+        suggestion_text
+            .insert(comparator.NormalizeForComparison(
+                (*suggestions)[i].value + label,
+                autofill::AutofillProfileComparator::DISCARD_WHITESPACE))
+            .second;
+
+    if (text_inserted) {
+      if (index_to_add_suggestion != i) {
+        (*suggestions)[index_to_add_suggestion] = (*suggestions)[i];
+      }
+      // The given |suggestions| are already sorted from highest to lowest
+      // ranking. Suggestions with lower indices have a higher ranking and
+      // should be kept.
+      (*suggestions)[index_to_add_suggestion].additional_label = labels[i];
+      (*suggestions)[index_to_add_suggestion].label = labels[i];
 
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
-    if (add_profile_icon) {
-      (*suggestions)[i].icon = "accountBoxIcon";
-    }
+      if (add_profile_icon) {
+        (*suggestions)[index_to_add_suggestion].icon = "accountBoxIcon";
+      }
 #endif  // !defined(OS_ANDROID) && !defined(OS_IOS)
+      ++index_to_add_suggestion;
+    }
+  }
+
+  if (index_to_add_suggestion < suggestions->size()) {
+    suggestions->resize(index_to_add_suggestion);
   }
 }
 

components/autofill/core/browser/ui/suggestion_selection.h

@@ -65,12 +65,16 @@ void RemoveProfilesNotUsedSinceTimestamp(
     std::vector<AutofillProfile*>* profiles);
 
 // Prepares a collection of Suggestions to show to the user. Adds |labels| to
-// their corresponding |suggestions|. A label corresponds to the suggestion with
-// the same index. Adds an icon on desktop platforms when |add_profile_icon| is
-// true.
+// their corresponding |suggestions| and removes duplicates, if any. A label
+// corresponds to the suggestion with the same index. Adds an icon on desktop
+// platforms when |add_profile_icon| is true.
+//
+// NOTE: |suggestions| are assumed to have already been sorted from most to
+// least important.
 void PrepareSuggestions(bool add_profile_icon,
                         const std::vector<base::string16>& labels,
-                        std::vector<Suggestion>* suggestions);
+                        std::vector<Suggestion>* suggestions,
+                        const AutofillProfileComparator& comparator);
 
 }  // namespace suggestion_selection
 }  // namespace autofill

components/autofill/core/browser/ui/suggestion_selection_unittest.cc

@@ -519,5 +519,62 @@ TEST_F(SuggestionSelectionTest, RemoveProfilesNotUsedSinceTimestamp) {
   }
 }
 
+TEST_F(SuggestionSelectionTest,
+       PrepareSuggestions_DiscardDuplicateSuggestions) {
+  std::vector<Suggestion> suggestions{
+      Suggestion(base::ASCIIToUTF16("Jon Snow")),
+      Suggestion(base::ASCIIToUTF16("Jon Snow")),
+      Suggestion(base::ASCIIToUTF16("Jon Snow")),
+      Suggestion(base::ASCIIToUTF16("Jon Snow"))};
+
+  const std::vector<base::string16> labels{
+      base::ASCIIToUTF16("2 Beyond-the-Wall Rd"),
+      base::ASCIIToUTF16("1 Winterfell Ln"),
+      base::ASCIIToUTF16("2 Beyond-the-Wall Rd"),
+      base::ASCIIToUTF16("2 Beyond-the-Wall Rd.")};
+
+  PrepareSuggestions(/*add_profile_icon=*/false, labels, &suggestions,
+                     comparator_);
+
+  // Suggestions are sorted from highest to lowest rank, so check that
+  // duplicates with a lower rank are removed.
+  EXPECT_THAT(
+      suggestions,
+      ElementsAre(
+          AllOf(Field(&Suggestion::value, base::ASCIIToUTF16("Jon Snow")),
+                Field(&Suggestion::label,
+                      base::ASCIIToUTF16("2 Beyond-the-Wall Rd"))),
+          AllOf(Field(&Suggestion::value, base::ASCIIToUTF16("Jon Snow")),
+                Field(&Suggestion::label,
+                      base::ASCIIToUTF16("1 Winterfell Ln")))));
+}
+
+TEST_F(SuggestionSelectionTest,
+       PrepareSuggestions_KeepNonDuplicateSuggestions) {
+  std::vector<Suggestion> suggestions{
+      Suggestion(base::ASCIIToUTF16("Sansa")),
+      Suggestion(base::ASCIIToUTF16("Sansa")),
+      Suggestion(base::ASCIIToUTF16("Brienne"))};
+
+  const std::vector<base::string16> labels{
+      base::ASCIIToUTF16("1 Winterfell Ln"), base::ASCIIToUTF16(""),
+      base::ASCIIToUTF16("1 Winterfell Ln")};
+
+  PrepareSuggestions(/*add_profile_icon=*/false, labels, &suggestions,
+                     comparator_);
+
+  EXPECT_THAT(
+      suggestions,
+      ElementsAre(
+          AllOf(
+              Field(&Suggestion::value, base::ASCIIToUTF16("Sansa")),
+              Field(&Suggestion::label, base::ASCIIToUTF16("1 Winterfell Ln"))),
+          AllOf(Field(&Suggestion::value, base::ASCIIToUTF16("Sansa")),
+                Field(&Suggestion::label, base::ASCIIToUTF16(""))),
+          AllOf(Field(&Suggestion::value, base::ASCIIToUTF16("Brienne")),
+                Field(&Suggestion::label,
+                      base::ASCIIToUTF16("1 Winterfell Ln")))));
+}
+
 }  // namespace suggestion_selection
 }  // namespace autofill