Autofill: Add contents of CVC unmask prompt dialog on OSX.

chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm

Index: chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm
diff --git a/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm b/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm
index cc8897d4e7692583d115c363dd61d96f79105a6e..b52c1878e67dfe56af69c350f9509d4d1b29263b 100644
--- a/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm
+++ b/chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.mm
@@ -2,10 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/strings/sys_string_conversions.h"
+#include "chrome/browser/ui/autofill/autofill_dialog_models.h"
 #include "chrome/browser/ui/autofill/card_unmask_prompt_controller.h"
 #include "chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.h"
+#import "chrome/browser/ui/cocoa/autofill/autofill_pop_up_button.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_button.h"
 #include "chrome/browser/ui/chrome_style.h"
+#import "chrome/browser/ui/cocoa/constrained_window/constrained_window_control_utils.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_sheet.h"
 #import "chrome/browser/ui/cocoa/constrained_window/constrained_window_custom_window.h"
 #import "chrome/browser/ui/cocoa/key_equivalent_constants.h"
@@ -14,7 +18,11 @@
 #include "ui/base/l10n/l10n_util.h"
 
 namespace {
+
 const CGFloat kButtonGap = 6.0f;
+const CGFloat kDialogContentMinWidth = 210.0f;
+const CGFloat kCvcInputWidth = 64.0f;
+
 }  // namespace
 
 namespace autofill {
@@ -30,12 +38,16 @@ CardUnmaskPromptView* CardUnmaskPromptView::CreateAndShow(
 CardUnmaskPromptViewBridge::CardUnmaskPromptViewBridge(
     CardUnmaskPromptController* controller)
     : controller_(controller) {
-  sheet_controller_.reset([[CardUnmaskPromptViewCocoa alloc]
+  view_controller_.reset([[CardUnmaskPromptViewCocoa alloc]
       initWithWebContents:controller_->GetWebContents()
                    bridge:this]);
+
+  // Setup the constrained window that will show the view.
+  base::scoped_nsobject<NSWindow> window([[ConstrainedWindowCustomWindow alloc]
+      initWithContentRect:[[view_controller_ view] bounds]]);
+  [window setContentView:[view_controller_ view]];
   base::scoped_nsobject<CustomConstrainedWindowSheet> sheet(
-      [[CustomConstrainedWindowSheet alloc]
-          initWithCustomWindow:[sheet_controller_ window]]);
+      [[CustomConstrainedWindowSheet alloc] initWithCustomWindow:window]);
   constrained_window_.reset(
       new ConstrainedWindowMac(this, controller_->GetWebContents(), sheet));
 }
@@ -44,6 +56,8 @@ CardUnmaskPromptViewBridge::~CardUnmaskPromptViewBridge() {
 }
 
 void CardUnmaskPromptViewBridge::ControllerGone() {
+  controller_ = nullptr;
+  PerformClose();
 }
 
 void CardUnmaskPromptViewBridge::DisableAndWaitForVerification() {
@@ -55,7 +69,12 @@ void CardUnmaskPromptViewBridge::GotVerificationResult(bool success) {
 void CardUnmaskPromptViewBridge::OnConstrainedWindowClosed(
     ConstrainedWindowMac* window) {
   constrained_window_.reset();
-  controller_->OnUnmaskDialogClosed();
+  if (controller_)
+    controller_->OnUnmaskDialogClosed();
+}
+
+CardUnmaskPromptController* CardUnmaskPromptViewBridge::GetController() {
+  return controller_;
 }
 
 void CardUnmaskPromptViewBridge::PerformClose() {
@@ -68,19 +87,27 @@ void CardUnmaskPromptViewBridge::PerformClose() {
 
 @implementation CardUnmaskPromptViewCocoa
 
++ (AutofillPopUpButton*)buildDatePopupWithModel:(ui::ComboboxModel&)model {
+  AutofillPopUpButton* popup =
+      [[AutofillPopUpButton alloc] initWithFrame:NSZeroRect pullsDown:NO];
+
+  for (int i = 0; i < model.GetItemCount(); ++i) {
+    [popup addItemWithTitle:base::SysUTF16ToNSString(model.GetItemAt(i))];
+  }
+  [popup setDefaultValue:base::SysUTF16ToNSString(
+                             model.GetItemAt(model.GetDefaultIndex()))];
+  [popup sizeToFit];
+  return popup;
+}
+
 - (id)initWithWebContents:(content::WebContents*)webContents
                    bridge:(autofill::CardUnmaskPromptViewBridge*)bridge {
   DCHECK(webContents);
   DCHECK(bridge);
 
-  NSRect frame = NSMakeRect(0, 0, 550, 600);
-  base::scoped_nsobject<ConstrainedWindowCustomWindow> window(
-      [[ConstrainedWindowCustomWindow alloc] initWithContentRect:frame]);
-  if ((self = [super initWithWindow:window])) {
+  if ((self = [super initWithNibName:nil bundle:nil])) {
     webContents_ = webContents;
     bridge_ = bridge;
-
-    [self buildWindowButtons];
   }
   return self;
 }
@@ -89,38 +116,173 @@ void CardUnmaskPromptViewBridge::PerformClose() {
   bridge_->PerformClose();
 }
 
-- (void)buildWindowButtons {
-  base::scoped_nsobject<NSView> buttonContainer(
-      [[NSView alloc] initWithFrame:NSZeroRect]);
+- (void)loadView {
+  autofill::CardUnmaskPromptController* controller = bridge_->GetController();
+  DCHECK(controller);
+
+  base::scoped_nsobject<NSBox> mainView(
+      [[NSBox alloc] initWithFrame:NSZeroRect]);
+  [mainView setBoxType:NSBoxCustom];
+  [mainView setBorderType:NSNoBorder];
+  [mainView setTitlePosition:NSNoTitle];
+  [mainView
+      setContentViewMargins:NSMakeSize(chrome_style::kHorizontalPadding, 0.0)];
+
+  // Title label.
+  NSTextField* title = constrained_window::CreateLabel();
+  NSAttributedString* titleString =
+      constrained_window::GetAttributedLabelString(
+          SysUTF16ToNSString(controller->GetWindowTitle()),
+          chrome_style::kTitleFontStyle, NSNaturalTextAlignment,
+          NSLineBreakByWordWrapping);
+  [title setAttributedStringValue:titleString];
+  [title sizeToFit];
+  [mainView addSubview:title];
+
+  // Instructions label.
+  NSTextField* instructions = constrained_window::CreateLabel();
+  NSAttributedString* instructionsString =
+      constrained_window::GetAttributedLabelString(
+          SysUTF16ToNSString(controller->GetInstructionsMessage()),
+          chrome_style::kTextFontStyle, NSNaturalTextAlignment,
+          NSLineBreakByWordWrapping);
+  [instructions setAttributedStringValue:instructionsString];
+  [mainView addSubview:instructions];
+
+  // Expiration date.
+  base::scoped_nsobject<AutofillPopUpButton> monthPopup;
+  base::scoped_nsobject<AutofillPopUpButton> yearPopup;
+  NSSize expirationDateSize = NSZeroSize;
+  if (controller->ShouldRequestExpirationDate()) {
+    // Month.
+    autofill::MonthComboboxModel monthModel;
+    monthPopup.reset(
+        [CardUnmaskPromptViewCocoa buildDatePopupWithModel:monthModel]);
+    [mainView addSubview:monthPopup];
+
+    // Year.
+    autofill::YearComboboxModel yearModel;
+    yearPopup.reset(
+        [CardUnmaskPromptViewCocoa buildDatePopupWithModel:yearModel]);
+    [mainView addSubview:yearPopup];
+
+    // Dimensions.

[groby-ooo-7-16, 2015/03/03 18:06:54]

That is kind of awkward. Wrap the comboboxes in a separate view, then
expirationDateSize == [expirationDateView frame].size

Also: 
[monthPopup setFrameOrigin:NSMakePoint(0,0)]
[yearPopup setFrameOrigin:NSMakePoint(NSMaxX([monthPopup frame]) + kButtonGap,
0);
NSRect expirationSize = NSUnionRect([monthPopup frame], [yearPopup frame];
expirationSize.width += kButtonGap;
[expirationView setFrameSize:expirationSize)

This should do a way with a bunch of calculations further down.

[bondd, 2015/03/04 20:45:30]

Done.

+    expirationDateSize.width = NSWidth([monthPopup frame]) +
+                               NSWidth([yearPopup frame]) + kButtonGap * 2;
+    expirationDateSize.height =
+        std::max(NSHeight([monthPopup frame]), NSHeight([yearPopup frame]));
+  }
+
+  // CVC text input.
+  base::scoped_nsobject<NSTextField> cvcInput(
+      [[NSTextField alloc] initWithFrame:NSZeroRect]);
+  [[cvcInput cell]
+      setPlaceholderString:l10n_util::GetNSString(
+                               IDS_AUTOFILL_DIALOG_PLACEHOLDER_CVC)];
+  [[cvcInput cell] setScrollable:YES];

[groby-ooo-7-16, 2015/03/03 18:06:54]

We know the maximum size of a CVC - can we just pick a fixed size instead of
making this scrollable?

[bondd, 2015/03/04 20:45:30]

What I've got here is the same functionality as Views implementation. I'd rather
have the two versions as similar as possible until the UI designers weigh in.

[Evan Stade, 2015/03/05 02:38:02]

I'm with Rachel here. UX designers aren't going to weigh in on every detail of
this granularity. We should do the best we can on every platform. (I don't think
Views has a way of limiting the length of the input.)

[bondd, 2015/03/06 23:57:15]

Apparently it is not trivial to do in OSX either:
http://lists.apple.com/archives/cocoa-dev/2013/Oct/msg00615.html

Unless someone objects I am going to leave this for a future CL. It doesn't make
sense to me that we should hold up this CL and other basic dialog functionality
to write a custom NSFormatter at this point. There are bigger fish to fry. :)

+  [cvcInput sizeToFit];
+  [mainView addSubview:cvcInput];
 
-  base::scoped_nsobject<NSButton> button(
+  // CVC image.
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  NSImage* cvcImage =
+      rb.GetNativeImageNamed(controller->GetCvcImageRid()).ToNSImage();
+  base::scoped_nsobject<NSImageView> cvcImageView(
+      [[NSImageView alloc] initWithFrame:NSZeroRect]);
+  [cvcImageView setImage:cvcImage];
+  [mainView addSubview:cvcImageView];
+
+  // Cancel button.
+  base::scoped_nsobject<NSButton> cancelButton(
+      [[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
+  [cancelButton setTitle:l10n_util::GetNSStringWithFixup(IDS_CANCEL)];
+  [cancelButton setKeyEquivalent:kKeyEquivalentEscape];
+  [cancelButton setTarget:self];
+  [cancelButton setAction:@selector(closeSheet:)];
+  [cancelButton sizeToFit];
+  [mainView addSubview:cancelButton];
+
+  // Verify button.
+  base::scoped_nsobject<NSButton> verifyButton(
       [[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
-  [button setTitle:l10n_util::GetNSStringWithFixup(IDS_CANCEL)];
-  [button setKeyEquivalent:kKeyEquivalentEscape];
-  [button setTarget:self];
-  [button setAction:@selector(closeSheet:)];
-  [button sizeToFit];
-  [buttonContainer addSubview:button];
-
-  CGFloat nextX = NSMaxX([button frame]) + kButtonGap;
-  button.reset([[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
-  [button setFrameOrigin:NSMakePoint(nextX, 0)];
-  [button setTitle:l10n_util::GetNSStringWithFixup(
-                       IDS_AUTOFILL_DIALOG_SUBMIT_BUTTON)];
-  [button setKeyEquivalent:kKeyEquivalentReturn];
-  [button setTarget:self];
-  [button setAction:@selector(closeSheet:)];
-  [button sizeToFit];
-  [buttonContainer addSubview:button];
-
-  const CGFloat dialogOffset = NSWidth([[self window] frame]) -
-                               chrome_style::kHorizontalPadding -
-                               NSMaxX([button frame]);
-  [buttonContainer
-      setFrame:NSMakeRect(dialogOffset, chrome_style::kClientBottomPadding,
-                          NSMaxX([button frame]), NSMaxY([button frame]))];
-
-  [[[self window] contentView] addSubview:buttonContainer];
+  // TODO(bondd): use l10n string.

[groby-ooo-7-16, 2015/03/03 18:06:54]

Please do so :)

[bondd, 2015/03/04 20:45:30]

Same TODO exists in the Views implementation:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

I believe we are waiting on UI designers before we add the l10n string.

[Evan Stade, 2015/03/05 02:38:02]

right. It's less a matter of "the value of this string may change" and more a
matter of "the set of strings may change". So in many places I've punted on l10n
for now.

+  [verifyButton setTitle:@"Verify"];
+  [verifyButton setKeyEquivalent:kKeyEquivalentReturn];
+  [verifyButton setTarget:self];
+  [verifyButton setAction:@selector(closeSheet:)];
+  [verifyButton sizeToFit];
+  [mainView addSubview:verifyButton];
+
+  // Calculate input row dimensions.
+  NSSize inputRowSize;
+  inputRowSize.width = kCvcInputWidth + kButtonGap + [cvcImage size].width +

[groby-ooo-7-16, 2015/03/03 18:06:54]

You can get that width by just positioning the elements and then getting NSMaxX
- do you really need to pre-compute it before you laid out?

[bondd, 2015/03/04 20:45:30]

Done.

+                       expirationDateSize.width;
+  CGFloat maxCvcHeight =
+      std::max(NSHeight([cvcInput frame]), [cvcImage size].height);
+  inputRowSize.height = std::max(maxCvcHeight, expirationDateSize.height);
+
+  // Calculate dialog content width.
+  CGFloat contentWidth = std::max(NSWidth([title frame]), inputRowSize.width);
+  contentWidth = std::max(contentWidth, kDialogContentMinWidth);
+
+  // Layout the elements, starting at the bottom and moving up.
+
+  NSPoint pos = NSMakePoint(contentWidth, chrome_style::kClientBottomPadding);
+
+  // Verify and Cancel buttons.
+  pos.x -= NSWidth([verifyButton frame]);
+  [verifyButton setFrameOrigin:pos];
+  pos.x -= kButtonGap + NSWidth([cancelButton frame]);
+  [cancelButton setFrameOrigin:pos];
+  pos.y += NSHeight([cancelButton frame]);
+
+  // Get start position of input row.
+  pos.x = 0.0;
+  pos.y += chrome_style::kRowPadding;
+
+  // Expiration date.

[groby-ooo-7-16, 2015/03/03 18:06:54]

This entire thing becomes

[expirationView setFrameOrigin:pos];
pos.x += NSWidth([expirationView frame]);

(You can send messages to nil objects. The result is nil. Or NSZeroRect for
NSRect)

[bondd, 2015/03/04 20:45:30]

Done.

+  if (controller->ShouldRequestExpirationDate()) {
+    [monthPopup setFrame:NSMakeRect(pos.x, pos.y, NSWidth([monthPopup frame]),
+                                    inputRowSize.height)];
+    pos.x += NSWidth([monthPopup frame]) + kButtonGap;
+
+    [yearPopup setFrame:NSMakeRect(pos.x, pos.y, NSWidth([yearPopup frame]),
+                                   inputRowSize.height)];
+    pos.x += NSWidth([yearPopup frame]) + kButtonGap;
+  }
+
+  // Center cvcInput vertically in the input row.
+  CGFloat cvcInputY =
+      pos.y + ceil((inputRowSize.height - NSHeight([cvcInput frame])) * 0.5);
+  [cvcInput setFrame:NSMakeRect(pos.x, cvcInputY, kCvcInputWidth,
+                                NSHeight([cvcInput frame]))];
+  pos.x += NSWidth([cvcInput frame]) + kButtonGap;
+
+  // CVC image.
+  [cvcImageView setFrame:NSMakeRect(pos.x, pos.y, [cvcImage size].width,
+                                    inputRowSize.height)];

[groby-ooo-7-16, 2015/03/03 18:06:54]

Why not just use the original image size? And do a setFrameOrigin?

[bondd, 2015/03/04 20:45:30]

Done.

+  pos.x += NSWidth([cvcImageView frame]);
+
+  // Instruction label.
+  pos.x = 0.0;

[groby-ooo-7-16, 2015/03/03 18:06:54]

pos = NSMakePoint(0.0, NSMaxY([cvcImage frame]) + chrome_style::kRowPadding)

This expresses the relationship to the previous element a bit more directly. In
fact, you might even just pass this to setFrameOrigin.

Here and elsewhere.

[bondd, 2015/03/04 20:45:30]

Done.

+  pos.y += inputRowSize.height + chrome_style::kRowPadding;
+  [instructions setFrameOrigin:pos];
+  NSSize instructionsSize = [[instructions cell]
+      cellSizeForBounds:NSMakeRect(0.0, 0.0, contentWidth, CGFLOAT_MAX)];
+  [instructions setFrameSize:instructionsSize];
+
+  // Title label.
+  pos.y += instructionsSize.height + chrome_style::kRowPadding;

[groby-ooo-7-16, 2015/03/03 18:06:54]

NSMakePoint(0.0, NSMaxY([instructions frame] + chrome_style::kRowPadding);

[bondd, 2015/03/04 20:45:30]

Done.

+  [title setFrameOrigin:pos];
+
+  // Dialog size.
+  CGFloat dialogHeight = NSMaxY([title frame]) + chrome_style::kTitleTopPadding;

[groby-ooo-7-16, 2015/03/03 18:06:54]

Shouldn't the bottom have the same padding? In which case, the inset does this?

[bondd, 2015/03/04 20:45:30]

AFAICT the padding is different for top and bottom:
kTitleTopPadding == 15
kClientBottomPadding == 20

I'm using it the same way as this code:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

That's why I'm not using the NSBox inset for vertical spacing. Please let me
know if I'm missing something here, or if the code I'm basing this on is not the
way I should be doing this.

+
+  [mainView
+      setFrameSize:NSMakeSize(

[groby-ooo-7-16, 2015/03/03 18:06:54]

[mainView sizeToFit] should do.

[bondd, 2015/03/04 20:45:30]

See above comment - top and bottom padding are different.
I tried doing [mainView sizeToFit] and then tweaking the vertical dimensions,
but IMO it was harder to understand than what I've got here. 

+                       contentWidth + [mainView contentViewMargins].width * 2.0,
+                       dialogHeight)];
+  [self setView:mainView];
 }
 
 @end