[Autofill] Support "importing" verified profiles and credit cards.

[Autofill] Support "importing" verified profiles and credit cards.

BUG=244672
TEST=unit tests
R=estade@chromium.org

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

[Evan Stade, 2013-06-12 20:40:16]

https://chromiumcodereview.appspot.com/16034018/diff/3001/components/autofill...
File components/autofill/browser/personal_data_manager.cc (right):

https://chromiumcodereview.appspot.com/16034018/diff/3001/components/autofill...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
nit: merged = true is a little confusing here, since you didn't actually merge
it (you dropped it).

https://chromiumcodereview.appspot.com/16034018/diff/3001/components/autofill...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
So if the old profile is verified, and the new profile is verified, you will get
2 copies of the same thing? Seems like it should be

if (old_is_verified && !new_is_verified)
  drop it
else
  merge it

[Ilya Sherman, 2013-06-13 05:41:37]

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
On 2013/06/12 20:40:17, Evan Stade wrote:
> nit: merged = true is a little confusing here, since you didn't actually merge
> it (you dropped it).

Is it better now with "merged = true;" outside of the if-else ladder, and with
the expanded comment?  If not, do you have any suggestions on how to make this
clearer?

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
On 2013/06/12 20:40:17, Evan Stade wrote:
> So if the old profile is verified, and the new profile is verified, you will
get
> 2 copies of the same thing? Seems like it should be
> 
> if (old_is_verified && !new_is_verified)
>   drop it
> else
>   merge it

Done.

[Evan Stade, 2013-06-13 17:48:29]

lgtm

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
On 2013/06/13 05:41:37, Ilya Sherman wrote:
> On 2013/06/12 20:40:17, Evan Stade wrote:
> > nit: merged = true is a little confusing here, since you didn't actually
merge
> > it (you dropped it).
> 
> Is it better now with "merged = true;" outside of the if-else ladder, and with
> the expanded comment?  If not, do you have any suggestions on how to make this
> clearer?

my suggestion would be:

// Set to true if |existing_profiles| already contains an equivalent profile.
bool matching_profile_found = false;

[Ilya Sherman, 2013-06-14 23:58:25]

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
File components/autofill/browser/personal_data_manager.cc (right):

https://codereview.chromium.org/16034018/diff/3001/components/autofill/browse...
components/autofill/browser/personal_data_manager.cc:753: merged = true;
On 2013/06/13 17:48:29, Evan Stade wrote:
> On 2013/06/13 05:41:37, Ilya Sherman wrote:
> > On 2013/06/12 20:40:17, Evan Stade wrote:
> > > nit: merged = true is a little confusing here, since you didn't actually
> merge
> > > it (you dropped it).
> > 
> > Is it better now with "merged = true;" outside of the if-else ladder, and
with
> > the expanded comment?  If not, do you have any suggestions on how to make
this
> > clearer?
> 
> my suggestion would be:
> 
> // Set to true if |existing_profiles| already contains an equivalent profile.
> bool matching_profile_found = false;

Done.

[commit-bot: I haz the power, 2013-06-14 23:58:36]

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

[commit-bot: I haz the power, 2013-06-15 03:44:21]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-06-15 18:55:42]

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

[commit-bot: I haz the power, 2013-06-15 20:33:21]

Change committed as 206617