[Autofill] Deduplicate Autocomplete and Datalist suggestions, keeping the latter

components/autofill/core/browser/autofill_external_delegate.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/autofill_external_delegate.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/i18n/case_conversion.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 291 matching lines @@
   suggestions->back().frontend_id = POPUP_ITEM_ID_AUTOFILL_OPTIONS;
   if (IsKeyboardAccessoryEnabled())
     suggestions->back().icon = base::ASCIIToUTF16("settings");
 }
 
 void AutofillExternalDelegate::InsertDataListValues(
     std::vector<Suggestion>* suggestions) {
   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> datalist_set(data_list_values_.begin(),

[Evan Stade, 2015/11/19 03:13:03]

please keep datalist/data_list consistent

[Mathieu, 2015/11/19 13:22:17]

Done.

+                                        data_list_values_.end());
+  for (auto iter = suggestions->begin(); iter != suggestions->end();) {

[Evan Stade, 2015/11/19 03:13:03]

std::remove_if(suggestions->begin(), 
               suggestions->end(),
               [](const Suggestion& suggestion){return
data_list_set.count(suggestion.value) > 0 && suggestion.frontend_id ==
POPUP...;});

[Evan Stade, 2015/11/19 03:19:51]

note that you might have to put a & or something in the [] of the lambda to
capture the data_list_set

[vabr (Chromium), 2015/11/19 08:26:04]

Please use [&datalist_set], not just [&], because the latter is forbidden by the
style.

Also don't forget to call erase:
suggestions.erase(std::remove_if(...), suggestions.end());

Other than that, big +1 to using remove_if, it is much more efficient.

[Mathieu, 2015/11/19 13:22:16]

Done.

[Mathieu, 2015/11/19 13:22:17]

Done.

[Mathieu, 2015/11/19 13:22:17]

Done.

+    if (datalist_set.count(iter->value) &&

[vabr (Chromium), 2015/11/19 08:26:04]

optional: While using .count() results in succinct code, it is not ideal,
because it asks the program to compute more than needed. My suggestion here is
ContainsKey from base/stl_util.h.

[Mathieu, 2015/11/19 13:22:17]

Ah thanks! Done.

+        iter->frontend_id == POPUP_ITEM_ID_AUTOCOMPLETE_ENTRY) {
+      iter = suggestions->erase(iter);
+      continue;
+    }
+    ++iter;
+  }
+
 #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;
   }
 #endif
 
   // Insert the datalist elements at the beginning.
   suggestions->insert(suggestions->begin(), data_list_values_.size(),
                       Suggestion());
   for (size_t i = 0; i < data_list_values_.size(); i++) {
     (*suggestions)[i].value = data_list_values_[i];
     (*suggestions)[i].label = data_list_labels_[i];
     (*suggestions)[i].frontend_id = POPUP_ITEM_ID_DATALIST_ENTRY;
   }
 }
 
 }  // namespace autofill