[Payments] Have expiration date be on the same line in CC editor

chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc

Index: chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc b/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
index 09fa367985cc440e241a150383822593e2496e2b..61ea90f09229999bbb5828918b69da5ebc503fd2 100644
--- a/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
+++ b/chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc
@@ -87,6 +87,56 @@ std::vector<base::string16> GetExpirationYearItems() {
   return years;
 }
 
+// Validates the two comboboxes used for expiration date.
+class ExpirationDateValidationDelegate : public ValidationDelegate {
+ public:
+  ExpirationDateValidationDelegate(EditorViewController* controller,
+                                   const std::string& app_locale)
+      : controller_(controller), app_locale_(app_locale) {}
+
+  // Only the delegate knows how to validate the textfield.
+  bool ValidateTextfield(views::Textfield* textfield) override { return true; }
+
+  bool ValidateCombobox(views::Combobox* combobox) override {
+    // Get the combined date from the month and year dropdowns.
+    views::View* view_parent = combobox->parent();
+
+    views::Combobox* month_combobox = static_cast<views::Combobox*>(
+        view_parent->GetViewByID(autofill::CREDIT_CARD_EXP_MONTH));
+    base::string16 month =
+        month_combobox->model()->GetItemAt(month_combobox->selected_index());
+
+    views::Combobox* year_combobox = static_cast<views::Combobox*>(
+        view_parent->GetViewByID(autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR));
+    base::string16 year =
+        year_combobox->model()->GetItemAt(year_combobox->selected_index());
+
+    autofill::CreditCard card;
+    card.SetExpirationMonthFromString(month, app_locale_);
+    card.SetExpirationYearFromString(year);
+
+    bool is_expired = card.IsExpired(autofill::AutofillClock::Now());
+    controller_->DisplayErrorMessageForField(
+        autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR,
+        is_expired ? l10n_util::GetStringUTF16(
+                         IDS_PAYMENTS_VALIDATION_INVALID_CREDIT_CARD_EXPIRED)
+                   : base::string16());
+
+    month_combobox->SetInvalid(is_expired);
+    year_combobox->SetInvalid(is_expired);
+
+    return !is_expired;
+  }
+
+  void ComboboxModelChanged(views::Combobox* combobox) override {}
+
+ private:
+  EditorViewController* controller_;
+  const std::string& app_locale_;

[anthonyvd, 2017/05/19 19:16:59]

Maybe it doesn't matter in this case because the passed string outlives this
object but we shouldn't have reference type members. Copying a locale string
really shouldn't be an expensive operation anyways.

[Mathieu, 2017/05/19 21:13:46]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ExpirationDateValidationDelegate);
+};
+
 }  // namespace
 
 CreditCardEditorViewController::CreditCardEditorViewController(
@@ -165,6 +215,50 @@ CreditCardEditorViewController::CreateHeaderView() {
 }
 
 std::unique_ptr<views::View>
+CreditCardEditorViewController::CreateCustomFieldView(
+    autofill::ServerFieldType type) {
+  DCHECK(type == autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR);

[anthonyvd, 2017/05/19 19:16:59]

nit: DCHECK_EQ

[Mathieu, 2017/05/19 21:13:46]

Done.

+
+  std::unique_ptr<views::View> view = base::MakeUnique<views::View>();
+  std::unique_ptr<views::GridLayout> combobox_layout =
+      base::MakeUnique<views::GridLayout>(view.get());
+  // Column set for short fields.
+  views::ColumnSet* columns = combobox_layout->AddColumnSet(0);
+  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 1,
+                     views::GridLayout::USE_PREF, 0, 0);
+  // Space between the two comboboxes.
+  constexpr int kHorizontalSpacing = 8;
+  columns->AddPaddingColumn(0, kHorizontalSpacing);
+  columns->AddColumn(views::GridLayout::LEADING, views::GridLayout::CENTER, 1,
+                     views::GridLayout::USE_PREF, 0, 0);
+
+  combobox_layout->StartRow(0, 0);
+  constexpr int kInputFieldHeight = 28;
+  EditorField tmp_month{autofill::CREDIT_CARD_EXP_MONTH, base::string16(),
+                        EditorField::LengthHint::HINT_SHORT,
+                        /* required= */ true,

[anthonyvd, 2017/05/19 19:16:59]

/*required=*/true

[Mathieu, 2017/05/19 21:13:46]

Done.

+                        EditorField::ControlType::COMBOBOX};
+  std::unique_ptr<ValidatingCombobox> month_combobox =
+      CreateComboboxForField(tmp_month);
+  combobox_layout->AddView(month_combobox.release(), 1, 1,
+                           views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                           kInputFieldHeight);
+
+  EditorField tmp_year{autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR, base::string16(),
+                       EditorField::LengthHint::HINT_SHORT,
+                       /* required= */ true,

[anthonyvd, 2017/05/19 19:16:59]

/*required=*/true

[Mathieu, 2017/05/19 21:13:46]

Done.

+                       EditorField::ControlType::COMBOBOX};
+  std::unique_ptr<ValidatingCombobox> year_combobox =
+      CreateComboboxForField(tmp_year);
+  combobox_layout->AddView(year_combobox.release(), 1, 1,
+                           views::GridLayout::FILL, views::GridLayout::FILL, 0,
+                           kInputFieldHeight);
+
+  view->SetLayoutManager(combobox_layout.release());
+  return view;
+}
+
+std::unique_ptr<views::View>
 CreditCardEditorViewController::CreateExtraViewForField(
     autofill::ServerFieldType type) {
   if (type != kBillingAddressType)
@@ -191,14 +285,10 @@ std::vector<EditorField> CreditCardEditorViewController::GetFieldDefinitions() {
       {autofill::CREDIT_CARD_NAME_FULL,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_NAME_ON_CARD),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true},
-      {autofill::CREDIT_CARD_EXP_MONTH,
-       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_MONTH),
-       EditorField::LengthHint::HINT_SHORT, /* required= */ true,
-       EditorField::ControlType::COMBOBOX},
-      {autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR,
-       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_YEAR),
+      {autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR,
+       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_EXPIRATION_DATE),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true,
-       EditorField::ControlType::COMBOBOX},
+       EditorField::ControlType::CUSTOMFIELD},
       {kBillingAddressType,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_BILLING_ADDRESS),
        EditorField::LengthHint::HINT_SHORT, /* required= */ true,
