Fix regex for last name field in Autofill.

Fix regex for last name field in Autofill.

BUG=627906
Committed: https://crrev.com/437faa0c78cfbb6bbe549f90c50adc474f2bf911
Cr-Commit-Position: refs/heads/master@{#405764}

[nicolaso, 2016-07-13 18:27:42]

nicolaso@google.com changed reviewers:
+ sebsg@chromium.org

[sebsg, 2016-07-13 18:31:43]

I know this part of the code is not tested a lot, but could you please add a
simple test case? That way we can progressively add coverage for this part of
the code. Thanks!

[nicolaso, 2016-07-13 21:03:37]

On 2016/07/13 18:31:43, sebsg wrote:
> I know this part of the code is not tested a lot, but could you please add a
> simple test case? That way we can progressively add coverage for this part of
> the code. Thanks!

Added a form to a browser test, to check that the regular expression doesn't
match things it shouldn't.

[sebsg, 2016-07-14 15:35:20]

Nice! Just a small question.

https://codereview.chromium.org/2151593002/diff/20001/chrome/test/data/autofi...
File chrome/test/data/autofill/heuristics/input/007_i18n_ko.html (right):

https://codereview.chromium.org/2151593002/diff/20001/chrome/test/data/autofi...
chrome/test/data/autofill/heuristics/input/007_i18n_ko.html:28: <!-- check that
"성명" (full name) does not accidentally match "성" (last
Can you put that one in the first form (and thus not need the other 2)? Or would
it break your test? And thanks for the comment in the HTML it is really useful!

[nicolaso, 2016-07-14 19:19:44]

https://codereview.chromium.org/2151593002/diff/20001/chrome/test/data/autofi...
File chrome/test/data/autofill/heuristics/input/007_i18n_ko.html (right):

https://codereview.chromium.org/2151593002/diff/20001/chrome/test/data/autofi...
chrome/test/data/autofill/heuristics/input/007_i18n_ko.html:28: <!-- check that
"성명" (full name) does not accidentally match "성" (last
On 2016/07/14 15:35:19, sebsg wrote:
> Can you put that one in the first form (and thus not need the other 2)? Or
would
> it break your test? And thanks for the comment in the HTML it is really
useful!

I improved the test for it to be more significant. I think it's better that this
is in a separate form, because of how we parse name fields [1]:

1. We try to find a pair of "first name" and "last name" fields.
2. If that is not found, we try to find a "full name" field.

Here, we want to check that a *pair* of first/last name fields is not detected.
If we put the "성명" in another form, the result could be influenced by the rest
of the fields in the form. It's not strictly necessary to have a separate form,
but it's more self-contained, and it makes it clearer what we're testing for.

[1]
https://cs.chromium.org/chromium/src/components/autofill/core/browser/name_fi...

[sebsg, 2016-07-15 04:14:15]

LGTM. Nice improvement. +mathp@ for ownership of autofill_regex_constants.cc

[sebsg, 2016-07-15 11:56:09]

sebsg@chromium.org changed reviewers:
+ mathp@chromium.org

[sebsg, 2016-07-15 11:56:10]

+ mathp@ for ownership of autofill_regex_constants.cc.

[Mathieu, 2016-07-15 13:11:24]

lgtm

[nicolaso, 2016-07-15 15:04:18]

The CQ bit was checked by nicolaso@google.com

[commit-bot: I haz the power, 2016-07-15 15:04:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 16:10:27]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-15 16:10:33]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 16:12:14]

Description was changed from

==========
Fix regex for last name field in Autofill.

BUG=627906
==========

to

==========
Fix regex for last name field in Autofill.

BUG=627906

Committed: https://crrev.com/437faa0c78cfbb6bbe549f90c50adc474f2bf911
Cr-Commit-Position: refs/heads/master@{#405764}
==========

[commit-bot: I haz the power, 2016-07-15 16:12:16]

Patchset 3 (id:??) landed as
https://crrev.com/437faa0c78cfbb6bbe549f90c50adc474f2bf911
Cr-Commit-Position: refs/heads/master@{#405764}