AutoFill fill billing address when credit card settings specify a billing address.

AutoFill fill billing address when credit card settings specify a billing address.

Fixes address type matching in |AddressField::AddressTypeFromText| method.  Also, extends |AutoFillManager::FillAutoFillFormData| method to fill associated billing address when filling a credit card that has billing information set in preferences.  Adds new unit test AutoFillManagerTest.FillCreditCardFormWithBilling and fixes others.

BUG=44227
TEST=AddressFieldTest.ParseOneLineAddress, \
AddressFieldTest.ParseOneLineAddressBilling, \
AddressFieldTest.ParseOneLineAddressShipping, \
AutoFillManagerTest.FillCreditCardForm, \
AutoFillManagerTest.FillCreditCardFormWithBilling, \
FormStructureTest.HeuristicsSample8, \
and manual test with everything2.html bug file.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47731

[GeorgeY, 2010-05-18 22:51:33]

LGTM

[James Hawkins, 2010-05-19 00:22:47]

http://codereview.chromium.org/2078016/diff/1/2
File chrome/browser/autofill/address_field.cc (right):

http://codereview.chromium.org/2078016/diff/1/2#newcode404
chrome/browser/autofill/address_field.cc:404: else
nit: no need for else.

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (left):

http://codereview.chromium.org/2078016/diff/1/6#oldcode389
chrome/browser/autofill/autofill_manager_unittest.cc:389:
CreateTestFormDataWithValues(&values);
What's the motivation for this change?  How is |form| being filled with values?

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/6#newcode420
chrome/browser/autofill/autofill_manager_unittest.cc:420:
CreateTestFormField("First Name", "firstname", "", "text", &field);
I'm confused: we're filling the form, but where are the values?

http://codereview.chromium.org/2078016/diff/1/6#newcode421
chrome/browser/autofill/autofill_manager_unittest.cc:421:
EXPECT_TRUE(field.StrictlyEquals(results.fields[0]));
reminder: append Hack

http://codereview.chromium.org/2078016/diff/1/7
File chrome/browser/autofill/form_structure_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/7#newcode413
chrome/browser/autofill/form_structure_unittest.cc:413:
EXPECT_EQ(ADDRESS_BILLING_LINE1, form_structure->field(2)->heuristic_type());
hmm, how did we get this wrong in the first place?

http://codereview.chromium.org/2078016/diff/1/8
File webkit/glue/form_field.cc (right):

http://codereview.chromium.org/2078016/diff/1/8#newcode62
webkit/glue/form_field.cc:62: form_control_type_ == field.form_control_type_);
Hmm, should probably add size_ == field.size_ up here as well.  That was a
mistake that it got left out.

http://codereview.chromium.org/2078016/diff/1/9
File webkit/glue/form_field.h (right):

http://codereview.chromium.org/2078016/diff/1/9#newcode38
webkit/glue/form_field.h:38: // Equality tests for identity which does not
include |value_| or |size_|.
Make sure to add a comment about the fact that this is broken and we really need
to use a field ID to determine if one field "Is" another field, and that
operator== should behave like StrictlyEquals.

[dhollowa, 2010-05-19 02:04:24]

http://codereview.chromium.org/2078016/diff/1/2
File chrome/browser/autofill/address_field.cc (right):

http://codereview.chromium.org/2078016/diff/1/2#newcode404
chrome/browser/autofill/address_field.cc:404: else
On 2010/05/19 00:22:48, James Hawkins wrote:
> nit: no need for else.

Done.

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (left):

http://codereview.chromium.org/2078016/diff/1/6#oldcode389
chrome/browser/autofill/autofill_manager_unittest.cc:389:
CreateTestFormDataWithValues(&values);
On 2010/05/19 00:22:48, James Hawkins wrote:
> What's the motivation for this change?  How is |form| being filled with
values?

The way it was working previously had |CreateTestFormField| filling in the
fields, but it really should be |FillAutoFillFormData| that fills in the form. 
So I scrapped the separate |values| object and just use the original (unfilled)
|form| to fill in with |FillAutoFillFormData|.  Using |CreateTestFormField| was
creating false positives in the tests.

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/6#newcode420
chrome/browser/autofill/autofill_manager_unittest.cc:420:
CreateTestFormField("First Name", "firstname", "", "text", &field);
On 2010/05/19 00:22:48, James Hawkins wrote:
> I'm confused: we're filling the form, but where are the values?

The values are coming from the profiles and credit cards created by the fixture.
 They're being filled in with the call to |FillAutoFillFormData| and then
verified here.

http://codereview.chromium.org/2078016/diff/1/6#newcode421
chrome/browser/autofill/autofill_manager_unittest.cc:421:
EXPECT_TRUE(field.StrictlyEquals(results.fields[0]));
On 2010/05/19 00:22:48, James Hawkins wrote:
> reminder: append Hack

