Now behind a Feature and also will not show if the number of impressi…

…ons is higher than some value passed by Variations params.

BUG=626442
TEST=AutofillManagerTest

Review-Url: https://codereview.chromium.org/2146823003
Cr-Commit-Position: refs/heads/master@{#405615}

components/autofill/core/browser/autofill_experiments.cc

@@ -7,18 +7,23 @@
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/metrics/field_trial.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/autofill_switches.h"
 #include "components/prefs/pref_service.h"
 #include "components/sync_driver/sync_service.h"
+#include "components/variations/variations_associated_data.h"
 #include "google_apis/gaia/gaia_auth_util.h"
 
 namespace autofill {
 
 const base::Feature kAutofillProfileCleanup{"AutofillProfileCleanup",
                                             base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kAutofillCreditCardSigninPromo{
+    "AutofillCreditCardSigninPromo", base::FEATURE_DISABLED_BY_DEFAULT};
+const char kCreditCardSigninPromoImpressionLimitParamKey[] = "impression_limit";
 
 bool IsAutofillEnabled(const PrefService* pref_service) {
   return pref_service->GetBoolean(prefs::kAutofillEnabled);
@@ -34,6 +39,21 @@ bool IsAutofillProfileCleanupEnabled() {
   return base::FeatureList::IsEnabled(kAutofillProfileCleanup);
 }
 
+bool IsAutofillCreditCardSigninPromoEnabled() {
+  return base::FeatureList::IsEnabled(kAutofillCreditCardSigninPromo);
+}
+
+int GetCreditCardSigninPromoImpressionLimit() {
+  int impression_limit;
+  std::string param_value = variations::GetVariationParamValueByFeature(
+      kAutofillCreditCardSigninPromo,
+      kCreditCardSigninPromoImpressionLimitParamKey);
+  if (!param_value.empty() && base::StringToInt(param_value, &impression_limit))
+    return impression_limit;
+
+  return 0;
+}
+
 bool OfferStoreUnmaskedCards() {
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   // The checkbox can be forced on with a flag, but by default we don't store

components/autofill/core/browser/autofill_experiments.h

@@ -20,6 +20,8 @@ class PrefService;
 namespace autofill {
 
 extern const base::Feature kAutofillProfileCleanup;
+extern const base::Feature kAutofillCreditCardSigninPromo;
+extern const char kCreditCardSigninPromoImpressionLimitParamKey[];
 
 // Returns true if autofill should be enabled. See also
 // IsInAutofillSuggestionsDisabledExperiment below.
@@ -34,6 +36,13 @@ bool IsInAutofillSuggestionsDisabledExperiment();
 // Returns whether the Autofill profile cleanup feature is enabled.
 bool IsAutofillProfileCleanupEnabled();
 
+// Returns whether the Autofill credit card signin promo should be shown.
+bool IsAutofillCreditCardSigninPromoEnabled();
+
+// Returns the maximum number of impressions of the credit card signin promo, or
+// 0 if there are no limits.
+int GetCreditCardSigninPromoImpressionLimit();
+
 // Returns true if the user should be offered to locally store unmasked cards.
 // This controls whether the option is presented at all rather than the default
 // response of the option.

components/autofill/core/browser/autofill_manager.cc

@@ -221,6 +221,10 @@ AutofillManager::~AutofillManager() {}
 // static
 void AutofillManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
+  // This pref is not synced because it's for a signin promo, which by
+  // definition will not be synced.
+  registry->RegisterIntegerPref(
+      prefs::kAutofillCreditCardSigninPromoImpressionCount, 0);
   registry->RegisterBooleanPref(
       prefs::kAutofillEnabled,
       true,
@@ -274,11 +278,30 @@ bool AutofillManager::ShouldShowScanCreditCard(const FormData& form,
 bool AutofillManager::ShouldShowCreditCardSigninPromo(
     const FormData& form,
     const FormFieldData& field) {
+  if (!IsAutofillCreditCardSigninPromoEnabled())
+    return false;
+
   // Check whether we are dealing with a credit card field and whether it's
   // appropriate to show the promo (e.g. the platform is supported).
   AutofillField* autofill_field = GetAutofillField(form, field);
-  return autofill_field && autofill_field->Type().group() == CREDIT_CARD &&
-         client_->ShouldShowSigninPromo();
+  if (!autofill_field || autofill_field->Type().group() != CREDIT_CARD ||
+      !client_->ShouldShowSigninPromo())
+    return false;
+
+  // The last step is checking if we are under the limit of impressions (a limit
+  // of 0 means there is no limit);
+  int impression_limit = GetCreditCardSigninPromoImpressionLimit();
+  int impression_count = client_->GetPrefs()->GetInteger(
+      prefs::kAutofillCreditCardSigninPromoImpressionCount);
+  if (impression_limit == 0 || impression_count < impression_limit) {
+    // The promo will be shown. Increment the impression count.
+    client_->GetPrefs()->SetInteger(
+        prefs::kAutofillCreditCardSigninPromoImpressionCount,
+        impression_count + 1);
+    return true;
+  }
+
+  return false;
 }
 
 void AutofillManager::OnFormsSeen(const std::vector<FormData>& forms,

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -11,10 +11,12 @@
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/format_macros.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
+#include "base/metrics/field_trial.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
@@ -27,6 +29,7 @@
 #include "build/build_config.h"
 #include "components/autofill/core/browser/autocomplete_history_manager.h"
 #include "components/autofill/core/browser/autofill_download_manager.h"
+#include "components/autofill/core/browser/autofill_experiments.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/credit_card.h"
@@ -44,6 +47,7 @@
 #include "components/autofill/core/common/form_field_data.h"
 #include "components/prefs/pref_service.h"
 #include "components/rappor/test_rappor_service.h"
+#include "components/variations/variations_associated_data.h"
 #include "grit/components_strings.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -731,6 +735,8 @@ class TestAutofillExternalDelegate : public AutofillExternalDelegate {
 
 class AutofillManagerTest : public testing::Test {
  public:
+  AutofillManagerTest() : field_trial_list_(nullptr) {}
+
   void SetUp() override {
     autofill_client_.SetPrefs(test::PrefServiceForTesting());
     personal_data_.set_database(autofill_client_.GetDatabase());
@@ -749,6 +755,10 @@ class AutofillManagerTest : public testing::Test {
         autofill_manager_.get(),
         autofill_driver_.get()));
     autofill_manager_->SetExternalDelegate(external_delegate_.get());
+
+    // Clear all the things.
+    base::FeatureList::ClearInstanceForTesting();
+    variations::testing::ClearAllVariationParams();
   }
 
   void TearDown() override {
@@ -765,6 +775,32 @@ class AutofillManagerTest : public testing::Test {
     request_context_ = nullptr;
   }
 
+  void EnableCreditCardSigninPromoFeatureWithLimit(int impression_limit) {
+    const std::string kTrialName = "MyTrial";
+    const std::string kGroupName = "Group1";
+
+    scoped_refptr<base::FieldTrial> trial(
+        base::FieldTrialList::CreateFieldTrial(kTrialName, kGroupName));
+    std::map<std::string, std::string> params;
+    params[kCreditCardSigninPromoImpressionLimitParamKey] =
+        base::IntToString(impression_limit);
+    ASSERT_TRUE(
+        variations::AssociateVariationParams(kTrialName, kGroupName, params));
+
+    // Enable the feature.
+    std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList);
+    feature_list->RegisterFieldTrialOverride(
+        kAutofillCreditCardSigninPromo.name,
+        base::FeatureList::OVERRIDE_ENABLE_FEATURE, trial.get());
+    base::FeatureList::SetInstance(std::move(feature_list));
+
+    // Double-checking our params made it.
+    std::map<std::string, std::string> actualParams;
+    EXPECT_TRUE(variations::GetVariationParamsByFeature(
+        kAutofillCreditCardSigninPromo, &actualParams));
+    EXPECT_EQ(params, actualParams);
+  }
+
   void GetAutofillSuggestions(int query_id,
                               const FormData& form,
                               const FormFieldData& field) {
@@ -942,6 +978,7 @@ class AutofillManagerTest : public testing::Test {
   TestPaymentsClient* payments_client_;
   TestAutofillDownloadManager* download_manager_;
   TestPersonalDataManager personal_data_;
+  base::FieldTrialList field_trial_list_;
 };
 
 class TestFormStructure : public FormStructure {
@@ -4888,8 +4925,60 @@ TEST_F(AutofillManagerTest, NoCreditCardSuggestionsForNonPrefixTokenMatch) {
 }
 
 // Test that ShouldShowCreditCardSigninPromo behaves as expected for a credit
-// card form.
-TEST_F(AutofillManagerTest, ShouldShowCreditCardSigninPromo_CreditCardField) {
+// card form, with no impression limit and the feature enabled.
+TEST_F(AutofillManagerTest,
+       ShouldShowCreditCardSigninPromo_CreditCardField_NoLimit) {
+  // Enable the feature with no impression limit.
+  EnableCreditCardSigninPromoFeatureWithLimit(0);
+
+  // Set up our form data.
+  FormData form;
+  CreateTestCreditCardFormData(&form, true, false);
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field;
+  test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
+
+  // The result will depend on what ShouldShowSigninPromo(). We control its
+  // output and verify that both cases behave as expected.
+  EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
+      .WillOnce(testing::Return(true));
+  EXPECT_TRUE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+
+  EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
+      .WillOnce(testing::Return(false));
+  EXPECT_FALSE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+}
+
+// Test that ShouldShowCreditCardSigninPromo doesn't show for a credit card form
+// when the feature is off.
+TEST_F(AutofillManagerTest,
+       ShouldShowCreditCardSigninPromo_CreditCardField_FeatureOff) {
+  // Set up our form data.
+  FormData form;
+  CreateTestCreditCardFormData(&form, true, false);
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field;
+  test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
+
+  // Returns false without calling ShouldShowSigninPromo().
+  EXPECT_CALL(autofill_client_, ShouldShowSigninPromo()).Times(0);
+  EXPECT_FALSE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+}
+
+// Test that ShouldShowCreditCardSigninPromo behaves as expected for a credit
+// card form with an impression limit and no impressions yet.
+TEST_F(AutofillManagerTest,
+       ShouldShowCreditCardSigninPromo_CreditCardField_UnmetLimit) {
+  // Enable the feature with an impression limit.
+  EnableCreditCardSigninPromoFeatureWithLimit(10);
+  // No impressions yet.
+  ASSERT_EQ(0, autofill_client_.GetPrefs()->GetInteger(
+                   prefs::kAutofillCreditCardSigninPromoImpressionCount));
+
   // Set up our form data.
   FormData form;
   CreateTestCreditCardFormData(&form, true, false);
@@ -4899,13 +4988,58 @@ TEST_F(AutofillManagerTest, ShouldShowCreditCardSigninPromo_CreditCardField) {
   FormFieldData field;
   test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
 
+  // The mock implementation of ShouldShowSigninPromo() will return true here,
+  // creating an impression, and false below, preventing an impression.
   EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
       .WillOnce(testing::Return(true));
   EXPECT_TRUE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
 
+  // Expect to now have an impression.
+  EXPECT_EQ(1, autofill_client_.GetPrefs()->GetInteger(
+                   prefs::kAutofillCreditCardSigninPromoImpressionCount));
+
   EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
       .WillOnce(testing::Return(false));
   EXPECT_FALSE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+
+  // No additional impression.
+  EXPECT_EQ(1, autofill_client_.GetPrefs()->GetInteger(
+                   prefs::kAutofillCreditCardSigninPromoImpressionCount));
+}
+
+// Test that ShouldShowCreditCardSigninPromo behaves as expected for a credit
+// card form with an impression limit that has been attained already.
+TEST_F(AutofillManagerTest,
+       ShouldShowCreditCardSigninPromo_CreditCardField_WithAttainedLimit) {
+  // Enable the feature with an impression limit.
+  EnableCreditCardSigninPromoFeatureWithLimit(10);
+
+  // Set up our form data.
+  FormData form;
+  CreateTestCreditCardFormData(&form, true, false);
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field;
+  test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
+
+  // Set the impression count to the same value as the limit.
+  autofill_client_.GetPrefs()->SetInteger(
+      prefs::kAutofillCreditCardSigninPromoImpressionCount, 10);
+
+  // Both calls will now return false, regardless of the mock implementation of
+  // ShouldShowSigninPromo().
+  EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
+      .WillOnce(testing::Return(true));
+  EXPECT_FALSE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+
+  EXPECT_CALL(autofill_client_, ShouldShowSigninPromo())
+      .WillOnce(testing::Return(false));
+  EXPECT_FALSE(autofill_manager_->ShouldShowCreditCardSigninPromo(form, field));
+
+  // Number of impressions stay the same.
+  EXPECT_EQ(10, autofill_client_.GetPrefs()->GetInteger(
+                    prefs::kAutofillCreditCardSigninPromoImpressionCount));
 }
 
 // Test that ShouldShowCreditCardSigninPromo behaves as expected for an address

components/autofill/core/common/autofill_pref_names.cc

@@ -7,6 +7,10 @@
 namespace autofill {
 namespace prefs {
 
+// Number of times the credit card signin promo has been shown.
+const char kAutofillCreditCardSigninPromoImpressionCount[] =
+    "autofill.credit_card_signin_promo_impression_count";
+
 // Boolean that is true if Autofill is enabled and allowed to save profile data.
 const char kAutofillEnabled[] = "autofill.enabled";
 

components/autofill/core/common/autofill_pref_names.h

@@ -11,6 +11,7 @@ namespace prefs {
 // Alphabetical list of preference names specific to the Autofill
 // component. Keep alphabetized, and document each in the .cc file.
 
+extern const char kAutofillCreditCardSigninPromoImpressionCount[];
 extern const char kAutofillEnabled[];
 extern const char kAutofillProfileUseDatesFixed[];
 extern const char kAutofillLastVersionDeduped[];