Autofill OSX: Add RetriableErrorMessage text label.

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

 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #include "chrome/browser/ui/autofill/autofill_dialog_models.h"
+#include "chrome/browser/ui/autofill/autofill_dialog_types.h"
 #include "chrome/browser/ui/autofill/card_unmask_prompt_controller.h"
 #import "chrome/browser/ui/cocoa/autofill/autofill_tooltip_controller.h"
 #include "chrome/browser/ui/cocoa/autofill/card_unmask_prompt_view_bridge.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"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
+#include "skia/ext/skia_utils_mac.h"
 #include "ui/base/cocoa/window_size_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
 const CGFloat kButtonGap = 6.0f;
 const CGFloat kDialogContentMinWidth = 210.0f;
 const CGFloat kCvcInputWidth = 64.0f;
 const ui::ResourceBundle::FontStyle kProgressFontStyle =
     chrome_style::kTitleFontStyle;
+const ui::ResourceBundle::FontStyle kErrorFontStyle =
+    chrome_style::kTextFontStyle;
 
 }  // namespace
 
 namespace autofill {
 
 // static
 CardUnmaskPromptView* CardUnmaskPromptView::CreateAndShow(
     CardUnmaskPromptController* controller) {
   return new CardUnmaskPromptViewBridge(controller);
 }
@@ skipping 37 matching lines @@
     [view_controller_ setProgressOverlayText:
                           l10n_util::GetStringUTF16(
                               IDS_AUTOFILL_CARD_UNMASK_VERIFICATION_SUCCESS)];
 
     base::MessageLoop::current()->PostDelayedTask(
         FROM_HERE, base::Bind(&CardUnmaskPromptViewBridge::PerformClose,
                               weak_ptr_factory_.GetWeakPtr()),
         base::TimeDelta::FromSeconds(1));
   } else {
     [view_controller_ setProgressOverlayText:base::string16()];
+    // TODO(bondd): Views version never hides |errorLabel_|. When Views decides
+    // when to hide it then do the same thing here.
+    [view_controller_ setRetriableErrorMessage:error_message];

[bondd, 2015/03/23 21:27:55]

setRetriableErrorMessage is same method name as in Views implementation, for
consistency.

[groby-ooo-7-16, 2015/03/24 01:26:49]

Can we fix the views version too, then?

[bondd, 2015/03/24 23:46:47]

Leaving shed this color per discussion elsewhere in this patch set's review
comments. :)

   }
 }
 
 void CardUnmaskPromptViewBridge::OnConstrainedWindowClosed(
     ConstrainedWindowMac* window) {
   if (controller_)
     controller_->OnUnmaskDialogClosed();
   base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
 }
 
 CardUnmaskPromptController* CardUnmaskPromptViewBridge::GetController() {
   return controller_;
 }
 
 void CardUnmaskPromptViewBridge::PerformClose() {
   constrained_window_->CloseWebContentsModalDialog();
 }
 
 }  // autofill
 
 #pragma mark CardUnmaskPromptViewCocoa
 
 @implementation CardUnmaskPromptViewCocoa {
+  base::scoped_nsobject<NSView> expirationView_;

[groby-ooo-7-16, 2015/03/24 01:26:49]

This is never used outside -loadView, so no need to make it an ivar.

[bondd, 2015/03/24 23:46:47]

Done.

+  base::scoped_nsobject<NSView> inputRowView_;
+  base::scoped_nsobject<NSView> storageView_;
+
+  base::scoped_nsobject<NSTextField> titleLabel_;
+  base::scoped_nsobject<NSTextField> instructionsLabel_;
   base::scoped_nsobject<NSTextField> cvcInput_;
   base::scoped_nsobject<NSPopUpButton> monthPopup_;
   base::scoped_nsobject<NSPopUpButton> yearPopup_;
+  base::scoped_nsobject<NSButton> cancelButton_;
   base::scoped_nsobject<NSButton> verifyButton_;
   base::scoped_nsobject<NSButton> storageCheckbox_;
   base::scoped_nsobject<AutofillTooltipController> storageTooltip_;
-  base::scoped_nsobject<NSView> inputRow_;
-  base::scoped_nsobject<NSTextField> progressOverlayText_;
+  base::scoped_nsobject<NSTextField> errorLabel_;
+  base::scoped_nsobject<NSTextField> progressOverlayLabel_;
 
   int monthPopupDefaultIndex_;
   int yearPopupDefaultIndex_;
 
   // Owns |self|.
   autofill::CardUnmaskPromptViewBridge* bridge_;
 }
 
 + (NSPopUpButton*)buildDatePopupWithModel:(ui::ComboboxModel&)model {
   NSPopUpButton* popup =
@@ skipping 33 matching lines @@
 
   return self;
 }
 
 - (void)setProgressOverlayText:(const base::string16&)text {
   if (!text.empty()) {
     NSAttributedString* attributedString =
         constrained_window::GetAttributedLabelString(
             SysUTF16ToNSString(text), kProgressFontStyle, NSCenterTextAlignment,
             NSLineBreakByWordWrapping);
-    [progressOverlayText_ setAttributedStringValue:attributedString];
+    [progressOverlayLabel_ setAttributedStringValue:attributedString];
   }
 
-  [progressOverlayText_ setHidden:text.empty()];
-  [inputRow_ setHidden:!text.empty()];
+  [progressOverlayLabel_ setHidden:text.empty()];
+  [inputRowView_ setHidden:!text.empty()];
   [self updateVerifyButtonEnabled];
 }
 