Done.

http://codereview.chromium.org/2078016/diff/1/7
File chrome/browser/autofill/form_structure_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/7#newcode413
chrome/browser/autofill/form_structure_unittest.cc:413:
EXPECT_EQ(ADDRESS_BILLING_LINE1, form_structure->field(2)->heuristic_type());
On 2010/05/19 00:22:48, James Hawkins wrote:
> hmm, how did we get this wrong in the first place?

Couldn't have been us.  ;-)

http://codereview.chromium.org/2078016/diff/1/8
File webkit/glue/form_field.cc (right):

http://codereview.chromium.org/2078016/diff/1/8#newcode62
webkit/glue/form_field.cc:62: form_control_type_ == field.form_control_type_);
On 2010/05/19 00:22:48, James Hawkins wrote:
> Hmm, should probably add size_ == field.size_ up here as well.  That was a
> mistake that it got left out.

Done.

http://codereview.chromium.org/2078016/diff/1/9
File webkit/glue/form_field.h (right):

http://codereview.chromium.org/2078016/diff/1/9#newcode38
webkit/glue/form_field.h:38: // Equality tests for identity which does not
include |value_| or |size_|.
On 2010/05/19 00:22:48, James Hawkins wrote:
> Make sure to add a comment about the fact that this is broken and we really
need
> to use a field ID to determine if one field "Is" another field, and that
> operator== should behave like StrictlyEquals.

Done.

[James Hawkins, 2010-05-19 20:50:48]

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/6#newcode420
chrome/browser/autofill/autofill_manager_unittest.cc:420:
CreateTestFormField("First Name", "firstname", "", "text", &field);
On 2010/05/19 02:04:24, dhollowa wrote:
> On 2010/05/19 00:22:48, James Hawkins wrote:
> > I'm confused: we're filling the form, but where are the values?
> 
> The values are coming from the profiles and credit cards created by the
fixture.
>  They're being filled in with the call to |FillAutoFillFormData| and then
> verified here.

I mean, GetAutoFillFormDataFilledMessage should return a FormData with
FormFields that have values filled out.  These checks don't have values.

http://codereview.chromium.org/2078016/diff/1/7
File chrome/browser/autofill/form_structure_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/7#newcode413
chrome/browser/autofill/form_structure_unittest.cc:413:
EXPECT_EQ(ADDRESS_BILLING_LINE1, form_structure->field(2)->heuristic_type());
On 2010/05/19 02:04:24, dhollowa wrote:
> On 2010/05/19 00:22:48, James Hawkins wrote:
> > hmm, how did we get this wrong in the first place?
> 
> Couldn't have been us.  ;-)

What I mean to say was, can you explain how this was wrong before and what part
of the CL fixes this?

[dhollowa, 2010-05-19 21:28:06]

http://codereview.chromium.org/2078016/diff/1/6
File chrome/browser/autofill/autofill_manager_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/6#newcode420
chrome/browser/autofill/autofill_manager_unittest.cc:420:
CreateTestFormField("First Name", "firstname", "", "text", &field);
On 2010/05/19 20:50:48, James Hawkins wrote:
> On 2010/05/19 02:04:24, dhollowa wrote:
> > On 2010/05/19 00:22:48, James Hawkins wrote:
> > > I'm confused: we're filling the form, but where are the values?
> > 
> > The values are coming from the profiles and credit cards created by the
> fixture.
> >  They're being filled in with the call to |FillAutoFillFormData| and then
> > verified here.
> 
> I mean, GetAutoFillFormDataFilledMessage should return a FormData with
> FormFields that have values filled out.  These checks don't have values.

We only expect CC related fields to have values.  Note, for instance,
"nameoncard" below, has the expected value filled out.

http://codereview.chromium.org/2078016/diff/1/7
File chrome/browser/autofill/form_structure_unittest.cc (right):

http://codereview.chromium.org/2078016/diff/1/7#newcode413
chrome/browser/autofill/form_structure_unittest.cc:413:
EXPECT_EQ(ADDRESS_BILLING_LINE1, form_structure->field(2)->heuristic_type());
On 2010/05/19 20:50:48, James Hawkins wrote:
> On 2010/05/19 02:04:24, dhollowa wrote:
> > On 2010/05/19 00:22:48, James Hawkins wrote:
> > > hmm, how did we get this wrong in the first place?
> > 
> > Couldn't have been us.  ;-)
> 
> What I mean to say was, can you explain how this was wrong before and what
part
> of the CL fixes this?

Yes, this was a bug in the |AddressField::AddressTypeFromText| method.  That
code should have been matching the "billing" prefix, but wasn't.

[James Hawkins, 2010-05-19 21:34:25]

Ok, LGTM