Fix profile comparison re: address lines.

Fix profile comparison re: address lines.

BUG=375545
TEST=try to edit an address to add address line 3 in chrome://settings

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

[Dan Beam, 2014-06-16 19:30:20]

https://codereview.chromium.org/332213004/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_profile_unittest.cc (right):

https://codereview.chromium.org/332213004/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_profile_unittest.cc:660: NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL);
what kind of pointer do you want? we have NULL and NULL... [1]

[1] https://www.youtube.com/watch?v=anwy2MPT5RE

https://codereview.chromium.org/332213004/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_profile_unittest.cc:667: EXPECT_GT(0,
a.Compare(b));
nit: EXPECT_NE(0,

[Dan Beam, 2014-06-16 19:30:24]

lgtm

[Evan Stade, 2014-06-16 19:33:03]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2014-06-16 19:33:09]

https://codereview.chromium.org/332213004/diff/1/components/autofill/core/bro...
File components/autofill/core/browser/autofill_profile_unittest.cc (right):

https://codereview.chromium.org/332213004/diff/1/components/autofill/core/bro...
components/autofill/core/browser/autofill_profile_unittest.cc:667: EXPECT_GT(0,
a.Compare(b));
On 2014/06/16 19:30:20, Dan Beam wrote:
> nit: EXPECT_NE(0, 

I kind of agree, but I'm matching the format of the other tests.

[commit-bot: I haz the power, 2014-06-16 19:35:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/332213004/1

[commit-bot: I haz the power, 2014-06-17 00:12:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-17 00:12:28]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[Evan Stade, 2014-06-17 23:33:01]

+aruslan

Had to delete a test. I don't think the test's desired behavior is correct. If
two profiles differ in the street address, they are not merged. It only passed
before because the difference came after the second line of the street address.

[aruslan, 2014-06-17 23:39:37]

On 2014/06/17 23:33:01, Evan Stade wrote:
> +aruslan
> 
> Had to delete a test. I don't think the test's desired behavior is correct. If
> two profiles differ in the street address, they are not merged. It only passed
> before because the difference came after the second line of the street
address.

That's correct -- we want to have two unmerged profiles.
The test had to be incorrect as it was waiting for http://crbug.com/375545 to
be fixed.
If it is at all possible I'd ask to fix the test instead of deleting it.

Basically, replace the Last 6 lines with these 3, and it should pass:
String profileGuid2 = assertEquals(2, mHelper.getNumberOfProfiles());	
AutofillProfile storedProfile2 = mHelper.getProfile(profileGuid2);	
assertEquals(streetAddress2, storedProfile2.getStreetAddress());

[Evan Stade, 2014-06-17 23:52:25]

On 2014/06/17 23:39:37, aruslan wrote:
> On 2014/06/17 23:33:01, Evan Stade wrote:
> > +aruslan
> > 
> > Had to delete a test. I don't think the test's desired behavior is correct.
If
> > two profiles differ in the street address, they are not merged. It only
passed
> > before because the difference came after the second line of the street
> address.
> 
> That's correct -- we want to have two unmerged profiles.
> The test had to be incorrect as it was waiting for http://crbug.com/375545 to
> be fixed.
> If it is at all possible I'd ask to fix the test instead of deleting it.
> 
> Basically, replace the Last 6 lines with these 3, and it should pass:
> String profileGuid2 = assertEquals(2, mHelper.getNumberOfProfiles());	
> AutofillProfile storedProfile2 = mHelper.getProfile(profileGuid2);	
> assertEquals(streetAddress2, storedProfile2.getStreetAddress());

sure, can do

[Evan Stade, 2014-06-17 23:56:30]

updated

[aruslan, 2014-06-17 23:58:38]

lgtm thanks!

[Evan Stade, 2014-06-18 00:00:48]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2014-06-18 00:02:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/estade@chromium.org/332213004/40001

[commit-bot: I haz the power, 2014-06-18 12:59:20]

Change committed as 278044