Autofill Credit Card Signin Promo: Put behind Feature

components/autofill/core/browser/autofill_manager_unittest.cc

Index: components/autofill/core/browser/autofill_manager_unittest.cc
diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc
index 5ebd6204cb7769aaa0270aa06f1841a0fec031e2..4af20728a60378721138d66313b2ed28c9b06a5c 100644
--- a/components/autofill/core/browser/autofill_manager_unittest.cc
+++ b/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) {
@@ -956,6 +992,7 @@ class AutofillManagerTest : public testing::Test {
   TestPaymentsClient* payments_client_;
   TestAutofillDownloadManager* download_manager_;
   TestPersonalDataManager personal_data_;
+  base::FieldTrialList field_trial_list_;
 };
 
 class TestFormStructure : public FormStructure {
@@ -4910,8 +4947,11 @@ TEST_F(AutofillManagerTest, NoCreditCardSuggestionsForNonPrefixTokenMatch) {
 }
 
 // Test that ShouldShowCreditCardSigninPromo behaves as expected for a credit
-// card form.
+// card form, with no impression limit and the feature enabled.
 TEST_F(AutofillManagerTest, ShouldShowCreditCardSigninPromo_CreditCardField) {

[sebsg, 2016/07/14 15:32:13]

Nit: add _NoLimit as a suffix to be more consistent with the rest of the tests.

[Mathieu, 2016/07/14 21:51:44]

Done.

+  // Enable the feature with no impression limit.
+  EnableCreditCardSigninPromoFeatureWithLimit(0);
+
   // Set up our form data.
   FormData form;
   CreateTestCreditCardFormData(&form, true, false);
@@ -4930,6 +4970,94 @@ TEST_F(AutofillManagerTest, ShouldShowCreditCardSigninPromo_CreditCardField) {
   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_WithUnmetLimit) {

[sebsg, 2016/07/14 15:32:13]

Nit: I think ...Field_UnmetLimit gives the same information with less
characters. But that's just me being picky.

[Mathieu, 2016/07/14 21:51:44]

Done.

+  // 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);
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field;
+  test::CreateTestFormField("Name on Card", "nameoncard", "", "text", &field);
+
+  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));

[sebsg, 2016/07/14 15:32:13]

Could you clarify that you are testing 2 separate ways of showing or not showing
the promo here? It was not obvious to me when I read it. Thanks!

[Mathieu, 2016/07/14 21:51:44]

Done.

+  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.
+  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
 // form.
 TEST_F(AutofillManagerTest, ShouldShowCreditCardSigninPromo_AddressField) {