Autocomplete - Don't store fields we've identified as CVC.

It's possible Chrome will fail to identify a CVC field, and then Autocomplete will save the value. It's not easy to tell if something is a cvc just by looking at it (3 or 4 numerical digits could be anything).

BUG=479374

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

Cr-Commit-Position: refs/heads/master@{#326615}

5 files changed, 75 insertions, 13 deletions

diff --git a/components/autofill/core/browser/autocomplete_history_manager.cc b/components/autofill/core/browser/autocomplete_history_manager.cc
index 79d75b8..f969dbf 100644
--- a/components/autofill/core/browser/autocomplete_history_manager.cc
+++ b/components/autofill/core/browser/autocomplete_history_manager.cc
@@ -124,6 +124,8 @@ void AutocompleteHistoryManager::OnFormSubmitted(const FormData& form) {
   //  - autocomplete is not disabled
   //  - value is not a credit card number
   //  - value is not a SSN
+  //  - field was not identified as a CVC field (this is handled in
+  //    AutofillManager)
   std::vector<FormFieldData> values;
   for (std::vector<FormFieldData>::const_iterator iter =
            form.fields.begin();
diff --git a/components/autofill/core/browser/autocomplete_history_manager_unittest.cc b/components/autofill/core/browser/autocomplete_history_manager_unittest.cc
index d3733b5..1bedd27 100644
--- a/components/autofill/core/browser/autocomplete_history_manager_unittest.cc
+++ b/components/autofill/core/browser/autocomplete_history_manager_unittest.cc
@@ -166,7 +166,9 @@ TEST_F(AutocompleteHistoryManagerTest, SearchField) {
 }
 
 // Tests that text entered into fields specifying autocomplete="off" is not sent
-// to the WebDatabase to be saved.
+// to the WebDatabase to be saved. Note this is also important as the mechanism
+// for preventing CVCs from being saved.
+// See AutofillManagerTest.DontSaveCvcInAutocompleteHistory
 TEST_F(AutocompleteHistoryManagerTest, FieldWithAutocompleteOff) {
   FormData form;
   form.name = ASCIIToUTF16("MyForm");
diff --git a/components/autofill/core/browser/autofill_manager.cc b/components/autofill/core/browser/autofill_manager.cc
index beb4281..17a1722 100644
--- a/components/autofill/core/browser/autofill_manager.cc
+++ b/components/autofill/core/browser/autofill_manager.cc
@@ -383,12 +383,24 @@ bool AutofillManager::OnFormSubmitted(const FormData& form) {
   if (!IsValidFormData(form))
     return false;
 
-  // Let Autocomplete know as well.
-  autocomplete_history_manager_->OnFormSubmitted(form);
-
+  // We will always give Autocomplete a chance to save the data.
   scoped_ptr<FormStructure> submitted_form = ValidateSubmittedForm(form);
-  if (!submitted_form)
+  if (!submitted_form) {
+    autocomplete_history_manager_->OnFormSubmitted(form);
     return false;
+  }
+
+  // However, if Autofill has recognized a field as CVC, that shouldn't be
+  // saved.
+  FormData form_for_autocomplete = submitted_form->ToFormData();
+  form_for_autocomplete.user_submitted = form.user_submitted;
+  for (size_t i = 0; i < submitted_form->field_count(); ++i) {
+    if (submitted_form->field(i)->Type().GetStorableType() ==
+        CREDIT_CARD_VERIFICATION_CODE) {
+      form_for_autocomplete.fields[i].should_autocomplete = false;
+    }
+  }
+  autocomplete_history_manager_->OnFormSubmitted(form_for_autocomplete);
 
   address_form_event_logger_->OnFormSubmitted();
   credit_card_form_event_logger_->OnFormSubmitted();
diff --git a/components/autofill/core/browser/autofill_manager.h b/components/autofill/core/browser/autofill_manager.h
index f69634a..c0696cf 100644
--- a/components/autofill/core/browser/autofill_manager.h
+++ b/components/autofill/core/browser/autofill_manager.h
@@ -474,6 +474,8 @@ class AutofillManager : public AutofillDownloadManager::Observer,
                            FormSubmittedAutocompleteEnabled);
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest,
                            AutocompleteOffRespectedForAutocomplete);
+  FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest,
+                           DontSaveCvcInAutocompleteHistory);
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest, DontOfferToSaveWalletCard);
   DISALLOW_COPY_AND_ASSIGN(AutofillManager);
 };
