MacViews: support Views credit card unmask prompt

MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
Committed: https://crrev.com/c14582ff083f4c907573efd58e0246083cdd0581
Cr-Commit-Position: refs/heads/master@{#395901}

[Elly Fong-Jones, 2016-05-24 16:36:49]

The CQ bit was checked by ellyjones@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-24 16:36:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2000863002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2000863002/20001

[commit-bot: I haz the power, 2016-05-24 17:30:26]

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

[commit-bot: I haz the power, 2016-05-24 17:30:27]

Dry run: This issue passed the CQ dry run.

[Elly Fong-Jones, 2016-05-24 17:34:07]

Description was changed from

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
==========

to

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
==========

[Elly Fong-Jones, 2016-05-24 17:34:08]

ellyjones@chromium.org changed reviewers:
+ sky@chromium.org

[Elly Fong-Jones, 2016-05-24 17:34:14]

sky: ptal? :)

[Evan Stade, 2016-05-24 18:01:47]

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

[Evan Stade, 2016-05-24 18:01:48]

https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc (right):

https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc:12: return new
CardUnmaskPromptViews(controller, web_contents);
imo adding a whole new file for this, especially because the gypi file needs to
explicitly exclude it on mac, is not worth it. Why not just #if
!defined(OS_MACOSX) in place?

[Elly Fong-Jones, 2016-05-24 18:08:53]

https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc (right):

https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc:12: return new
CardUnmaskPromptViews(controller, web_contents);
On 2016/05/24 18:01:48, Evan Stade wrote:
> imo adding a whole new file for this, especially because the gypi file needs
to
> explicitly exclude it on mac, is not worth it. Why not just #if
> !defined(OS_MACOSX) in place?

OS_MACOSX is true in mac_views_browser==1 builds, which need to use the full
Views stack and no Cocoa code. This file is included in mac_views_browser==1
builds but not Cocoa builds.

[Evan Stade, 2016-05-24 22:09:39]

On 2016/05/24 18:08:53, Elly Jones wrote:
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> File chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc
(right):
> 
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc:12: return
new
> CardUnmaskPromptViews(controller, web_contents);
> On 2016/05/24 18:01:48, Evan Stade wrote:
> > imo adding a whole new file for this, especially because the gypi file needs
> to
> > explicitly exclude it on mac, is not worth it. Why not just #if
> > !defined(OS_MACOSX) in place?
> 
> OS_MACOSX is true in mac_views_browser==1 builds, which need to use the full
> Views stack and no Cocoa code. This file is included in mac_views_browser==1
> builds but not Cocoa builds.

there's no macro for mac views browser? TOOLKIT_VIEWS doesn't work?

[Elly Fong-Jones, 2016-05-25 09:49:07]

On 2016/05/24 22:09:39, Evan Stade wrote:
> On 2016/05/24 18:08:53, Elly Jones wrote:
> >
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> > File chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc
> (right):
> > 
> >
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> > chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc:12: return
> new
> > CardUnmaskPromptViews(controller, web_contents);
> > On 2016/05/24 18:01:48, Evan Stade wrote:
> > > imo adding a whole new file for this, especially because the gypi file
needs
> > to
> > > explicitly exclude it on mac, is not worth it. Why not just #if
> > > !defined(OS_MACOSX) in place?
> > 
> > OS_MACOSX is true in mac_views_browser==1 builds, which need to use the full
> > Views stack and no Cocoa code. This file is included in mac_views_browser==1
> > builds but not Cocoa builds.
> 
> there's no macro for mac views browser? TOOLKIT_VIEWS doesn't work?

Nope, there is not :(. All Mac builds have TOOLKIT_VIEWS==1 and OS_MACOSX;
there's no compile-time way to tell mac_views_browser==1 apart from
mac_views_browser==0 except inside gyp vars, so generally we've been splitting
code that cares about mac_views_browser by files.

[Elly Fong-Jones, 2016-05-25 11:17:43]

sky: ping? :)

[sky, 2016-05-25 15:51:21]

LGTM

[Elly Fong-Jones, 2016-05-25 16:11:18]

The CQ bit was checked by ellyjones@chromium.org

[commit-bot: I haz the power, 2016-05-25 16:11:35]

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

[commit-bot: I haz the power, 2016-05-25 16:15:31]

Description was changed from

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
==========

to

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
==========

[commit-bot: I haz the power, 2016-05-25 16:15:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-25 16:17:06]

Description was changed from

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677
==========

to

==========
MacViews: support Views credit card unmask prompt

Much of the Views autofill code is Mac-ready. This CL also adds two shims, one
for Cocoa browsers and one for Views browsers.

BUG=605677

Committed: https://crrev.com/c14582ff083f4c907573efd58e0246083cdd0581
Cr-Commit-Position: refs/heads/master@{#395901}
==========

[commit-bot: I haz the power, 2016-05-25 16:17:09]

Patchset 2 (id:??) landed as
https://crrev.com/c14582ff083f4c907573efd58e0246083cdd0581
Cr-Commit-Position: refs/heads/master@{#395901}

[Evan Stade, 2016-05-25 21:08:25]

On 2016/05/25 09:49:07, Elly Jones wrote:
> On 2016/05/24 22:09:39, Evan Stade wrote:
> > On 2016/05/24 18:08:53, Elly Jones wrote:
> > >
> >
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> > > File chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc
> > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2000863002/diff/20001/chrome/browser/ui/views...
> > > chrome/browser/ui/views/autofill/card_unmask_prompt_views_shim.cc:12:
return
> > new
> > > CardUnmaskPromptViews(controller, web_contents);
> > > On 2016/05/24 18:01:48, Evan Stade wrote:
> > > > imo adding a whole new file for this, especially because the gypi file
> needs
> > > to
> > > > explicitly exclude it on mac, is not worth it. Why not just #if
> > > > !defined(OS_MACOSX) in place?
> > > 
> > > OS_MACOSX is true in mac_views_browser==1 builds, which need to use the
full
> > > Views stack and no Cocoa code. This file is included in
mac_views_browser==1
> > > builds but not Cocoa builds.
> > 
> > there's no macro for mac views browser? TOOLKIT_VIEWS doesn't work?
> 
> Nope, there is not :(. All Mac builds have TOOLKIT_VIEWS==1 and OS_MACOSX;
> there's no compile-time way to tell mac_views_browser==1 apart from
> mac_views_browser==0 except inside gyp vars, so generally we've been splitting
> code that cares about mac_views_browser by files.

seems like that should be pretty easy to fix, no? In BUILD.gn,

   if (mac_views_browser) {
     defines += [ "MAC_VIEWS_BROWSER=1" ]
   }