Move card unmask interfaces and controller into the autofill component.

Move card unmask interfaces and controller into the autofill component.

These will be used in an upcoming CL implementing the iOS version of the card unmask prompt view.

BUG=484806
Committed: https://crrev.com/982b06f6e72d19b019676c8f51c6a8f63e580c19
Cr-Commit-Position: refs/heads/master@{#330537}

[Justin Donnelly, 2015-05-13 23:49:06]

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

[Justin Donnelly, 2015-05-13 23:49:36]

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
File components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc
(right):

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc:17: //
#include "grit/theme_resources.h"
This is not accessible from within components. Could it be? Should it be?

If not, then I propose replacing GetCvcImageRid() with a new method,
IsCardAmex() and moving the ternary expression into each of the view
implementations. In other words, replace:

int cvc_image_rid = controller_->GetCvcImageRid();

with:

int cvc_image_rid = controller_->IsCardAmex()
    ? IDR_CREDIT_CARD_CVC_HINT_AMEX
    : IDR_CREDIT_CARD_CVC_HINT;

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc:233:
/*
See comment above for why this is temporarily commented out.

[Evan Stade, 2015-05-14 00:04:34]

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
File components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc
(right):

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc:17: //
#include "grit/theme_resources.h"
On 2015/05/13 23:49:36, Justin Donnelly wrote:
> This is not accessible from within components. Could it be? Should it be?
> 
> If not, then I propose replacing GetCvcImageRid() with a new method,
> IsCardAmex() and moving the ternary expression into each of the view
> implementations. In other words, replace:
> 
> int cvc_image_rid = controller_->GetCvcImageRid();
> 
> with:
> 
> int cvc_image_rid = controller_->IsCardAmex()
>     ? IDR_CREDIT_CARD_CVC_HINT_AMEX
>     : IDR_CREDIT_CARD_CVC_HINT;

just move the images to components/autofill_scaled_resources.grdp ?

[Justin Donnelly, 2015-05-14 01:27:49]

Patchset #2 (id:20001) has been deleted

[Justin Donnelly, 2015-05-14 01:29:30]

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
File components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc
(right):

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc:17: //
#include "grit/theme_resources.h"
On 2015/05/14 00:04:34, Evan Stade wrote:
> On 2015/05/13 23:49:36, Justin Donnelly wrote:
> > This is not accessible from within components. Could it be? Should it be?
> > 
> > If not, then I propose replacing GetCvcImageRid() with a new method,
> > IsCardAmex() and moving the ternary expression into each of the view
> > implementations. In other words, replace:
> > 
> > int cvc_image_rid = controller_->GetCvcImageRid();
> > 
> > with:
> > 
> > int cvc_image_rid = controller_->IsCardAmex()
> >     ? IDR_CREDIT_CARD_CVC_HINT_AMEX
> >     : IDR_CREDIT_CARD_CVC_HINT;
> 
> just move the images to components/autofill_scaled_resources.grdp ?

Ah ok, duh. Done.

https://codereview.chromium.org/1138653003/diff/1/components/autofill/core/br...
components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.cc:233:
/*
On 2015/05/13 23:49:36, Justin Donnelly wrote:
> See comment above for why this is temporarily commented out.

Fixed.

[Evan Stade, 2015-05-14 21:44:40]

i'm not an owner of everything here, but it all lgtm

https://codereview.chromium.org/1138653003/diff/50001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/create_card_unmask_prompt_view.h (right):

https://codereview.chromium.org/1138653003/diff/50001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/create_card_unmask_prompt_view.h:16: // A
cross-platform function for creating the card unmask view. This function is
nit: I'd make this comment something like "Factory function for
CardUnmaskPromptView on non-iOS platforms."

[Justin Donnelly, 2015-05-14 22:00:23]

https://codereview.chromium.org/1138653003/diff/50001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/create_card_unmask_prompt_view.h (right):

https://codereview.chromium.org/1138653003/diff/50001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/create_card_unmask_prompt_view.h:16: // A
cross-platform function for creating the card unmask view. This function is
On 2015/05/14 21:44:39, Evan Stade wrote:
> nit: I'd make this comment something like "Factory function for
> CardUnmaskPromptView on non-iOS platforms."

Done.

[Justin Donnelly, 2015-05-14 22:04:40]

jdonnelly@chromium.org changed reviewers:
+ groby@chromium.org, oshima@chromium.org, sdefresne@chromium.org,
tedchoc@chromium.org

[Justin Donnelly, 2015-05-14 22:04:41]

tedchoc for c/b/ui/android/autofill
groby for c/b/ui/cocoa/autofill
oshima for chrome/app/theme
sdefresne for components/resources

[Ted C, 2015-05-14 22:10:53]

On 2015/05/14 22:04:41, Justin Donnelly wrote:
> tedchoc for c/b/ui/android/autofill
> groby for c/b/ui/cocoa/autofill
> oshima for chrome/app/theme
> sdefresne for components/resources

c/b/ui/android/autofill - lgtm

[oshima, 2015-05-14 22:20:10]

c/a/theme lgtm

[groby-ooo-7-16, 2015-05-15 18:08:53]

c/g/ui/cocoa LGTM % nit

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:59:
CardUnmaskPromptView* CreateCardUnmaskPromptView(
Why not the static factory anymore? Isn't that our default pattern for
x-platform factories?

[Justin Donnelly, 2015-05-15 18:29:26]

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:59:
CardUnmaskPromptView* CreateCardUnmaskPromptView(
On 2015/05/15 18:08:53, groby wrote:
> Why not the static factory anymore? Isn't that our default pattern for
> x-platform factories?

This function is used on the non-iOS platforms. The static method on the View
interface can't be used anymore because that interface is moving to //components
and iOS doesn't grok WebContents.

[groby-ooo-7-16, 2015-05-15 18:41:40]

SLGTM

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm (right):

https://codereview.chromium.org/1138653003/diff/70001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm:59:
CardUnmaskPromptView* CreateCardUnmaskPromptView(
On 2015/05/15 18:29:26, Justin Donnelly wrote:
> On 2015/05/15 18:08:53, groby wrote:
> > Why not the static factory anymore? Isn't that our default pattern for
> > x-platform factories?
> 
> This function is used on the non-iOS platforms. The static method on the View
> interface can't be used anymore because that interface is moving to
//components
> and iOS doesn't grok WebContents.

This is somewhat disturbing - but certainly beyond the scope of this CL :)

[sdefresne, 2015-05-19 14:13:13]

lgtm

[Justin Donnelly, 2015-05-19 14:13:37]

The CQ bit was checked by jdonnelly@chromium.org

[Justin Donnelly, 2015-05-19 14:13:37]

The patchset sent to the CQ was uploaded after l-g-t-m from estade@chromium.org,
tedchoc@chromium.org, groby@chromium.org, oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/1138653003/#ps90001
(title: "Merge (esp for estade's new card link CL)")

[commit-bot: I haz the power, 2015-05-19 14:13:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1138653003/90001

[commit-bot: I haz the power, 2015-05-19 15:04:26]

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

[commit-bot: I haz the power, 2015-05-19 15:04:28]

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

[Justin Donnelly, 2015-05-19 15:20:33]

The CQ bit was checked by jdonnelly@chromium.org

[Justin Donnelly, 2015-05-19 15:20:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, groby@chromium.org, oshima@chromium.org,
sdefresne@chromium.org, estade@chromium.org
Link to the patchset: https://codereview.chromium.org/1138653003/#ps110001
(title: "Fix missing override declaration")

[commit-bot: I haz the power, 2015-05-19 15:20:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1138653003/110001

[commit-bot: I haz the power, 2015-05-19 17:14:28]

Committed patchset #6 (id:110001)

[commit-bot: I haz the power, 2015-05-19 17:16:03]

Patchset 6 (id:??) landed as
https://crrev.com/982b06f6e72d19b019676c8f51c6a8f63e580c19
Cr-Commit-Position: refs/heads/master@{#330537}