Cleanup: roll AutofillCCInfoBarDelegate into AutofillSaveCardInfoBarDelegateMobile.

Cleanup: roll AutofillCCInfoBarDelegate into AutofillSaveCardInfoBarDelegateMobile.

The former is no longer used on desktop and therefore no longer needs to be maintained as a separate class. Depends on https://codereview.chromium.org/1761073005/ which removes the last reference.

BUG=

Committed: https://crrev.com/564a23ef69b81c62a54d1b86b843ec5f49c037a9
Cr-Commit-Position: refs/heads/master@{#380650}

[Justin Donnelly, 2016-03-08 19:05:21]

Patchset #1 (id:1) has been deleted

[Justin Donnelly, 2016-03-08 19:05:29]

Patchset #1 (id:20001) has been deleted

[Justin Donnelly, 2016-03-08 19:05:49]

Patchset #1 (id:40001) has been deleted

[Justin Donnelly, 2016-03-08 19:21:45]

jdonnelly@chromium.org changed reviewers:
+ estade@chromium.org

[Evan Stade, 2016-03-08 21:41:49]

https://codereview.chromium.org/1780443002/diff/60001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/1780443002/diff/60001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:330:
'browser/autofill/autofill_save_card_infobar_delegate_mobile_unittest.cc',
is this used by ios? if not, maybe rename s/mobile/android

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:40:
CreditCard::GetCreditCardType(card.GetRawInfo(CREDIT_CARD_NUMBER)))),
can't you just do CreditCard::IconResourceId(card.type())

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:96:
#if !defined(OS_MACOSX) && !defined(OS_IOS) && !defined(OS_ANDROID)
these defines don't seem still necessary. In fact the entire function can be
removed.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:97:
return gfx::VectorIconId::AUTOFILL;
I wonder if this is still used somewhere.. probably not.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h:45:
int GetIconId() const override;
why are some of these public and some private

[Justin Donnelly, 2016-03-08 22:41:47]

https://codereview.chromium.org/1780443002/diff/60001/chrome/chrome_tests_uni...
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/1780443002/diff/60001/chrome/chrome_tests_uni...
chrome/chrome_tests_unit.gypi:330:
'browser/autofill/autofill_save_card_infobar_delegate_mobile_unittest.cc',
On 2016/03/08 21:41:49, Evan Stade wrote:
> is this used by ios? if not, maybe rename s/mobile/android

It will be used by iOS soon.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:40:
CreditCard::GetCreditCardType(card.GetRawInfo(CREDIT_CARD_NUMBER)))),
On 2016/03/08 21:41:49, Evan Stade wrote:
> can't you just do CreditCard::IconResourceId(card.type())

Done.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:96:
#if !defined(OS_MACOSX) && !defined(OS_IOS) && !defined(OS_ANDROID)
On 2016/03/08 21:41:49, Evan Stade wrote:
> these defines don't seem still necessary. In fact the entire function can be
> removed.

Done.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:97:
return gfx::VectorIconId::AUTOFILL;
On 2016/03/08 21:41:49, Evan Stade wrote:
> I wonder if this is still used somewhere.. probably not.

Removed.

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h:45:
int GetIconId() const override;
On 2016/03/08 21:41:49, Evan Stade wrote:
> why are some of these public and some private

They were public in the old class, but didn't need to be. I've made them
private.

[Evan Stade, 2016-03-08 22:52:42]

lgtm

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h:45:
int GetIconId() const override;
On 2016/03/08 22:41:47, Justin Donnelly wrote:
> On 2016/03/08 21:41:49, Evan Stade wrote:
> > why are some of these public and some private
> 
> They were public in the old class, but didn't need to be. I've made them
> private.

typically I like to make inherited things public (or retain the parent class's
visibility) because it's somewhat deceptive to mark something as private when
the callsite is actually not part of |this|

https://codereview.chromium.org/1780443002/diff/80001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc
(right):