+- (void)setRetriableErrorMessage:(const base::string16&)text {
+  NSAttributedString* attributedString =
+      constrained_window::GetAttributedLabelString(
+          SysUTF16ToNSString(text), kErrorFontStyle, NSNaturalTextAlignment,
+          NSLineBreakByWordWrapping);
+  [errorLabel_ setAttributedStringValue:attributedString];
+  [self performLayout];
+}
+
 - (void)updateVerifyButtonEnabled {
   autofill::CardUnmaskPromptController* controller = bridge_->GetController();
   DCHECK(controller);
 
   BOOL enable =
-      ![inputRow_ isHidden] &&
+      ![inputRowView_ isHidden] &&
       controller->InputCvcIsValid(
           base::SysNSStringToUTF16([cvcInput_ stringValue])) &&
       (!monthPopup_ ||
        [monthPopup_ indexOfSelectedItem] != monthPopupDefaultIndex_) &&
       (!yearPopup_ ||
        [yearPopup_ indexOfSelectedItem] != yearPopupDefaultIndex_);
 
   [verifyButton_ setEnabled:enable];
 }
 
@@ skipping 40 matching lines @@
   // "?" icon with tooltip.
   storageTooltip_.reset([[AutofillTooltipController alloc]
       initWithArrowLocation:info_bubble::kTopRight]);
   [storageTooltip_ setImage:ui::ResourceBundle::GetSharedInstance()
                                 .GetNativeImageNamed(IDR_AUTOFILL_TOOLTIP_ICON)
                                 .ToNSImage()];
   [storageTooltip_
       setMessage:base::SysUTF16ToNSString(l10n_util::GetStringUTF16(
                      IDS_AUTOFILL_CARD_UNMASK_PROMPT_STORAGE_TOOLTIP))];
   [view addSubview:[storageTooltip_ view]];
-  [[storageTooltip_ view]
-      setFrameOrigin:NSMakePoint(NSMaxX([storageCheckbox_ frame]) + kButtonGap,
-                                 0)];
+  [[storageTooltip_ view] setFrameOrigin:
+      NSMakePoint(NSMaxX([storageCheckbox_ frame]) + kButtonGap, 0)];
 
   [CardUnmaskPromptViewCocoa sizeToFitView:view];
   [CardUnmaskPromptViewCocoa verticallyCenterSubviewsInView:view];
   return view;
 }
 
