Autofill OSX: Show error message if expiration date is in the past.

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 799bd76661debeaa951b38ecfe5ce79074444a02..f5e9903252e159cbfc3b8933f3d85ff8789eb776 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
@@ -210,26 +210,16 @@ void CardUnmaskPromptViewBridge::PerformClose() {
 }
 
 - (void)updateVerifyButtonEnabled {
-  autofill::CardUnmaskPromptController* controller = bridge_->GetController();
-  DCHECK(controller);

[bondd, 2015/03/25 21:03:59]

After looking deeper into dialog close+destruction code I no longer think this
DCHECK is necessary. I'm removing it in this CL to be consistent with
-expirationDateIsValid. (It would be confusing to DCHECK in some methods and not
others.)

-
-  BOOL enable =
-      ![inputRowView_ isHidden] &&
-      controller->InputCvcIsValid(
-          base::SysNSStringToUTF16([cvcInput_ stringValue])) &&
-      (!monthPopup_ ||
-       [monthPopup_ indexOfSelectedItem] != monthPopupDefaultIndex_) &&
-      (!yearPopup_ ||
-       [yearPopup_ indexOfSelectedItem] != yearPopupDefaultIndex_);
+  BOOL enable = ![inputRowView_ isHidden] &&
+                bridge_->GetController()->InputCvcIsValid(
+                    base::SysNSStringToUTF16([cvcInput_ stringValue])) &&
+                [self expirationDateIsValid];
 
   [verifyButton_ setEnabled:enable];
 }
 
 - (void)onVerify:(id)sender {
-  autofill::CardUnmaskPromptController* controller = bridge_->GetController();
-  DCHECK(controller);

[bondd, 2015/03/25 21:03:59]

Same here.

-
-  controller->OnUnmaskResponse(
+  bridge_->GetController()->OnUnmaskResponse(
       base::SysNSStringToUTF16([cvcInput_ stringValue]),
       base::SysNSStringToUTF16([monthPopup_ titleOfSelectedItem]),
       base::SysNSStringToUTF16([yearPopup_ titleOfSelectedItem]),
@@ -240,7 +230,25 @@ void CardUnmaskPromptViewBridge::PerformClose() {
   bridge_->PerformClose();
 }
 
+- (BOOL)expirationDateIsValid {
+  if (!bridge_->GetController()->ShouldRequestExpirationDate())
+    return true;
+
+  return bridge_->GetController()->InputExpirationIsValid(
+      base::SysNSStringToUTF16([monthPopup_ titleOfSelectedItem]),
+      base::SysNSStringToUTF16([yearPopup_ titleOfSelectedItem]));
+}
+
 - (void)onExpirationDateChanged:(id)sender {
+  if ([self expirationDateIsValid]) {
+    [self setRetriableErrorMessage:base::string16()];

[bondd, 2015/03/25 21:03:59]

Open CL https://codereview.chromium.org/1038503003/ makes it so that
-setRetriableErrorMessage no longer calls -performLayout. I'll add the
-performLayout
call to this method during the rebase of whichever CL doesn't land first.

+  } else if ([monthPopup_ indexOfSelectedItem] != monthPopupDefaultIndex_ &&

[groby-ooo-7-16, 2015/03/25 21:44:58]

Why would the default month/year not be validated?

[bondd, 2015/03/26 00:42:27]

Yeah, I noticed this too. My guess is that it's because we don't want the
message to show up after the user changes one of the popups away from the
default, but hasn't had a chance to do the other one yet.
This is the same logic as in the Views version. Maybe estade@ can comment on
whether this is a feature or a bug. FWIW the behavior makes sense to me.

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

Is the default item at least not selectable? (I.e. once I picked a year, I can't
go back to "Year"?)

+             [yearPopup_ indexOfSelectedItem] != yearPopupDefaultIndex_) {
+    [self setRetriableErrorMessage:
+              l10n_util::GetStringUTF16(
+                  IDS_AUTOFILL_CARD_UNMASK_INVALID_EXPIRATION_DATE)];
+  }
+
   [self updateVerifyButtonEnabled];
 }