https://codereview.chromium.org/1780443002/diff/80001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:114:
save_card_callback_.Reset();
nit: why reset this here and not in cancel? reset in both or neither imo

[Justin Donnelly, 2016-03-09 03:46:07]

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h
(right):

https://codereview.chromium.org/1780443002/diff/60001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h:45:
int GetIconId() const override;
On 2016/03/08 22:52:42, Evan Stade wrote:
> On 2016/03/08 22:41:47, Justin Donnelly wrote:
> > On 2016/03/08 21:41:49, Evan Stade wrote:
> > > why are some of these public and some private
> > 
> > They were public in the old class, but didn't need to be. I've made them
> > private.
> 
> typically I like to make inherited things public (or retain the parent class's
> visibility) because it's somewhat deceptive to mark something as private when
> the callsite is actually not part of |this|

I've had reviewers ask me to make overridden methods private for reasons I can't
quite recall now. I agree with your rationale, so I've gone ahead and made all
these public.

https://codereview.chromium.org/1780443002/diff/80001/components/autofill/cor...
File
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc
(right):

https://codereview.chromium.org/1780443002/diff/80001/components/autofill/cor...
components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc:114:
save_card_callback_.Reset();
On 2016/03/08 22:52:42, Evan Stade wrote:
> nit: why reset this here and not in cancel? reset in both or neither imo

I've asked about this in https://codereview.chromium.org/12378055/ where this
code was introduced.

[Justin Donnelly, 2016-03-09 03:46:43]

The CQ bit was checked by jdonnelly@chromium.org

[Justin Donnelly, 2016-03-09 03:46:44]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1780443002/#ps100001
(title: "Make overriden methods public.")

[commit-bot: I haz the power, 2016-03-09 03:47:04]

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

[commit-bot: I haz the power, 2016-03-09 03:47:05]

This CL has an open dependency (Issue 1761073005 Patch 1). Please resolve the
dependency and try again.

[Justin Donnelly, 2016-03-09 18:52:18]

Patchset #4 (id:120001) has been deleted

[Justin Donnelly, 2016-03-10 19:11:10]

The CQ bit was checked by jdonnelly@chromium.org

[Justin Donnelly, 2016-03-10 19:11:10]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1780443002/#ps100001
(title: "Make overriden methods public.")

[commit-bot: I haz the power, 2016-03-10 19:11:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1780443002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1780443002/100001

[commit-bot: I haz the power, 2016-03-10 20:53:56]

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

[commit-bot: I haz the power, 2016-03-10 20:53:57]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Justin Donnelly, 2016-03-11 15:06:19]

The CQ bit was checked by jdonnelly@chromium.org

[commit-bot: I haz the power, 2016-03-11 15:06:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1780443002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1780443002/100001

[commit-bot: I haz the power, 2016-03-11 16:38:56]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-03-11 16:40:13]

Description was changed from

==========
Cleanup: roll AutofillCCInfoBarDelegate into
AutofillSaveCardInfoBarDelegateMobile.

The former is no longer used on desktop and therefore no longer needs to be
maintained as a separate class. Depends on
https://codereview.chromium.org/1761073005/ which removes the last reference.

BUG=
==========

to

==========
Cleanup: roll AutofillCCInfoBarDelegate into
AutofillSaveCardInfoBarDelegateMobile.

The former is no longer used on desktop and therefore no longer needs to be
maintained as a separate class. Depends on
https://codereview.chromium.org/1761073005/ which removes the last reference.

BUG=

Committed: https://crrev.com/564a23ef69b81c62a54d1b86b843ec5f49c037a9
Cr-Commit-Position: refs/heads/master@{#380650}
==========

[commit-bot: I haz the power, 2016-03-11 16:40:14]

Patchset 3 (id:??) landed as
https://crrev.com/564a23ef69b81c62a54d1b86b843ec5f49c037a9
Cr-Commit-Position: refs/heads/master@{#380650}