+- (void)performLayout {
+  // Calculate dialog content width.
+  CGFloat contentWidth =
+      std::max(NSWidth([titleLabel_ frame]), NSWidth([inputRowView_ frame]));
+  contentWidth = std::max(contentWidth, NSWidth([storageView_ frame]));
+  contentWidth = std::max(contentWidth, kDialogContentMinWidth);

[groby-ooo-7-16, 2015/03/24 01:26:49]

This is weird to me - we only invoke this after init, or when the error message
has changed. And yet, the frame size is not dependent on the error message?

[bondd, 2015/03/24 23:46:47]

Dialog width stays the same, but dialog height changes depending on error
message.
See [[self view] setFrameSize:...] near the bottom of this method.

I could calculate contentWidth in -loadView, but it makes more logical sense to
put it in -performLayout, IMO. Saves keeping it in an ivar too.

+
+  [storageView_

[groby-ooo-7-16, 2015/03/24 01:26:49]

At this point, the layout is sufficiently complex that a small ASCII diagram
would be awesome. (Disregard if subject to frequent changes for now, but please
let's make sure that's in here before we're done. TODO, if you must)

[bondd, 2015/03/24 23:46:47]

Done.

+      setFrameOrigin:NSMakePoint(0, chrome_style::kClientBottomPadding)];
+
+  [verifyButton_ setFrameOrigin:
+      NSMakePoint(contentWidth - NSWidth([verifyButton_ frame]),
+                  NSMaxY([storageView_ frame]) + chrome_style::kRowPadding)];
+
+  [cancelButton_
+      setFrameOrigin:NSMakePoint(NSMinX([verifyButton_ frame]) - kButtonGap -
+                                     NSWidth([cancelButton_ frame]),
+                                 NSMinY([verifyButton_ frame]))];
+
+  // Error message label.

[groby-ooo-7-16, 2015/03/24 01:26:49]

Please do use a complete sentence, here and elsewhere. Even if it's just "Lay
out error message label."

[bondd, 2015/03/24 23:46:47]

Done. Removed some of these comments because code is easier to read and
self-explanatory now that -cellSizeForBounds has been pulled out into its own
method. Turned remaining comments into sentences.

+  [errorLabel_ setFrameOrigin:NSMakePoint(0, NSMaxY([cancelButton_ frame]) +
+                                                 chrome_style::kRowPadding)];
+  NSSize errorSize = [[errorLabel_ cell]
+      cellSizeForBounds:NSMakeRect(0, 0, contentWidth, CGFLOAT_MAX)];

[groby-ooo-7-16, 2015/03/24 01:26:50]

We do this in several places - probably worth hoisting into base

[bondd, 2015/03/24 23:46:47]

I hoisted it to the top of this class, with a TODO to move it to base. IMO
moving it to base right now (and writing associated unit test) has too much
potential to distract from M43 branch deadline target. 

+  [errorLabel_ setFrameSize:errorSize];
+
+  // Input row.
+  [inputRowView_ setFrameOrigin:NSMakePoint(0, NSMaxY([errorLabel_ frame]) +
+                                                   chrome_style::kRowPadding)];
+
+  // Instruction label.
+  [instructionsLabel_
+      setFrameOrigin:NSMakePoint(0, NSMaxY([inputRowView_ frame]) +
+                                        chrome_style::kRowPadding)];
+  NSSize instructionsSize = [[instructionsLabel_ cell]
+      cellSizeForBounds:NSMakeRect(0, 0, contentWidth, CGFLOAT_MAX)];
+  [instructionsLabel_ setFrameSize:instructionsSize];
+
+  // Title label.
+  [titleLabel_
+      setFrameOrigin:NSMakePoint(0, NSMaxY([instructionsLabel_ frame]) +
+                                        chrome_style::kRowPadding)];
+
+  // Center progressOverlayLabel_ vertically within inputRowView_ frame.
+  CGFloat progressHeight = ui::ResourceBundle::GetSharedInstance()
+                               .GetFont(kProgressFontStyle)
+                               .GetHeight();
+  [progressOverlayLabel_
+      setFrame:NSMakeRect(0, ceil(NSMidY([inputRowView_ frame]) -
+                                  progressHeight / 2.0),
+                          contentWidth, progressHeight)];
+
+  // Dialog size.
+  [[self view]
+      setFrameSize:NSMakeSize(
+                       contentWidth + chrome_style::kHorizontalPadding * 2.0,
+                       NSMaxY([titleLabel_ frame]) +
+                           chrome_style::kTitleTopPadding)];
+
+  NSRect frameRect =
+      [[[self view] window] frameRectForContentRect:[[self view] frame]];
+  [[[self view] window] setFrame:frameRect display:YES];
+}
+
 - (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)];
 
-  inputRow_.reset([[NSView alloc] initWithFrame:NSZeroRect]);
-  [mainView addSubview:inputRow_];
+  inputRowView_.reset([[NSView alloc] initWithFrame:NSZeroRect]);
+  [mainView addSubview:inputRowView_];
 
-  base::scoped_nsobject<NSView> storageView(
-      [self createStorageViewWithController:controller]);
-  [mainView addSubview:storageView];
+  storageView_ = [self createStorageViewWithController:controller];
+  [mainView addSubview:storageView_];
+
+  progressOverlayLabel_.reset([constrained_window::CreateLabel() retain]);
+  [progressOverlayLabel_ setHidden:YES];
+  [mainView addSubview:progressOverlayLabel_];
 
   // Title label.
-  NSTextField* title = constrained_window::CreateLabel();
+  titleLabel_.reset([constrained_window::CreateLabel() retain]);
   NSAttributedString* titleString =
       constrained_window::GetAttributedLabelString(
           SysUTF16ToNSString(controller->GetWindowTitle()),
           chrome_style::kTitleFontStyle, NSNaturalTextAlignment,
           NSLineBreakByWordWrapping);
