Commit 990a4f73 authored by estade's avatar estade Committed by Commit bot

Autofill - sort profile and credit card suggestions by MFU.

BUG=342426

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

Cr-Commit-Position: refs/heads/master@{#317142}
parent c902ed8d
......@@ -217,6 +217,18 @@ static bool CompareVotes(const std::pair<std::string, int>& a,
return a.second < b.second;
}
// Ranks two data models according to their recency of use. Currently this will
// place all server (Wallet) cards and addresses below all locally saved ones,
// which is probably not what we want. TODO(estade): figure out relative ranking
// of server data.
bool RankByMfu(const AutofillDataModel* a, const AutofillDataModel* b) {
if (a->use_count() != b->use_count())
return a->use_count() > b->use_count();
// Ties are broken by MRU.
return a->use_date() > b->use_date();
}
} // namespace
PersonalDataManager::PersonalDataManager(const std::string& app_locale)
......@@ -717,13 +729,16 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
base::string16 field_contents_canon =
AutofillProfile::CanonicalizeProfileString(field_contents);
std::vector<AutofillProfile*> profiles = GetProfiles(true);
std::sort(profiles.begin(), profiles.end(), RankByMfu);
if (field_is_autofilled) {
// This field was previously autofilled. In this case, suggesting results
// based on prefix is useless since it will be the same thing. Instead,
// check for a field that may have multiple possible values (for example,
// multiple names for the same address) and suggest the alternates. This
// allows for easy correction of the data.
for (AutofillProfile* profile : GetProfiles(true)) {
for (AutofillProfile* profile : profiles) {
std::vector<base::string16> values =
GetMultiInfoInOneLine(profile, type, app_locale_);
......@@ -750,7 +765,7 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
} else {
// Match based on a prefix search.
std::vector<AutofillProfile*> matched_profiles;
for (AutofillProfile* profile : GetProfiles(true)) {
for (AutofillProfile* profile : profiles) {
std::vector<base::string16> values =
GetMultiInfoInOneLine(profile, type, app_locale_);
for (size_t i = 0; i < values.size(); i++) {
......@@ -826,6 +841,8 @@ std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
}
}
cards_to_suggest.sort(RankByMfu);
std::vector<Suggestion> suggestions;
for (const CreditCard* credit_card : cards_to_suggest) {
// Make a new suggestion.
......
......@@ -2709,21 +2709,22 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions) {
TEST_F(PersonalDataManagerTest, GetCreditCardSuggestions) {
EnableWalletCardImport();
// These GUIDs are reverse alphabetical to make validating expectations
// easier.
CreditCard credit_card0("287151C8-6AB1-487C-9095-28E80BE5DA15",
"https://www.example.com");
test::SetCreditCardInfo(&credit_card0,
"Clyde Barrow", "347666888555" /* American Express */, "04", "2015");
credit_card0.set_use_count(2);
personal_data_->AddCreditCard(credit_card0);
CreditCard credit_card1("1141084B-72D7-4B73-90CF-3D6AC154673B",
"https://www.example.com");
credit_card1.set_use_count(3);
test::SetCreditCardInfo(&credit_card1, "John Dillinger", "", "01", "2010");
personal_data_->AddCreditCard(credit_card1);
CreditCard credit_card2("002149C1-EE28-4213-A3B9-DA243FFF021B",
"https://www.example.com");
credit_card2.set_use_count(1);
test::SetCreditCardInfo(&credit_card2,
"Bonnie Parker", "518765432109" /* Mastercard */, "", "");
personal_data_->AddCreditCard(credit_card2);
......@@ -2737,21 +2738,22 @@ TEST_F(PersonalDataManagerTest, GetCreditCardSuggestions) {
personal_data_->GetCreditCardSuggestions(
AutofillType(CREDIT_CARD_NAME), base::string16());
ASSERT_EQ(3U, suggestions.size());
EXPECT_EQ(ASCIIToUTF16("Clyde Barrow"), suggestions[2].value);
EXPECT_EQ(ASCIIToUTF16("*8555"), suggestions[2].label);
EXPECT_EQ(ASCIIToUTF16("John Dillinger"), suggestions[1].value);
EXPECT_EQ(base::string16(), suggestions[1].label);
EXPECT_EQ(ASCIIToUTF16("Bonnie Parker"), suggestions[0].value);
EXPECT_EQ(ASCIIToUTF16("*2109"), suggestions[0].label);
// Ordered by MFU.
EXPECT_EQ(ASCIIToUTF16("Clyde Barrow"), suggestions[1].value);
EXPECT_EQ(ASCIIToUTF16("*8555"), suggestions[1].label);
EXPECT_EQ(ASCIIToUTF16("John Dillinger"), suggestions[0].value);
EXPECT_EQ(base::string16(), suggestions[0].label);
EXPECT_EQ(ASCIIToUTF16("Bonnie Parker"), suggestions[2].value);
EXPECT_EQ(ASCIIToUTF16("*2109"), suggestions[2].label);
// Sublabel is expiration date when filling card number.
suggestions = personal_data_->GetCreditCardSuggestions(
AutofillType(CREDIT_CARD_NUMBER), base::string16());
ASSERT_EQ(2U, suggestions.size());
EXPECT_EQ(ASCIIToUTF16("American Express - 8555"), suggestions[1].value);
EXPECT_EQ(ASCIIToUTF16("04/15"), suggestions[1].label);
EXPECT_EQ(ASCIIToUTF16("MasterCard - 2109"), suggestions[0].value);
EXPECT_EQ(base::string16(), suggestions[0].label);
EXPECT_EQ(ASCIIToUTF16("American Express - 8555"), suggestions[0].value);
EXPECT_EQ(ASCIIToUTF16("04/15"), suggestions[0].label);
EXPECT_EQ(ASCIIToUTF16("MasterCard - 2109"), suggestions[1].value);
EXPECT_EQ(base::string16(), suggestions[1].label);
// Add some server cards. If there are local dupes, the locals should be
// hidden.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment