[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}
cbchan · Oct 20, 2015 · 220c739 · 220c739
1 parent cd3db70
commit 220c739
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -16,7 +16,10 @@
     ],
     "AutofillProfileOrderByFrecency": [
         {
-            "group_name": "Enabled"
+            "group_name": "EnabledLimitTo3",
+            "params": {
+                "limit": "3"
+            }
         }
     ],
     "AutomaticTabDiscarding": [