-  [title setAttributedStringValue:titleString];
-  [title sizeToFit];
-  [mainView addSubview:title];
+  [titleLabel_ setAttributedStringValue:titleString];
+  [titleLabel_ sizeToFit];
+  [mainView addSubview:titleLabel_];
 
   // Instructions label.
-  NSTextField* instructions = constrained_window::CreateLabel();
+  instructionsLabel_.reset([constrained_window::CreateLabel() retain]);
   NSAttributedString* instructionsString =
       constrained_window::GetAttributedLabelString(
           SysUTF16ToNSString(controller->GetInstructionsMessage()),
           chrome_style::kTextFontStyle, NSNaturalTextAlignment,
           NSLineBreakByWordWrapping);
-  [instructions setAttributedStringValue:instructionsString];
-  [mainView addSubview:instructions];
+  [instructionsLabel_ setAttributedStringValue:instructionsString];
+  [mainView addSubview:instructionsLabel_];
 
   // Expiration date.
-  base::scoped_nsobject<NSView> expirationView;
   if (controller->ShouldRequestExpirationDate()) {
-    expirationView.reset([[NSView alloc] initWithFrame:NSZeroRect]);
+    expirationView_.reset([[NSView alloc] initWithFrame:NSZeroRect]);
 
     // Month.
     autofill::MonthComboboxModel monthModel;
     monthPopupDefaultIndex_ = monthModel.GetDefaultIndex();
     monthPopup_.reset(
         [CardUnmaskPromptViewCocoa buildDatePopupWithModel:monthModel]);
     [monthPopup_ setTarget:self];
     [monthPopup_ setAction:@selector(onExpirationDateChanged:)];
-    [expirationView addSubview:monthPopup_];
+    [expirationView_ addSubview:monthPopup_];
 
     // Year.
     autofill::YearComboboxModel yearModel;
     yearPopupDefaultIndex_ = yearModel.GetDefaultIndex();
     yearPopup_.reset(
         [CardUnmaskPromptViewCocoa buildDatePopupWithModel:yearModel]);
     [yearPopup_ setTarget:self];
     [yearPopup_ setAction:@selector(onExpirationDateChanged:)];
-    [expirationView addSubview:yearPopup_];
+    [expirationView_ addSubview:yearPopup_];
 
-    // Layout month and year within expirationView.
+    // Layout month and year within expirationView_.
     [yearPopup_
         setFrameOrigin:NSMakePoint(NSMaxX([monthPopup_ frame]) + kButtonGap,
                                    0)];
     NSRect expirationFrame =
         NSUnionRect([monthPopup_ frame], [yearPopup_ frame]);
     expirationFrame.size.width += kButtonGap;
-    [expirationView setFrame:expirationFrame];
-    [inputRow_ addSubview:expirationView];
+    [expirationView_ setFrame:expirationFrame];
+    [inputRowView_ addSubview:expirationView_];
   }
 
   // CVC text input.
   cvcInput_.reset([[NSTextField alloc] initWithFrame:NSZeroRect]);
   [[cvcInput_ cell]
       setPlaceholderString:l10n_util::GetNSString(
                                IDS_AUTOFILL_DIALOG_PLACEHOLDER_CVC)];
   [[cvcInput_ cell] setScrollable:YES];
   [cvcInput_ setDelegate:self];
   [cvcInput_ sizeToFit];