@@ -299,6 +389,10 @@ CreditCardEditorViewController::CreateValidationDelegate(
     const EditorField& field) {
   // The supported card networks for non-cc-number types are not passed to avoid
   // the data copy in the delegate.
+  if (field.type == autofill::CREDIT_CARD_EXP_MONTH ||
+      field.type == autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR)
+    return base::MakeUnique<ExpirationDateValidationDelegate>(
+        this, state()->GetApplicationLocale());
   return base::MakeUnique<
       CreditCardEditorViewController::CreditCardValidationDelegate>(
       field, this,
@@ -417,7 +511,7 @@ bool CreditCardEditorViewController::CreditCardValidationDelegate::
             ? autofill::IsValidCreditCardNumberForBasicCardNetworks(
                   value, supported_card_networks_, &error_message)
             : autofill::IsValidForType(value, field_.type, &error_message);
-    controller_->DisplayErrorMessageForField(field_, error_message);
+    controller_->DisplayErrorMessageForField(field_.type, error_message);
     return is_valid;
   }
 
@@ -426,7 +520,7 @@ bool CreditCardEditorViewController::CreditCardValidationDelegate::
       is_required_valid ? base::ASCIIToUTF16("")
                         : l10n_util::GetStringUTF16(
                               IDS_PAYMENTS_FIELD_REQUIRED_VALIDATION_MESSAGE);
-  controller_->DisplayErrorMessageForField(field_, displayed_message);
+  controller_->DisplayErrorMessageForField(field_.type, displayed_message);
   return is_required_valid;
 }