diff --git a/components/autofill/core/browser/autofill_manager_unittest.cc b/components/autofill/core/browser/autofill_manager_unittest.cc
index d2255cb5..cc04eb0 100644
--- a/components/autofill/core/browser/autofill_manager_unittest.cc
+++ b/components/autofill/core/browser/autofill_manager_unittest.cc
@@ -45,6 +45,7 @@
 using base::ASCIIToUTF16;
 using base::UTF8ToUTF16;
 using testing::_;
+using testing::SaveArg;
 
 namespace autofill {
 
@@ -2417,11 +2418,8 @@ TEST_F(AutofillManagerTest, FormSubmittedAutocompleteEnabled) {
   autofill_manager_.reset(
       new TestAutofillManager(autofill_driver_.get(), &client, NULL));
   autofill_manager_->set_autofill_enabled(false);
-  scoped_ptr<MockAutocompleteHistoryManager> autocomplete_history_manager;
-  autocomplete_history_manager.reset(
+  autofill_manager_->autocomplete_history_manager_.reset(
       new MockAutocompleteHistoryManager(autofill_driver_.get(), &client));
-  autofill_manager_->autocomplete_history_manager_ =
-      autocomplete_history_manager.Pass();
 
   // Set up our form data.
   FormData form;
@@ -2471,11 +2469,8 @@ TEST_F(AutofillManagerTest, AutocompleteOffRespectedForAutocomplete) {
   autofill_manager_->set_autofill_enabled(false);
   autofill_manager_->SetExternalDelegate(external_delegate_.get());
 
-  scoped_ptr<MockAutocompleteHistoryManager> autocomplete_history_manager;
-  autocomplete_history_manager.reset(
+  autofill_manager_->autocomplete_history_manager_.reset(
       new MockAutocompleteHistoryManager(autofill_driver_.get(), &client));
-  autofill_manager_->autocomplete_history_manager_ =
-      autocomplete_history_manager.Pass();
   MockAutocompleteHistoryManager* m = static_cast<
       MockAutocompleteHistoryManager*>(
           autofill_manager_->autocomplete_history_manager_.get());
@@ -3130,6 +3125,55 @@ TEST_F(AutofillManagerTest, GetCreditCardSuggestionsForNumberSpitAcrossFields) {
           "04/12", kVisaCard, autofill_manager_->GetPackedCreditCardID(4)));
 }
 
+// Test that inputs detected to be CVC inputs are forced to
+// !should_autocomplete for AutocompleteHistoryManager::OnFormSubmitted.
+TEST_F(AutofillManagerTest, DontSaveCvcInAutocompleteHistory) {
+  autofill_manager_->autocomplete_history_manager_.reset(
+      new MockAutocompleteHistoryManager(autofill_driver_.get(),
+                                         &autofill_client_));
+  FormData form_seen_by_ahm;
+  MockAutocompleteHistoryManager* mock_ahm =
+      static_cast<MockAutocompleteHistoryManager*>(
+          autofill_manager_->autocomplete_history_manager_.get());
+  EXPECT_CALL(*mock_ahm, OnFormSubmitted(_))
+      .WillOnce(SaveArg<0>(&form_seen_by_ahm));
+
+  FormData form;
+  form.name = ASCIIToUTF16("MyForm");
+  form.origin = GURL("http://myform.com/form.html");
+  form.action = GURL("http://myform.com/submit.html");
+  form.user_submitted = true;
+
+  struct {
+    const char* label;
+    const char* name;
+    const char* value;
+    ServerFieldType expected_field_type;
+  } fields[] = {
+      {"Card number", "1", "4234-5678-9012-3456", CREDIT_CARD_NUMBER},
+      {"Card verification code", "2", "123", CREDIT_CARD_VERIFICATION_CODE},
+      {"expiration date", "3", "04/2020", CREDIT_CARD_EXP_4_DIGIT_YEAR},
+  };
+
+  for (size_t i = 0; i < arraysize(fields); ++i) {
+    FormFieldData field;
+    test::CreateTestFormField(fields[i].label, fields[i].name, fields[i].value,
+                              "text", &field);
+    form.fields.push_back(field);
+  }
+
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+  FormSubmitted(form);
+
+  EXPECT_EQ(form.fields.size(), form_seen_by_ahm.fields.size());
+  ASSERT_EQ(arraysize(fields), form_seen_by_ahm.fields.size());
+  for (size_t i = 0; i < arraysize(fields); ++i) {
+    EXPECT_EQ(form_seen_by_ahm.fields[i].should_autocomplete,
+              fields[i].expected_field_type != CREDIT_CARD_VERIFICATION_CODE);
+  }
+}
+
 TEST_F(AutofillManagerTest, DontOfferToSaveWalletCard) {
   // This line silences the warning from RealPanWalletClient about matching
   // sync and wallet server types.