Don't suggest credit card with low match confidence

If the credit card name match was on "name", only return it as a match if there
is a matched card number and expiration or security code.

Bug: 1167977
Change-Id: I18cd0f6ee4205e16ee2a4cdba95502c0299016bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2636833
Commit-Queue: Stephen Sigwart <ssigwart@gmail.com>
Reviewed-by: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/heads/master@{#846388}

components/autofill/core/browser/form_parsing/credit_card_field.cc

@@ -24,6 +24,7 @@
 #include "components/autofill/core/browser/form_parsing/autofill_scanner.h"
 #include "components/autofill/core/browser/form_parsing/form_field.h"
 #include "components/autofill/core/common/autofill_clock.h"
+#include "components/autofill/core/common/autofill_features.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -91,6 +92,7 @@ std::unique_ptr<FormField> CreditCardField::Parse(
   auto credit_card_field = std::make_unique<CreditCardField>(log_manager);
   size_t saved_cursor = scanner->SaveCursor();
   int nb_unknown_fields = 0;
+  bool cardholder_name_match_has_low_confidence = false;
 
   const std::vector<MatchingPattern>& name_on_card_patterns =
       PatternProvider::GetInstance().GetMatchPatterns("NAME_ON_CARD",
@@ -135,6 +137,7 @@ std::unique_ptr<FormField> CreditCardField::Parse(
                      name_on_card_contextual_patterns,
                      &credit_card_field->cardholder_,
                      {log_manager, "kNameOnCardContextualRe"})) {
+        cardholder_name_match_has_low_confidence = true;
         continue;
       }
     } else if (!credit_card_field->cardholder_last_) {
@@ -277,8 +280,18 @@ std::unique_ptr<FormField> CreditCardField::Parse(
   // Some pages have a billing address field after the cardholder name field.
   // For that case, allow only just the cardholder name field.  The remaining
   // CC fields will be picked up in a following CreditCardField.
-  if (credit_card_field->cardholder_)
-    return std::move(credit_card_field);
+  if (credit_card_field->cardholder_) {
+    // If we got the cardholder name with a dangerous check, require at least a
+    // card number and one of expiration or verification fields.
+    if (!base::FeatureList::IsEnabled(
+            features::kAutofillStrictContextualCardNameConditions) ||
+        !cardholder_name_match_has_low_confidence ||
+        (!credit_card_field->numbers_.empty() &&
+         (credit_card_field->verification_ ||
+          credit_card_field->HasExpiration()))) {
+      return std::move(credit_card_field);
+    }
+  }
 
   // On some pages, the user selects a card type using radio buttons
   // (e.g. test page Apple Store Billing.html).  We can't handle that yet,

components/autofill/core/browser/form_parsing/credit_card_field_unittest.cc

@@ -11,9 +11,11 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/form_parsing/autofill_scanner.h"
 #include "components/autofill/core/common/autofill_clock.h"
+#include "components/autofill/core/common/autofill_features.h"
 #include "components/autofill/core/common/form_field_data.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -1067,4 +1069,119 @@ TEST_F(CreditCardFieldTest, ParseNonConsecutiveCvc) {
   ASSERT_TRUE(field_candidates_map_.find(cvc2) == field_candidates_map_.end());
 }
 
+TEST_F(CreditCardFieldTest, ParseCreditCardContextualNameNotCard) {
+  base::test::ScopedFeatureList enabled;
+  enabled.InitWithFeatures(
+      {features::kAutofillStrictContextualCardNameConditions}, {});
+
+  FormFieldData field;
+  field.form_control_type = "text";
+
+  field.label = ASCIIToUTF16("Account ID");
+  field.name = ASCIIToUTF16("acctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Account Name");
+  field.name = ASCIIToUTF16("name");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Move to Account ID");
+  field.name = ASCIIToUTF16("toAcctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  Parse();
+  ASSERT_EQ(nullptr, field_.get());
+}
+
+TEST_F(CreditCardFieldTest, ParseCreditCardContextualNameNotCardAcctMatch) {
+  base::test::ScopedFeatureList enabled;
+  enabled.InitWithFeatures(
+      {features::kAutofillStrictContextualCardNameConditions}, {});
+
+  FormFieldData field;
+  field.form_control_type = "text";
+
+  field.label = ASCIIToUTF16("Account ID");
+  field.name = ASCIIToUTF16("acctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Account Name");
+  field.name = ASCIIToUTF16("acctName");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Move to Account ID");
+  field.name = ASCIIToUTF16("toAcctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  Parse();
+  // TODO(crbug.com/1167977): This should be null, but waiting before changing
+  // kNameOnCardRe to use word boundaries.
+  ASSERT_NE(nullptr, field_.get());
+}
+
+TEST_F(CreditCardFieldTest, ParseCreditCardContextualNameWithExpiration) {
+  base::test::ScopedFeatureList enabled;
+  enabled.InitWithFeatures(
+      {features::kAutofillStrictContextualCardNameConditions}, {});
+
+  FormFieldData field;
+  field.form_control_type = "text";
+
+  field.label = ASCIIToUTF16("Account ID");
+  field.name = ASCIIToUTF16("acctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Account Name");
+  field.name = ASCIIToUTF16("name");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Exp Month");
+  field.name = ASCIIToUTF16("ccmonth");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Exp Year");
+  field.name = ASCIIToUTF16("ccyear");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  Parse();
+  ASSERT_NE(nullptr, field_.get());
+}
+
+TEST_F(CreditCardFieldTest, ParseCreditCardContextualNameWithVerification) {
+  base::test::ScopedFeatureList enabled;
+  enabled.InitWithFeatures(
+      {features::kAutofillStrictContextualCardNameConditions}, {});
+
+  FormFieldData field;
+  field.form_control_type = "text";
+
+  field.label = ASCIIToUTF16("Account ID");
+  field.name = ASCIIToUTF16("acctNum");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Account Name");
+  field.name = ASCIIToUTF16("name");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  field.label = ASCIIToUTF16("Verification");
+  field.name = ASCIIToUTF16("cvv");
+  field.unique_renderer_id = MakeFieldRendererId();
+  list_.push_back(std::make_unique<AutofillField>(field));
+
+  Parse();
+  ASSERT_NE(nullptr, field_.get());
+}
+
 }  // namespace autofill

components/autofill/core/common/autofill_features.cc

@@ -298,6 +298,13 @@ const base::Feature kAutofillShowTypePredictions{
 const base::Feature kAutofillSkipComparingInferredLabels{
     "AutofillSkipComparingInferredLabels", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Controls whether we require an expiration date or verification field when a
+// name field is detected for a credit card, but we aren't confident it's not
+// a non-credit card specific name field.
+const base::Feature kAutofillStrictContextualCardNameConditions{
+    "AutofillStrictContextualCardNameConditions",
+    base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Controls whether Autofill should search prefixes of all words/tokens when
 // filtering profiles, or only on prefixes of the whole string.
 const base::Feature kAutofillTokenPrefixMatching{

components/autofill/core/common/autofill_features.h

@@ -71,6 +71,7 @@ extern const base::Feature kAutofillSectionUponRedundantNameInfo;
 extern const base::Feature kAutofillServerCommunication;
 extern const base::Feature kAutofillShowTypePredictions;
 extern const base::Feature kAutofillSkipComparingInferredLabels;
+extern const base::Feature kAutofillStrictContextualCardNameConditions;
 extern const base::Feature kAutofillTokenPrefixMatching;
 extern const base::Feature kAutofillUploadThrottling;
 extern const base::Feature kAutofillUseAlternativeStateNameMap;