-  [cvcInput_ setFrame:NSMakeRect(NSMaxX([expirationView frame]), 0,
+  [cvcInput_ setFrame:NSMakeRect(NSMaxX([expirationView_ frame]), 0,
                                  kCvcInputWidth, NSHeight([cvcInput_ frame]))];
-  [inputRow_ addSubview:cvcInput_];
+  [inputRowView_ addSubview:cvcInput_];
 
   // 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];
   [cvcImageView setFrameSize:[cvcImage size]];
   [cvcImageView
       setFrameOrigin:NSMakePoint(NSMaxX([cvcInput_ frame]) + kButtonGap, 0)];
-  [inputRow_ addSubview:cvcImageView];
+  [inputRowView_ addSubview:cvcImageView];
+
+  // Error message label.
+  errorLabel_.reset([constrained_window::CreateLabel() retain]);
+  [errorLabel_
+      setTextColor:gfx::SkColorToCalibratedNSColor(autofill::kWarningColor)];
+  [mainView addSubview:errorLabel_];
 
   // Cancel button.
-  base::scoped_nsobject<NSButton> cancelButton(
+  cancelButton_.reset(
       [[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
-  [cancelButton setTitle:l10n_util::GetNSStringWithFixup(IDS_CANCEL)];
-  [cancelButton setKeyEquivalent:kKeyEquivalentEscape];
-  [cancelButton setTarget:self];
-  [cancelButton setAction:@selector(onCancel:)];
-  [cancelButton sizeToFit];
-  [mainView addSubview:cancelButton];
+  [cancelButton_ setTitle:l10n_util::GetNSStringWithFixup(IDS_CANCEL)];
+  [cancelButton_ setKeyEquivalent:kKeyEquivalentEscape];
+  [cancelButton_ setTarget:self];
+  [cancelButton_ setAction:@selector(onCancel:)];
+  [cancelButton_ sizeToFit];
+  [mainView addSubview:cancelButton_];
 
   // Verify button.
   verifyButton_.reset(
       [[ConstrainedWindowButton alloc] initWithFrame:NSZeroRect]);
   // TODO(bondd): use l10n string.
   [verifyButton_ setTitle:@"Verify"];
   [verifyButton_ setKeyEquivalent:kKeyEquivalentReturn];
   [verifyButton_ setTarget:self];
   [verifyButton_ setAction:@selector(onVerify:)];
   [verifyButton_ sizeToFit];
   [self updateVerifyButtonEnabled];
   [mainView addSubview:verifyButton_];
 
-  // Layout inputRow_.
-  [CardUnmaskPromptViewCocoa sizeToFitView:inputRow_];
-  [CardUnmaskPromptViewCocoa verticallyCenterSubviewsInView:inputRow_];
-
-  // Calculate dialog content width.
-  CGFloat contentWidth =
-      std::max(NSWidth([title frame]), NSWidth([inputRow_ frame]));
-  contentWidth = std::max(contentWidth, NSWidth([storageView frame]));
-  contentWidth = std::max(contentWidth, kDialogContentMinWidth);
-
-  // Layout mainView contents, starting at the bottom and moving up.
-
-  [storageView
-      setFrameOrigin:NSMakePoint(0, chrome_style::kClientBottomPadding)];
-
-  // Verify and Cancel buttons.
-  [verifyButton_
-      setFrameOrigin:NSMakePoint(contentWidth - NSWidth([verifyButton_ frame]),
-                                 NSMaxY([storageView frame]) +
-                                     chrome_style::kRowPadding)];
-
-  [cancelButton
-      setFrameOrigin:NSMakePoint(NSMinX([verifyButton_ frame]) - kButtonGap -
-                                     NSWidth([cancelButton frame]),
-                                 NSMinY([verifyButton_ frame]))];
-
-  // Input row.
-  [inputRow_ setFrameOrigin:NSMakePoint(0, NSMaxY([cancelButton frame]) +
-                                               chrome_style::kRowPadding)];
-
-  // Instruction label.
-  [instructions setFrameOrigin:NSMakePoint(0, NSMaxY([inputRow_ frame]) +
-                                                  chrome_style::kRowPadding)];
-  NSSize instructionsSize = [[instructions cell]
-      cellSizeForBounds:NSMakeRect(0, 0, contentWidth, CGFLOAT_MAX)];
-  [instructions setFrameSize:instructionsSize];
-
-  // Title label.
-  [title setFrameOrigin:NSMakePoint(0, NSMaxY([instructions frame]) +
-                                           chrome_style::kRowPadding)];
-
-  // Dialog size.
-  [mainView
-      setFrameSize:NSMakeSize(
-                       contentWidth + [mainView contentViewMargins].width * 2.0,
-                       NSMaxY([title frame]) + chrome_style::kTitleTopPadding)];
-
-  // Add progress overlay.
-  progressOverlayText_.reset([constrained_window::CreateLabel() retain]);
-  CGFloat progressHeight = ui::ResourceBundle::GetSharedInstance()
-                               .GetFont(kProgressFontStyle)
-                               .GetHeight();
-  // Center the text vertically within inputRow_ frame.
-  [progressOverlayText_ setFrame:NSMakeRect(0, ceil(NSMidY([inputRow_ frame]) -
-                                                    progressHeight / 2.0),
-                                            contentWidth, progressHeight)];
-  [progressOverlayText_ setHidden:YES];
-  [mainView addSubview:progressOverlayText_];
+  // Layout inputRowView_.
+  [CardUnmaskPromptViewCocoa sizeToFitView:inputRowView_];
+  [CardUnmaskPromptViewCocoa verticallyCenterSubviewsInView:inputRowView_];
 
   [self setView:mainView];
+  [self performLayout];

[groby-ooo-7-16, 2015/03/24 01:26:49]

At this point -loadView will automatically display the window. Is that
intentional?

[bondd, 2015/03/24 23:46:46]

Done.

 }
 
 @end