AutoFill Empty profiles should not be saved from AutoFillDialog Add and Edit sheets on Mac.

AutoFill Empty profiles should not be saved from AutoFillDialog Add and Edit sheets on Mac.

Changes the AutoFillDialog on Mac to not store intermediate results of "Add" and "Edit" actions
when those actions result in empty address or credit card information.  Empty addresses and credit
card information is culled at this level now, not just after "Save" from the main dialog.

BUG=47742
TEST=AutoFillDialogControllerTest.AddNewProfile, AutoFillDialogControllerTest.AddNewEmptyProfile, AutoFillDialogControllerTest.AddNewCreditCard, AutoFillDialogControllerTest.AddNewEmptyCreditCard

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

[James Hawkins, 2010-07-21 20:57:21]

http://codereview.chromium.org/3061001/diff/1/5
File chrome/browser/autofill/autofill_profile.h (right):

http://codereview.chromium.org/3061001/diff/1/5#newcode85
chrome/browser/autofill/autofill_profile.h:85: static bool IsEmptyProfile(const
AutoFillProfile& profile);
Why make this static? I prefer an |IsEmpty| method that checks if |this| is
empty.

http://codereview.chromium.org/3061001/diff/1/7
File chrome/browser/autofill/credit_card.h (right):

http://codereview.chromium.org/3061001/diff/1/7#newcode63
chrome/browser/autofill/credit_card.h:63: static bool IsEmptyCreditCard(const
CreditCard& credit_card);
Same comment about the static.

http://codereview.chromium.org/3061001/diff/1/8
File chrome/browser/autofill/personal_data_manager.cc (right):

http://codereview.chromium.org/3061001/diff/1/8#newcode223
chrome/browser/autofill/personal_data_manager.cc:223:
AutoFillProfile::IsEmptyProfile),
I now see why you made this static on AutoFillProfile; however, I still prefer
an |IsEmpty| method and changing the implementation of IsEmptyProfile/CreditCard
in this file to just return profile.IsEmpty, etc.

[dhollowa, 2010-07-21 21:01:08]

http://codereview.chromium.org/3061001/diff/1/5
File chrome/browser/autofill/autofill_profile.h (right):

http://codereview.chromium.org/3061001/diff/1/5#newcode85
chrome/browser/autofill/autofill_profile.h:85: static bool IsEmptyProfile(const
AutoFillProfile& profile);
On 2010/07/21 20:57:21, James Hawkins wrote:
> Why make this static? I prefer an |IsEmpty| method that checks if |this| is
> empty.

It needs to be static to work with stl::erase_if

http://codereview.chromium.org/3061001/diff/1/7
File chrome/browser/autofill/credit_card.h (right):

http://codereview.chromium.org/3061001/diff/1/7#newcode63
chrome/browser/autofill/credit_card.h:63: static bool IsEmptyCreditCard(const
CreditCard& credit_card);
On 2010/07/21 20:57:21, James Hawkins wrote:
> Same comment about the static.

Same reply.

[dhollowa, 2010-07-21 21:06:59]

http://codereview.chromium.org/3061001/diff/1/8
File chrome/browser/autofill/personal_data_manager.cc (right):

http://codereview.chromium.org/3061001/diff/1/8#newcode223
chrome/browser/autofill/personal_data_manager.cc:223:
AutoFillProfile::IsEmptyProfile),
On 2010/07/21 20:57:21, James Hawkins wrote:
> I now see why you made this static on AutoFillProfile; however, I still prefer
> an |IsEmpty| method and changing the implementation of
IsEmptyProfile/CreditCard
> in this file to just return profile.IsEmpty, etc.

But then I'll have to replicate same in Mac dialog code.  Seems wrong.  I prefer
as is.

[dhollowa, 2010-07-21 22:45:20]

On 2010/07/21 21:06:59, dhollowa wrote:
> http://codereview.chromium.org/3061001/diff/1/8
> File chrome/browser/autofill/personal_data_manager.cc (right):
> 
> http://codereview.chromium.org/3061001/diff/1/8#newcode223
> chrome/browser/autofill/personal_data_manager.cc:223:
> AutoFillProfile::IsEmptyProfile),
> On 2010/07/21 20:57:21, James Hawkins wrote:
> > I now see why you made this static on AutoFillProfile; however, I still
prefer
> > an |IsEmpty| method and changing the implementation of
> IsEmptyProfile/CreditCard
> > in this file to just return profile.IsEmpty, etc.
> 
> But then I'll have to replicate same in Mac dialog code.  Seems wrong.  I
prefer
> as is.

We could have our cake and eat it too.  We can use:

  std::mem_fun_ref(&Foo::empty)

as the predicate instead of the static.
Let me know if you like it.

[James Hawkins, 2010-07-21 22:46:28]

On 2010/07/21 22:45:20, dhollowa wrote:
> On 2010/07/21 21:06:59, dhollowa wrote:
> > http://codereview.chromium.org/3061001/diff/1/8
> > File chrome/browser/autofill/personal_data_manager.cc (right):
> > 
> > http://codereview.chromium.org/3061001/diff/1/8#newcode223
> > chrome/browser/autofill/personal_data_manager.cc:223:
> > AutoFillProfile::IsEmptyProfile),
> > On 2010/07/21 20:57:21, James Hawkins wrote:
> > > I now see why you made this static on AutoFillProfile; however, I still
> prefer
> > > an |IsEmpty| method and changing the implementation of
> > IsEmptyProfile/CreditCard
> > > in this file to just return profile.IsEmpty, etc.
> > 
> > But then I'll have to replicate same in Mac dialog code.  Seems wrong.  I
> prefer
> > as is.
> 
> We could have our cake and eat it too.  We can use:
> 
>   std::mem_fun_ref(&Foo::empty)
> 
> as the predicate instead of the static.
> Let me know if you like it.

Yes, that's perfect. Thanks.

[dhollowa, 2010-07-21 23:05:02]

Done.

[James Hawkins, 2010-07-21 23:14:01]

LGTM