From 44820ba4336b5acfd238ef2bcb61b7b2dc647228 Mon Sep 17 00:00:00 2001
From: "isherman@chromium.org"
<isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu, 16 Dec 2010 05:14:07 +0000
Subject: [PATCH] Don't show duplicate Autofill suggestions.
BUG=65133
TEST=unit_tests --gtest_filter=AutoFillManagerTest.GetProfileSuggestionsWithDuplicates
Review URL: http://codereview.chromium.org/5877001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69380 0039d316-1c4b-4281-b951-d872f2087c98
---
chrome/browser/autofill/autofill_manager.cc | 57 +++++++++----------
chrome/browser/autofill/autofill_manager.h | 4 +-
.../autofill/autofill_manager_unittest.cc | 45 +++++++++++++++
3 files changed, 75 insertions(+), 31 deletions(-)
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 05b8dc4e1be90..26a558377e1a8 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -5,7 +5,7 @@
#include "chrome/browser/autofill/autofill_manager.h"
#include <limits>
-#include <string>
+#include <set>
#include "app/l10n_util.h"
#include "base/basictypes.h"
@@ -42,35 +42,35 @@ const double kAutoFillNegativeUploadRateDefaultValue = 0.01;
const string16::value_type kCreditCardPrefix[] = {'*',0};
const string16::value_type kLabelSeparator[] = {';',' ','*',0};
-// Removes duplicate elements whilst preserving original order of |elements| and
-// |unique_ids|.
-void RemoveDuplicateElements(
- std::vector<string16>* elements, std::vector<int>* unique_ids) {
- DCHECK_EQ(elements->size(), unique_ids->size());
-
- std::vector<string16> elements_copy;
+// Removes duplicate suggestions whilst preserving their original order.
+void RemoveDuplicateSuggestions(std::vector<string16>* values,
+ std::vector<string16>* labels,
+ std::vector<string16>* icons,
+ std::vector<int>* unique_ids) {
+ DCHECK_EQ(values->size(), labels->size());
+ DCHECK_EQ(values->size(), icons->size());
+ DCHECK_EQ(values->size(), unique_ids->size());
+
+ std::set<std::pair<string16, string16> > seen_suggestions;
+ std::vector<string16> values_copy;
+ std::vector<string16> labels_copy;
+ std::vector<string16> icons_copy;
std::vector<int> unique_ids_copy;
- for (size_t i = 0; i < elements->size(); ++i) {
- const string16& element = (*elements)[i];
-
- bool unique = true;
- for (std::vector<string16>::const_iterator copy_iter
- = elements_copy.begin();
- copy_iter != elements_copy.end(); ++copy_iter) {
- if (element == *copy_iter) {
- unique = false;
- break;
- }
- }
- if (unique) {
- elements_copy.push_back(element);
+ for (size_t i = 0; i < values->size(); ++i) {
+ const std::pair<string16, string16> suggestion((*values)[i], (*labels)[i]);
+ if (seen_suggestions.insert(suggestion).second) {
+ values_copy.push_back((*values)[i]);
+ labels_copy.push_back((*labels)[i]);
+ icons_copy.push_back((*icons)[i]);
unique_ids_copy.push_back((*unique_ids)[i]);
}
}
- elements->assign(elements_copy.begin(), elements_copy.end());
- unique_ids->assign(unique_ids_copy.begin(), unique_ids_copy.end());
+ values->swap(values_copy);
+ labels->swap(labels_copy);
+ icons->swap(icons_copy);
+ unique_ids->swap(unique_ids_copy);
}
// Precondition: |form_structure| and |form| should correspond to the same
@@ -258,14 +258,13 @@ bool AutoFillManager::GetAutoFillSuggestions(const FormData& form,
// If the form is auto-filled and the renderer is querying for suggestions,
// then the user is editing the value of a field. In this case, mimick
- // autocomplete. In particular, don't display labels, as that information is
- // redundant. In addition, remove duplicate values.
+ // autocomplete: don't display or icons, as that information is redundant.
if (FormIsAutoFilled(form_structure, form, is_filling_credit_card)) {
- RemoveDuplicateElements(&values, &unique_ids);
- labels.assign(values.size(), string16());
- icons.assign(values.size(), string16());
+ labels.assign(labels.size(), string16());
+ icons.assign(icons.size(), string16());
}
+ RemoveDuplicateSuggestions(&values, &labels, &icons, &unique_ids);
host->AutoFillSuggestionsReturned(values, labels, icons, unique_ids);
return true;
}
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 3a683367dc601..750aa7540601b 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -6,9 +6,9 @@
#define CHROME_BROWSER_AUTOFILL_AUTOFILL_MANAGER_H_
#pragma once
-#include <vector>
-#include <string>
#include <list>
+#include <string>
+#include <vector>
#include "base/gtest_prod_util.h"
#include "base/scoped_ptr.h"
diff --git a/chrome/browser/autofill/autofill_manager_unittest.cc b/chrome/browser/autofill/autofill_manager_unittest.cc
index bf6877f00a7ac..b454143284326 100644
--- a/chrome/browser/autofill/autofill_manager_unittest.cc
+++ b/chrome/browser/autofill/autofill_manager_unittest.cc
@@ -598,6 +598,51 @@ TEST_F(AutoFillManagerTest, GetProfileSuggestionsUnknownFields) {
EXPECT_FALSE(autofill_manager_->GetAutoFillSuggestions(form, field));
}
+// Test that we cull duplicate profile suggestions.
+TEST_F(AutoFillManagerTest, GetProfileSuggestionsWithDuplicates) {
+ // Set up our form data.
+ FormData form;
+ CreateTestAddressFormData(&form);
+ std::vector<FormData> forms(1, form);
+ autofill_manager_->FormsSeen(forms);
+
+ // Add a duplicate profile.
+ AutoFillProfile* duplicate_profile = static_cast<AutoFillProfile*>(
+ autofill_manager_->GetLabeledProfile("Home")->Clone());
+ autofill_manager_->AddProfile(duplicate_profile);
+
+ const FormField& field = form.fields[0];
+ rvh()->ResetAutoFillState(kDefaultPageID);
+ EXPECT_TRUE(autofill_manager_->GetAutoFillSuggestions(form, field));
+
+ // No suggestions provided, so send an empty vector as the results.
+ // This triggers the combined message send.
+ rvh()->AutocompleteSuggestionsReturned(std::vector<string16>());
+
+ // Test that we sent the right message to the renderer.
+ int page_id = 0;
+ std::vector<string16> values;
+ std::vector<string16> labels;
+ std::vector<string16> icons;
+ std::vector<int> unique_ids;
+ EXPECT_TRUE(GetAutoFillSuggestionsMessage(&page_id, &values, &labels, &icons,
+ &unique_ids));
+
+ string16 expected_values[] = {
+ ASCIIToUTF16("Elvis"),
+ ASCIIToUTF16("Charles")
+ };
+ string16 expected_labels[] = {
+ ASCIIToUTF16("3734 Elvis Presley Blvd."),
+ ASCIIToUTF16("123 Apple St.")
+ };
+ string16 expected_icons[] = {string16(), string16()};
+ int expected_unique_ids[] = {1, 2};
+ ExpectSuggestions(page_id, values, labels, icons, unique_ids,
+ kDefaultPageID, arraysize(expected_values), expected_values,
+ expected_labels, expected_icons, expected_unique_ids);
+}
+
// Test that we return no suggestions when autofill is disabled.
TEST_F(AutoFillManagerTest, GetProfileSuggestionsAutofillDisabledByUser) {
// Set up our form data.
--
GitLab