diff options
author | sebsg <sebsg@chromium.org> | 2015-10-20 11:31:26 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-10-20 18:31:59 +0000 |
commit | 220c73916470009f4b408fc15aa91fa5bf8d0df8 (patch) | |
tree | 7fbd948eecc6e5e2977c9f6238c2a55efb343a18 | |
parent | cd3db70d7039fcdffaae0d04ac92ce5dd3512d8e (diff) | |
download | chromium_src-220c73916470009f4b408fc15aa91fa5bf8d0df8.zip chromium_src-220c73916470009f4b408fc15aa91fa5bf8d0df8.tar.gz chromium_src-220c73916470009f4b408fc15aa91fa5bf8d0df8.tar.bz2 |
[Autofill] Field trial to show only three profile suggestions.
BUG=535993
Review URL: https://codereview.chromium.org/1390173003
Cr-Commit-Position: refs/heads/master@{#355111}
9 files changed, 190 insertions, 13 deletions
diff --git a/components/autofill/core/browser/personal_data_manager.cc b/components/autofill/core/browser/personal_data_manager.cc index 61cb7f1..0663afb 100644 --- a/components/autofill/core/browser/personal_data_manager.cc +++ b/components/autofill/core/browser/personal_data_manager.cc @@ -4,6 +4,8 @@ #include "components/autofill/core/browser/personal_data_manager.h" +#include <algorithm> + #include "base/i18n/case_conversion.h" #include "base/i18n/timezone.h" #include "base/metrics/field_trial.h" @@ -29,6 +31,7 @@ #include "components/signin/core/browser/account_tracker_service.h" #include "components/signin/core/browser/signin_manager.h" #include "components/signin/core/common/signin_pref_names.h" +#include "components/variations/variations_associated_data.h" #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h" #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_formatter.h" @@ -201,12 +204,17 @@ bool RankByMfu(const AutofillDataModel* a, const AutofillDataModel* b) { // Returns whether sorting autofill profile suggestions by frecency is enabled. bool IsAutofillProfileSortingByFrecencyEnabled() { const std::string group_name = - base::FieldTrialList::FindFullName("AutofillProfileOrderByFrecency"); - return base::StartsWith(group_name, "Enabled", base::CompareCase::SENSITIVE); + base::FieldTrialList::FindFullName(kFrecencyFieldTrialName); + return base::StartsWith(group_name, kFrecencyFieldTrialStateEnabled, + base::CompareCase::SENSITIVE); } } // namespace +const char kFrecencyFieldTrialName[] = "AutofillProfileOrderByFrecency"; +const char kFrecencyFieldTrialStateEnabled[] = "Enabled"; +const char kFrecencyFieldTrialLimitParam[] = "limit"; + PersonalDataManager::PersonalDataManager(const std::string& app_locale) : database_(NULL), is_data_loaded_(false), @@ -873,6 +881,14 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions( for (size_t i = 0; i < labels.size(); i++) unique_suggestions[i].label = labels[i]; + // Get the profile suggestions limit value set for the current frecency field + // trial group or SIZE_MAX if no limit is defined. + std::string limit_str = variations::GetVariationParamValue( + kFrecencyFieldTrialName, kFrecencyFieldTrialLimitParam); + size_t limit = base::StringToSizeT(limit_str, &limit) ? limit : SIZE_MAX; + + unique_suggestions.resize(std::min(unique_suggestions.size(), limit)); + return unique_suggestions; } diff --git a/components/autofill/core/browser/personal_data_manager.h b/components/autofill/core/browser/personal_data_manager.h index d4a5ead..637172e 100644 --- a/components/autofill/core/browser/personal_data_manager.h +++ b/components/autofill/core/browser/personal_data_manager.h @@ -50,6 +50,10 @@ void SetCreditCards(int, std::vector<autofill::CreditCard>*); namespace autofill { +extern const char kFrecencyFieldTrialName[]; +extern const char kFrecencyFieldTrialStateEnabled[]; +extern const char kFrecencyFieldTrialLimitParam[]; + // Handles loading and saving Autofill profile information to the web database. // This class also stores the profiles loaded from the database for use during // Autofill. diff --git a/components/autofill/core/browser/personal_data_manager_unittest.cc b/components/autofill/core/browser/personal_data_manager_unittest.cc index 25ffd73..79a66ee 100644 --- a/components/autofill/core/browser/personal_data_manager_unittest.cc +++ b/components/autofill/core/browser/personal_data_manager_unittest.cc @@ -34,6 +34,7 @@ #include "components/signin/core/browser/test_signin_client.h" #include "components/signin/core/common/signin_pref_names.h" #include "components/variations/entropy_provider.h" +#include "components/variations/variations_associated_data.h" #include "components/webdata/common/web_data_service_base.h" #include "components/webdata/common/web_database_service.h" #include "testing/gmock/include/gmock/gmock.h" @@ -182,11 +183,30 @@ class PersonalDataManagerTest : public testing::Test { ASSERT_EQ(1U, personal_data_->GetProfiles().size()); } - void EnableAutofillProfileOrderByFrecencyFieldTrial() { + // Sets up the frecency field trial group and parameter. Enables the frecency + // suggestions ordering if |frecency_enabled| and sets up the suggestions + // limit parameter to |limit_param| if it's not empty. + void EnableAutofillProfileOrderByFrecencyFieldTrial( + const bool frecency_enabled, + const std::string& limit_param) { + // Clear the existing |field_trial_list_| and variation parameters. + field_trial_list_.reset(NULL); field_trial_list_.reset( new base::FieldTrialList(new metrics::SHA1EntropyProvider("foo"))); + variations::testing::ClearAllVariationParams(); + + std::string group_name(frecency_enabled ? kFrecencyFieldTrialStateEnabled + : "Generic"); + + if (!limit_param.empty()) { + std::map<std::string, std::string> params; + params[kFrecencyFieldTrialLimitParam] = limit_param; + variations::AssociateVariationParams(kFrecencyFieldTrialName, group_name, + params); + } + field_trial_ = base::FieldTrialList::CreateFieldTrial( - "AutofillProfileOrderByFrecency", "Enabled"); + kFrecencyFieldTrialName, group_name); field_trial_->group(); } @@ -3433,18 +3453,18 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_RankByMru) { std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions( AutofillType(NAME_FIRST), base::ASCIIToUTF16("Ma"), false, std::vector<ServerFieldType>()); - ASSERT_EQ(3, static_cast<int>(suggestions.size())); + ASSERT_EQ(3U, suggestions.size()); EXPECT_EQ(suggestions[0].value, base::ASCIIToUTF16("Marion1_2")); EXPECT_EQ(suggestions[1].value, base::ASCIIToUTF16("Marion2_1")); EXPECT_EQ(suggestions[2].value, base::ASCIIToUTF16("Marion3_3")); // Verify the profiles are sorted by frecency when the flag is set. - EnableAutofillProfileOrderByFrecencyFieldTrial(); + EnableAutofillProfileOrderByFrecencyFieldTrial(true, ""); suggestions = personal_data_->GetProfileSuggestions( AutofillType(NAME_FIRST), base::ASCIIToUTF16("Ma"), false, std::vector<ServerFieldType>()); - ASSERT_EQ(3, static_cast<int>(suggestions.size())); + ASSERT_EQ(3U, suggestions.size()); EXPECT_EQ(suggestions[0].value, base::ASCIIToUTF16("Marion2_1")); EXPECT_EQ(suggestions[1].value, base::ASCIIToUTF16("Marion1_2")); EXPECT_EQ(suggestions[2].value, base::ASCIIToUTF16("Marion3_3")); @@ -3499,4 +3519,123 @@ TEST_F(PersonalDataManagerTest, SaveImportedProfile_StateAbbreviationToFull) { profiles2.front()->GetRawInfo(ADDRESS_HOME_STATE)); } +// Tests that GetProfileSuggestions returns all profiles suggestions by default +// and only two if the appropriate field trial is set. +TEST_F(PersonalDataManagerTest, GetProfileSuggestions_NumberOfSuggestions) { + // Set up 3 different profiles. + AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile1, "Marion1", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + personal_data_->AddProfile(profile1); + + AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile2, "Marion2", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + personal_data_->AddProfile(profile2); + + AutofillProfile profile3(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile3, "Marion3", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + personal_data_->AddProfile(profile3); + + ResetPersonalDataManager(USER_MODE_NORMAL); + + // Verify that all the profiles are suggested. + std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions( + AutofillType(NAME_FIRST), base::string16(), false, + std::vector<ServerFieldType>()); + EXPECT_EQ(3U, suggestions.size()); + + // Verify that only two profiles are suggested. + EnableAutofillProfileOrderByFrecencyFieldTrial(false, "2"); + + suggestions = personal_data_->GetProfileSuggestions( + AutofillType(NAME_FIRST), base::string16(), false, + std::vector<ServerFieldType>()); + EXPECT_EQ(2U, suggestions.size()); +} + +// Tests that GetProfileSuggestions returns two profiles suggestions ordered by +// frecency if the appropriate field trial group and parameter are set. +TEST_F(PersonalDataManagerTest, + GetProfileSuggestions_FrecencyAndSuggestionsLimit) { + // Set up the profiles. They are named with number suffixes X_Y so the X is + // the order in which they should be ordered by MRU and Y is the order in + // which they should be ranked by frecency. + AutofillProfile profile3(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile3, "Marion3_3", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + profile3.set_use_date(base::Time::Now() - base::TimeDelta::FromDays(1)); + profile3.set_use_count(5); + personal_data_->AddProfile(profile3); + + AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile1, "Marion2_1", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + profile1.set_use_date(base::Time::Now() - base::TimeDelta::FromDays(1)); + profile1.set_use_count(10); + personal_data_->AddProfile(profile1); + + AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile2, "Marion1_2", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + profile2.set_use_date(base::Time::Now() - base::TimeDelta::FromDays(15)); + profile2.set_use_count(300); + personal_data_->AddProfile(profile2); + + ResetPersonalDataManager(USER_MODE_NORMAL); + + // Verify that only two profiles are suggested and ordered by frecency. + EnableAutofillProfileOrderByFrecencyFieldTrial(true, "2"); + + std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions( + AutofillType(NAME_FIRST), base::ASCIIToUTF16("Ma"), false, + std::vector<ServerFieldType>()); + ASSERT_EQ(2U, suggestions.size()); + EXPECT_EQ(suggestions[0].value, base::ASCIIToUTF16("Marion2_1")); + EXPECT_EQ(suggestions[1].value, base::ASCIIToUTF16("Marion1_2")); +} + +// Tests that GetProfileSuggestions returns the right number of profile +// suggestions when the limit to three field trial is set and there are less +// than three profiles. +TEST_F(PersonalDataManagerTest, + GetProfileSuggestions_LimitIsLessThanProfileSuggestions) { + EnableAutofillProfileOrderByFrecencyFieldTrial(false, "3"); + + // Set up 2 different profiles. + AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile1, "Marion1", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + personal_data_->AddProfile(profile1); + + AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile2, "Marion2", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", + "123 Zoo St.\nSecond Line\nThird line", "unit 5", + "Hollywood", "CA", "91601", "US", "12345678910"); + personal_data_->AddProfile(profile2); + + ResetPersonalDataManager(USER_MODE_NORMAL); + + std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions( + AutofillType(NAME_FIRST), base::string16(), false, + std::vector<ServerFieldType>()); + EXPECT_EQ(2U, suggestions.size()); +} + } // namespace autofill diff --git a/testing/variations/fieldtrial_testing_config_android.json b/testing/variations/fieldtrial_testing_config_android.json index 24a3d95..92cf266 100644 --- a/testing/variations/fieldtrial_testing_config_android.json +++ b/testing/variations/fieldtrial_testing_config_android.json @@ -16,7 +16,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "ChromotingQUIC": [ diff --git a/testing/variations/fieldtrial_testing_config_chromeos.json b/testing/variations/fieldtrial_testing_config_chromeos.json index 04fe0b7..5b417c8 100644 --- a/testing/variations/fieldtrial_testing_config_chromeos.json +++ b/testing/variations/fieldtrial_testing_config_chromeos.json @@ -11,7 +11,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "CaptivePortalInterstitial": [ diff --git a/testing/variations/fieldtrial_testing_config_ios.json b/testing/variations/fieldtrial_testing_config_ios.json index 40d909d..2e72cf1 100644 --- a/testing/variations/fieldtrial_testing_config_ios.json +++ b/testing/variations/fieldtrial_testing_config_ios.json @@ -6,7 +6,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "ChromotingQUIC": [ diff --git a/testing/variations/fieldtrial_testing_config_linux.json b/testing/variations/fieldtrial_testing_config_linux.json index d6abb1a..af6ef4e 100644 --- a/testing/variations/fieldtrial_testing_config_linux.json +++ b/testing/variations/fieldtrial_testing_config_linux.json @@ -11,7 +11,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "CaptivePortalInterstitial": [ diff --git a/testing/variations/fieldtrial_testing_config_mac.json b/testing/variations/fieldtrial_testing_config_mac.json index e9d0492..d9b4744 100644 --- a/testing/variations/fieldtrial_testing_config_mac.json +++ b/testing/variations/fieldtrial_testing_config_mac.json @@ -11,7 +11,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "AutomaticTabDiscarding": [ diff --git a/testing/variations/fieldtrial_testing_config_win.json b/testing/variations/fieldtrial_testing_config_win.json index 68e2fa6..cd2b728 100644 --- a/testing/variations/fieldtrial_testing_config_win.json +++ b/testing/variations/fieldtrial_testing_config_win.json @@ -16,7 +16,10 @@ ], "AutofillProfileOrderByFrecency": [ { - "group_name": "Enabled" + "group_name": "EnabledLimitTo3", + "params": { + "limit": "3" + } } ], "AutomaticTabDiscarding": [ |