[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 cda610b6d2c97afe210aa3e135e72d79ec879d4f..3ee7d292581d4320b1b253395bf106525daae2f8 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,70 @@ std::vector<base::string16> GetExpirationYearItems() {
   return years;
 }
 
+bool IsCardExpired(const base::string16& month,
+                   base::string16& year,
+                   const std::string& app_locale) {
+  autofill::CreditCard card;
+  card.SetExpirationMonthFromString(month, app_locale);
+  card.SetExpirationYearFromString(year);
+  return card.IsExpired(autofill::AutofillClock::Now());
+}
+
+// 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) {}
+
+  bool IsValidTextfield(views::Textfield* textfield) override { return true; }

[anthonyvd, 2017/05/22 13:18:17]

nit: NOTREACHED() ?

[Mathieu, 2017/05/22 16:30:40]

Done.

+
+  bool IsValidCombobox(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(EditorViewController::GetInputFieldViewId(
+            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(EditorViewController::GetInputFieldViewId(
+            autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR)));
+    base::string16 year =
+        year_combobox->model()->GetItemAt(year_combobox->selected_index());
+
+    bool is_expired = IsCardExpired(month, year, app_locale_);
+    month_combobox->SetInvalid(is_expired);
+    year_combobox->SetInvalid(is_expired);
+
+    return !is_expired;
+  }
+
+  bool TextfieldValueChanged(views::Textfield* textfield) override {
+    return true;

[anthonyvd, 2017/05/22 13:18:17]

nit NOTREACHED() ?

[Mathieu, 2017/05/22 16:30:40]

Done.

+  }
+
+  bool ComboboxValueChanged(views::Combobox* combobox) override {
+    bool is_valid = IsValidCombobox(combobox);
+    controller_->DisplayErrorMessageForField(
+        autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR,
+        is_valid ? base::string16()
+                 : l10n_util::GetStringUTF16(
+                       IDS_PAYMENTS_VALIDATION_INVALID_CREDIT_CARD_EXPIRED));
+    return is_valid;
+  }
+
+  void ComboboxModelChanged(views::Combobox* combobox) override {}
+
+ private:
+  EditorViewController* controller_;
+  const std::string app_locale_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExpirationDateValidationDelegate);
+};
+
 }  // namespace
 
 CreditCardEditorViewController::CreditCardEditorViewController(
@@ -165,6 +229,58 @@ CreditCardEditorViewController::CreateHeaderView() {
 }
 
 std::unique_ptr<views::View>
+CreditCardEditorViewController::CreateCustomFieldView(
+    autofill::ServerFieldType type,
+    views::View** focusable_field,
+    bool* valid) {
+  DCHECK_EQ(type, autofill::CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR);
+
+  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.

[anthonyvd, 2017/05/22 13:18:17]

nit: not sure this comment is relevant anymore

[Mathieu, 2017/05/22 16:30:40]

Done.

+  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, EditorField::ControlType::COMBOBOX};
+  std::unique_ptr<ValidatingCombobox> month_combobox =
+      CreateComboboxForField(tmp_month);
+  *focusable_field = month_combobox.get();
+  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, 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());
+
+  // Set the initial validity of the custom view.
+  base::string16 month =
+      GetInitialValueForType(autofill::CREDIT_CARD_EXP_MONTH);
+  base::string16 year =
+      GetInitialValueForType(autofill::CREDIT_CARD_EXP_4_DIGIT_YEAR);
+  *valid = IsCardExpired(month, year, state()->GetApplicationLocale());
+  return view;
+}
+
+std::unique_ptr<views::View>
 CreditCardEditorViewController::CreateExtraViewForField(
     autofill::ServerFieldType type) {
   if (type != kBillingAddressType)
@@ -187,22 +303,18 @@ std::vector<EditorField> CreditCardEditorViewController::GetFieldDefinitions() {
   return std::vector<EditorField>{
       {autofill::CREDIT_CARD_NUMBER,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_CREDIT_CARD_NUMBER),
-       EditorField::LengthHint::HINT_SHORT, /* required= */ true,
+       EditorField::LengthHint::HINT_SHORT, /*required=*/true,
        EditorField::ControlType::TEXTFIELD_NUMBER},
       {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),
-       EditorField::LengthHint::HINT_SHORT, /* required= */ true,
-       EditorField::ControlType::COMBOBOX},
+       EditorField::LengthHint::HINT_SHORT, /*required=*/true},
+      {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::CUSTOMFIELD},
       {kBillingAddressType,
        l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_BILLING_ADDRESS),
-       EditorField::LengthHint::HINT_SHORT, /* required= */ true,
+       EditorField::LengthHint::HINT_SHORT, /*required=*/true,
        EditorField::ControlType::COMBOBOX}};
 }
 
@@ -239,7 +351,7 @@ bool CreditCardEditorViewController::ValidateModelAndSave() {
 
     if (field.second.type == kBillingAddressType) {
       views::Combobox* address_combobox = static_cast<views::Combobox*>(
-          dialog()->GetViewByID(kBillingAddressType));
+          dialog()->GetViewByID(GetInputFieldViewId(kBillingAddressType)));
       autofill::AddressComboboxModel* model =
           static_cast<autofill::AddressComboboxModel*>(
               address_combobox->model());
@@ -300,6 +412,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,
@@ -342,7 +458,7 @@ void CreditCardEditorViewController::FillContentView(
   // We need to search from the content view here, since the dialog may not have
   // the content view added to it yet.
   views::Combobox* combobox = static_cast<views::Combobox*>(
-      content_view->GetViewByID(kBillingAddressType));
+      content_view->GetViewByID(GetInputFieldViewId(kBillingAddressType)));
   // When the combobox has a single item, it's because it has no addresses
   // (otherwise, it would have the select header, and a separator before the
   // first address to choose from).
@@ -382,8 +498,8 @@ void CreditCardEditorViewController::ButtonPressed(views::Button* sender,
 
 void CreditCardEditorViewController::AddAndSelectNewBillingAddress(
     const autofill::AutofillProfile& profile) {
-  views::Combobox* address_combobox =
-      static_cast<views::Combobox*>(dialog()->GetViewByID(kBillingAddressType));
+  views::Combobox* address_combobox = static_cast<views::Combobox*>(
+      dialog()->GetViewByID(GetInputFieldViewId(kBillingAddressType)));
   autofill::AddressComboboxModel* model =
       static_cast<autofill::AddressComboboxModel*>(address_combobox->model());
   int index = model->AddNewProfile(profile);
@@ -420,7 +536,7 @@ bool CreditCardEditorViewController::CreditCardValidationDelegate::
     TextfieldValueChanged(views::Textfield* textfield) {
   base::string16 error_message;
   bool is_valid = ValidateValue(textfield->text(), &error_message);
-  controller_->DisplayErrorMessageForField(field_, error_message);
+  controller_->DisplayErrorMessageForField(field_.type, error_message);
   return is_valid;
 }
 
@@ -428,7 +544,7 @@ bool CreditCardEditorViewController::CreditCardValidationDelegate::
     ComboboxValueChanged(views::Combobox* combobox) {
   base::string16 error_message;
   bool is_valid = ValidateCombobox(combobox, nullptr);
-  controller_->DisplayErrorMessageForField(field_, error_message);
+  controller_->DisplayErrorMessageForField(field_.type, error_message);
   return is_valid;
 }