[Autofill] Deduplicate Autocomplete and Datalist suggestions, keeping…

… the latter

Doesn't make sense to have both if they are the same value, as described in the bug.

BUG=419239
TEST=AutofillExternalDelegateTest

Review URL: https://codereview.chromium.org/1458903003

Cr-Commit-Position: refs/heads/master@{#360627}

components/autofill/core/browser/autofill_external_delegate.cc

@@ -10,6 +10,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
+#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autocomplete_history_manager.h"
@@ -309,9 +310,22 @@ void AutofillExternalDelegate::InsertDataListValues(
   if (data_list_values_.empty())
     return;
 
+  // Go through the list of autocomplete values and remove them if they are in
+  // the list of datalist values.
+  std::set<base::string16> data_list_set(data_list_values_.begin(),
+                                         data_list_values_.end());
+  suggestions->erase(
+      std::remove_if(suggestions->begin(), suggestions->end(),
+                     [&data_list_set](const Suggestion& suggestion) {
+                       return suggestion.frontend_id ==
+                                  POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY &&
+                              ContainsKey(data_list_set, suggestion.value);
+                     }),
+      suggestions->end());
+
 #if !defined(OS_ANDROID)
-  // Insert the separator between the datalist and Autofill values (if there
-  // are any).
+  // Insert the separator between the datalist and Autofill/Autocomplete values
+  // (if there are any).
   if (!suggestions->empty()) {
     suggestions->insert(suggestions->begin(), Suggestion());
     (*suggestions)[0].frontend_id = POPUP_ITEM_ID_SEPARATOR;

components/autofill/core/browser/autofill_external_delegate.h

@@ -109,7 +109,9 @@ class AutofillExternalDelegate : public AutofillPopupDelegate {
   void ApplyAutofillOptions(std::vector<Suggestion>* suggestions);
 
   // Insert the data list values at the start of the given list, including
-  // any required separators.
+  // any required separators. Will also go through |suggestions| and remove
+  // duplicate autocomplete (not Autofill) suggestions, keeping their datalist
+  // version.
   void InsertDataListValues(std::vector<Suggestion>* suggestions);
 
   AutofillManager* manager_;  // weak.

components/autofill/core/browser/autofill_external_delegate_unittest.cc

@@ -292,6 +292,84 @@ TEST_F(AutofillExternalDelegateUnitTest, UpdateDataListWhileShowingPopup) {
                                                data_list_items);
 }
 
+// Test that we _don't_ de-dupe autofill values against datalist values. We
+// keep both with a separator.
+TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutofillDatalistValues) {
+  IssueOnQuery(kQueryId);
+
+  std::vector<base::string16> data_list_items;
+  data_list_items.push_back(base::ASCIIToUTF16("Rick"));
+  data_list_items.push_back(base::ASCIIToUTF16("Deckard"));
+
+  EXPECT_CALL(autofill_client_, UpdateAutofillPopupDataListValues(
+                                    data_list_items, data_list_items));
+
+  external_delegate_->SetCurrentDataListValues(data_list_items,
+                                               data_list_items);
+
+  // The enums must be cast to ints to prevent compile errors on linux_rel.
+  auto element_ids =
+      testing::ElementsAre(static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY),
+                           static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY),
+#if !defined(OS_ANDROID)
+                           static_cast<int>(POPUP_ITEM_ID_SEPARATOR),
+#endif
+                           kAutofillProfileId,
+#if !defined(OS_ANDROID)
+                           static_cast<int>(POPUP_ITEM_ID_SEPARATOR),
+#endif
+                           static_cast<int>(POPUP_ITEM_ID_AUTOFILL_OPTIONS));
+  EXPECT_CALL(autofill_client_,
+              ShowAutofillPopup(_, _, SuggestionVectorIdsAre(element_ids), _));
+
+  // Have an Autofill item that is identical to one of the datalist entries.
+  std::vector<Suggestion> autofill_item;
+  autofill_item.push_back(Suggestion());
+  autofill_item[0].value = ASCIIToUTF16("Rick");
+  autofill_item[0].frontend_id = kAutofillProfileId;
+  ;
+  external_delegate_->OnSuggestionsReturned(kQueryId, autofill_item);
+}
+
+// Test that we de-dupe autocomplete values against datalist values, keeping the
+// latter in case of a match.
+TEST_F(AutofillExternalDelegateUnitTest, DuplicateAutocompleteDatalistValues) {
+  IssueOnQuery(kQueryId);
+
+  std::vector<base::string16> data_list_items;
+  data_list_items.push_back(base::ASCIIToUTF16("Rick"));
+  data_list_items.push_back(base::ASCIIToUTF16("Deckard"));
+
+  EXPECT_CALL(autofill_client_, UpdateAutofillPopupDataListValues(
+                                    data_list_items, data_list_items));
+
+  external_delegate_->SetCurrentDataListValues(data_list_items,
+                                               data_list_items);
+
+  // The enums must be cast to ints to prevent compile errors on linux_rel.
+  auto element_ids = testing::ElementsAre(
+      // We are expecting only two data list entries.
+      static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY),
+      static_cast<int>(POPUP_ITEM_ID_DATALIST_ENTRY),
+#if !defined(OS_ANDROID)
+      static_cast<int>(POPUP_ITEM_ID_SEPARATOR),
+#endif
+      static_cast<int>(POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY));
+  EXPECT_CALL(autofill_client_,
+              ShowAutofillPopup(_, _, SuggestionVectorIdsAre(element_ids), _));
+
+  // Have an Autocomplete item that is identical to one of the datalist entries
+  // and one that is distinct.
+  std::vector<Suggestion> autocomplete_items;
+  autocomplete_items.push_back(Suggestion());
+  autocomplete_items[0].value = ASCIIToUTF16("Rick");
+  autocomplete_items[0].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY;
+  autocomplete_items.push_back(Suggestion());
+  autocomplete_items[1].value = ASCIIToUTF16("Cain");
+  autocomplete_items[1].frontend_id = POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY;
+  external_delegate_->OnSuggestionsReturned(kQueryId, autocomplete_items);
+}
+
 // Test that the Autofill popup is able to display warnings explaining why
 // Autofill is disabled for a website.
 // Regression test for http://crbug.com/247880