[Autofill] Fix heuristics to detect consecutive CVCs.

components/autofill/core/browser/credit_card_field.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/credit_card_field.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/memory/scoped_ptr.h"
@@ skipping 54 matching lines @@
     case CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR: {
       static int kMinimum4YearCcExpLength = strlen("12/2014");
       return max_length >= kMinimum4YearCcExpLength;
     }
     default:
       NOTREACHED();
       return false;
   }
 }
 
-
 }  // namespace
 
 // static
 scoped_ptr<FormField> CreditCardField::Parse(AutofillScanner* scanner) {
   if (scanner->IsEnd())
     return nullptr;
 
   scoped_ptr<CreditCardField> credit_card_field(new CreditCardField);
   size_t saved_cursor = scanner->SaveCursor();
 
@@ skipping 48 matching lines @@
     // We look for a card security code before we look for a credit card number
     // and match the general term "number". The security code has a plethora of
     // names; we've seen "verification #", "verification number", "card
     // identification number", and others listed in the regex pattern used
     // below.
     // Note: Some sites use type="tel" or type="number" for numerical inputs.
     const int kMatchNumAndTel = MATCH_DEFAULT | MATCH_NUMBER | MATCH_TELEPHONE;
     if (!credit_card_field->verification_ &&
         ParseFieldSpecifics(scanner,
                             base::UTF8ToUTF16(kCardCvcRe),
                             kMatchNumAndTel | MATCH_PASSWORD,

[Mathieu, 2016/03/04 18:54:38]

note: add MATCH_PASSWORD to the constant above.

[Toyama, 2016/03/04 19:29:40]

I think that makes sense, but it might introduce diffs. Because there's an extra
call to parse using just that match type. I think it'd be better to do this in a
separate CL.

[sebsg, 2016/03/09 16:38:35]

Once I rebase the other will use password also, so I'll do it then!

[sebsg, 2016/03/09 16:38:36]

Done.

                             &credit_card_field->verification_)) {
+      // A couple of sites have multiple verification codes right after an

[Toyama, 2016/03/04 19:08:49]

s/an another/another/

[sebsg, 2016/03/09 16:38:35]

Done.

+      // other. Allow the classification of these codes one by one.
+      AutofillField* cvv = credit_card_field->verification_;

[Mathieu, 2016/03/04 18:54:38]

rename to |saved_cvv|?

[Toyama, 2016/03/04 19:08:50]

const

[sebsg, 2016/03/09 16:38:35]

Done.

[sebsg, 2016/03/09 16:38:35]

Done.

+
+      // Make sure that only a verification code was found.

[Mathieu, 2016/03/04 18:54:38]

Combine the previous comment with this:

// If the verification code is the first detected field in the newly started
card, 

[Toyama, 2016/03/04 19:08:49]

s/a/one/

[sebsg, 2016/03/09 16:38:35]

Done.

[sebsg, 2016/03/09 16:38:35]

Done.

+      if (credit_card_field->numbers_.empty() &&
+          !credit_card_field->HasExpiration() &&
+          !credit_card_field->cardholder_) {
+        // Check if the previous field was a verification code.
+        scanner->RewindTo(scanner->SaveCursor() - 2);
+        if (ParseFieldSpecifics(scanner, base::UTF8ToUTF16(kCardCvcRe),
+                                kMatchNumAndTel | MATCH_PASSWORD,
+                                &credit_card_field->verification_)) {
+          // Reset the current cvv (The verification parse overwrote it).
+          credit_card_field->verification_ = cvv;
+          // Put the scanner back to the field right after the current cvv.
+          scanner->Advance();
+          return std::move(credit_card_field);

[Mathieu, 2016/03/04 18:54:38]

wait, isn't |credit_card_field| mostly empty here? Why are we returning?

[sebsg, 2016/03/09 16:38:35]

At this point it contains only a CVV. The effect of this will be that this cvv
will be classified as such and won't actually be the start of a new credit card.

+        } else {
+          // Put the scanner back to the field right after the current cvv.
+          scanner->Advance();
+          scanner->Advance();
+        }
+      }
+
       continue;
     }
 
     AutofillField* current_number_field;
     if (ParseFieldSpecifics(scanner,
                             base::UTF8ToUTF16(kCardNumberRe),
                             kMatchNumAndTel,
                             &current_number_field)) {
       // Avoid autofilling any credit card number field having very low or high
       // |start_index| on the HTML form.
@@ skipping 37 matching lines @@
   if (credit_card_field->cardholder_)
     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,
   // so we treat the card type as optional for now.
   // The existence of a number or cvc in combination with expiration date is
   // a strong enough signal that this is a credit card.  It is possible that
   // the number and name were parsed in a separate part of the form.  So if
   // the cvc and date were found independently they are returned.
   bool has_cc_number_or_verification = (credit_card_field->verification_ ||

[Toyama, 2016/03/04 19:29:40]

This is not your change, but this should be const.

[sebsg, 2016/03/09 16:38:35]

Done.

                                         !credit_card_field->numbers_.empty());
-  bool has_date_or_mm_yy = (credit_card_field->expiration_date_ ||
-                            (credit_card_field->expiration_month_ &&
-                             credit_card_field->expiration_year_));
-  if (has_cc_number_or_verification && has_date_or_mm_yy)
+  if (has_cc_number_or_verification && credit_card_field->HasExpiration())
     return std::move(credit_card_field);
 
   scanner->RewindTo(saved_cursor);
   return nullptr;
 }
 
 // static
 bool CreditCardField::LikelyCardMonthSelectField(AutofillScanner* scanner) {
   if (scanner->IsEnd())
     return false;
@@ skipping 251 matching lines @@
 }
 
 ServerFieldType CreditCardField::GetExpirationYearType() const {
   return (expiration_date_
               ? exp_year_type_
               : ((expiration_year_ && expiration_year_->max_length == 2)
                      ? CREDIT_CARD_EXP_2_DIGIT_YEAR
                      : CREDIT_CARD_EXP_4_DIGIT_YEAR));
 }
 
+bool CreditCardField::HasExpiration() const {
+  return expiration_date_ || (expiration_month_ && expiration_year_);
+}
+
 }  // namespace autofill