[Autofill] Autofill incorrectly fills credit card expiration year in 2-digit fields.

[Autofill] Autofill incorrectly fills credit card expiration year in 2-digit fields.

With current implementation the on parsing the credit card field heuristics
matches to the autofill::kExpirationYearRe and apparently it's assumes that a
4-digit credit card field (i.e. ServerFieldType CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR).

This patch considers *maxlength* attribute in addition to see whether the feild
is 2-digit input field or not.

BUG=406247
Committed: https://crrev.com/401f3492b9eaac6a3d964b7d5147f1e532fa95dc
Cr-Commit-Position: refs/heads/master@{#291734}

[Pritam Nikam, 2014-08-22 07:49:10]

Hi Ilya & Evan,
Please take a look.
Thanks!

[Evan Stade, 2014-08-22 17:30:18]

thanks, looks like a good improvement

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
File components/autofill/core/browser/credit_card_field.cc (right):

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
components/autofill/core/browser/credit_card_field.cc:109:
&credit_card_field->expiration_year_)) {
nit: could you fix the indent here

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
components/autofill/core/browser/credit_card_field.cc:111:
credit_card_field->is_two_digit_year_ = true;
I kinda think a more general way to tackle this (more robust to future changes)
would be to create a function:

bool IsTwoDigitYear() {
  return is_two_digit_year_ || (expiration_year_ && expiration_year_->max_length
== 2);
}

And use that instead of checking is_two_digit_year_. That seems like it would be
more robust to future changes or additions to the code...

Also maybe switch the member var from

  bool is_two_digit_year_;

to

  ServerFieldType exp_year_type_;

Then the function above becomes

ServerFieldType GetExpirationYearType()

I think this makes the calling site cleaner.

[Pritam Nikam, 2014-08-23 08:27:35]

Thanks Evan!
I've incorporated review comments. PTAL!

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
File components/autofill/core/browser/credit_card_field.cc (right):

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
components/autofill/core/browser/credit_card_field.cc:109:
&credit_card_field->expiration_year_)) {
On 2014/08/22 17:30:18, Evan Stade wrote:
> nit: could you fix the indent here

Done.

Applied git cl format on entire content.

https://codereview.chromium.org/493393003/diff/20001/components/autofill/core...
components/autofill/core/browser/credit_card_field.cc:111:
credit_card_field->is_two_digit_year_ = true;
+1
Added,
ServerFieldType GetExpirationYearType() const;

ServerFieldType exp_year_type_;

[Evan Stade, 2014-08-25 16:15:59]

lgtm

[Pritam Nikam, 2014-08-25 17:41:15]

The CQ bit was checked by pritam.nikam@samsung.com

[commit-bot: I haz the power, 2014-08-25 17:41:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pritam.nikam@samsung.com/493393003/60001

[commit-bot: I haz the power, 2014-08-25 18:43:50]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-25 19:15:10]

Committed patchset #2 (60001) as 1fd7f4962110b4a3cd533dd46bf15a8a44e1dda7

[commit-bot: I haz the power, 2014-09-10 02:36:37]

Patchset 2 (id:??) landed as
https://crrev.com/401f3492b9eaac6a3d964b7d5147f1e532fa95dc
Cr-Commit-Position: refs/heads/master@{#291734}