[Autofill] Add support for three pairs of new server field types.

[Autofill] Add support for three pairs of new server field types.

The field types included are "home" and "billing" variants of street address,
sorting code, and dependent locality.  Support for these has already been added
to the server.

BUG=299154, 280499
TEST=unit_tests
R=estade@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229340

[Evan Stade, 2013-10-16 23:18:32]

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/address.cc (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/address.cc:179: value.find(char16('\n')) ==
base::string16::npos) {
is it possible to put this check at a layer which would know whether newlines
were possible? i.e. typing a single line address into a textarea should work
because then it's not that the website was limiting.

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/field_types.h (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/field_types.h:15: // NOTE: This list MUST not
be modified except to keep it syncrhonized with the
spelling

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/field_types.h:115: // Includes of the lines of
a street address, including newlines, e.g.
missing a word?

[Ilya Sherman, 2013-10-17 01:12:35]

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/address.cc (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/address.cc:179: value.find(char16('\n')) ==
base::string16::npos) {
On 2013/10/16 23:18:33, Evan Stade wrote:
> is it possible to put this check at a layer which would know whether newlines
> were possible? i.e. typing a single line address into a textarea should work
> because then it's not that the website was limiting.

I thought about it and couldn't come up with a good way to do so.  In general,
it's o.k. to be conservative about when calls to SetInfo() will fail, as it just
means that it takes longer for valid data to be automagically aggregated.  So
while I agree that this heuristic isn't perfect, I think it's Good Enough™.

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/field_types.h (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/field_types.h:15: // NOTE: This list MUST not
be modified except to keep it syncrhonized with the
On 2013/10/16 23:18:33, Evan Stade wrote:
> spelling

Done.

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/field_types.h:115: // Includes of the lines of
a street address, including newlines, e.g.
On 2013/10/16 23:18:33, Evan Stade wrote:
> missing a word?

Done.

[Evan Stade, 2013-10-17 02:27:30]

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/address.cc (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/address.cc:179: value.find(char16('\n')) ==
base::string16::npos) {
On 2013/10/17 01:12:35, Ilya Sherman wrote:
> On 2013/10/16 23:18:33, Evan Stade wrote:
> > is it possible to put this check at a layer which would know whether
newlines
> > were possible? i.e. typing a single line address into a textarea should work
> > because then it's not that the website was limiting.
> 
> I thought about it and couldn't come up with a good way to do so.  In general,
> it's o.k. to be conservative about when calls to SetInfo() will fail, as it
just
> means that it takes longer for valid data to be automagically aggregated.  So
> while I agree that this heuristic isn't perfect, I think it's Good Enough™.

would it be too much of a hack or not work if you just appended an extra \n to
the value you extract from the textarea?

[Ilya Sherman, 2013-10-17 02:53:25]

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
File components/autofill/core/browser/address.cc (right):

https://codereview.chromium.org/27069006/diff/1/components/autofill/core/brow...
components/autofill/core/browser/address.cc:179: value.find(char16('\n')) ==
base::string16::npos) {
On 2013/10/17 02:27:30, Evan Stade wrote:
> On 2013/10/17 01:12:35, Ilya Sherman wrote:
> > On 2013/10/16 23:18:33, Evan Stade wrote:
> > > is it possible to put this check at a layer which would know whether
> newlines
> > > were possible? i.e. typing a single line address into a textarea should
work
> > > because then it's not that the website was limiting.
> > 
> > I thought about it and couldn't come up with a good way to do so.  In
general,
> > it's o.k. to be conservative about when calls to SetInfo() will fail, as it
> just
> > means that it takes longer for valid data to be automagically aggregated. 
So
> > while I agree that this heuristic isn't perfect, I think it's Good Enough™.
> 
> would it be too much of a hack or not work if you just appended an extra \n to
> the value you extract from the textarea?

Leading and trailing whitespace is trimmed off by the time the code flow reaches
here.  I guess we could special-case that at several layers, but it gets pretty
hacky.

[Evan Stade, 2013-10-17 14:57:59]

lgtm

[commit-bot: I haz the power, 2013-10-17 21:02:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/27069006/8001

[commit-bot: I haz the power, 2013-10-18 01:33:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/27069006/8001

[commit-bot: I haz the power, 2013-10-18 02:15:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/isherman@chromium.org/27069006/8001

[commit-bot: I haz the power, 2013-10-18 09:52:33]

Change committed as 229340