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

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

Index: chrome/browser/ui/views/payments/editor_view_controller.cc
diff --git a/chrome/browser/ui/views/payments/editor_view_controller.cc b/chrome/browser/ui/views/payments/editor_view_controller.cc
index de66b31d1a1a9009503ef960bf73eff6f059b5df..8f8866eb484d1290bb25b5679a5af8bfe0c2b9d8 100644
--- a/chrome/browser/ui/views/payments/editor_view_controller.cc
+++ b/chrome/browser/ui/views/payments/editor_view_controller.cc
@@ -43,8 +43,9 @@ enum class EditorViewControllerTags : int {
   SAVE_BUTTON = kFirstTagValue,
 };
 
-std::unique_ptr<views::View> CreateErrorLabelView(const base::string16& error,
-                                                  const EditorField& field) {
+std::unique_ptr<views::View> CreateErrorLabelView(
+    const base::string16& error,
+    autofill::ServerFieldType type) {
   std::unique_ptr<views::View> view = base::MakeUnique<views::View>();
 
   std::unique_ptr<views::BoxLayout> layout =
@@ -60,7 +61,7 @@ std::unique_ptr<views::View> CreateErrorLabelView(const base::string16& error,
   std::unique_ptr<views::Label> error_label =
       base::MakeUnique<views::Label>(error);
   error_label->set_id(static_cast<int>(DialogViewID::ERROR_LABEL_OFFSET) +
-                      field.type);
+                      type);
   error_label->SetFontList(
       error_label->GetDefaultFontList().DeriveWithSizeDelta(-1));
   error_label->SetEnabledColor(error_label->GetNativeTheme()->GetSystemColor(
@@ -84,15 +85,15 @@ EditorViewController::EditorViewController(
 EditorViewController::~EditorViewController() {}
 
 void EditorViewController::DisplayErrorMessageForField(
-    const EditorField& field,
+    autofill::ServerFieldType type,
     const base::string16& error_message) {
-  const auto& label_view_it = error_labels_.find(field);
+  const auto& label_view_it = error_labels_.find(type);
   DCHECK(label_view_it != error_labels_.end());
 
   label_view_it->second->RemoveAllChildViews(/*delete_children=*/true);
   if (!error_message.empty()) {
     label_view_it->second->AddChildView(
-        CreateErrorLabelView(error_message, field).release());
+        CreateErrorLabelView(error_message, type).release());
   }
   RelayoutPane();
 }
@@ -172,6 +173,21 @@ views::View* EditorViewController::GetFirstFocusedView() {
   return PaymentRequestSheetController::GetFirstFocusedView();
 }
 
+std::unique_ptr<ValidatingCombobox>
+EditorViewController::CreateComboboxForField(const EditorField& field) {
+  std::unique_ptr<ValidatingCombobox> combobox =
+      base::MakeUnique<ValidatingCombobox>(GetComboboxModelForType(field.type),
+                                           CreateValidationDelegate(field));
+  base::string16 initial_value = GetInitialValueForType(field.type);
+  if (!initial_value.empty())
+    combobox->SelectValue(initial_value);
+  // Using autofill field type as a view ID.
+  combobox->set_id(static_cast<int>(field.type));

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

That's probably not a good idea because it'll cause conflicts with other IDs
down the line and so we can't have any guarantee about what GetViewById is going
to return. Those bugs are probably going to be super hard to diagnose when they
break tests.

Maybe we can have an entry in DialogViewID for an offset where we start field
IDs at?

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

Done.

+  combobox->set_listener(this);
+  comboboxes_.insert(std::make_pair(combobox.get(), field));
+  return combobox;
+}
+
 void EditorViewController::ContentsChanged(views::Textfield* sender,
                                            const base::string16& new_contents) {
   static_cast<ValidatingTextfield*>(sender)->OnContentsChanged();
@@ -320,28 +336,26 @@ void EditorViewController::CreateInputField(views::GridLayout* layout,
     layout->AddView(text_field, 1, 1, views::GridLayout::FILL,
                     views::GridLayout::FILL, 0, kInputFieldHeight);
   } else if (field.control_type == EditorField::ControlType::COMBOBOX) {
-    ValidatingCombobox* combobox = new ValidatingCombobox(
-        GetComboboxModelForType(field.type), CreateValidationDelegate(field));
-    base::string16 initial_value = GetInitialValueForType(field.type);
-    if (!initial_value.empty())
-      combobox->SelectValue(initial_value);
-    // Using autofill field type as a view ID.
-    combobox->set_id(static_cast<int>(field.type));
-    combobox->set_listener(this);
-    comboboxes_.insert(std::make_pair(combobox, field));
+    std::unique_ptr<ValidatingCombobox> combobox =
+        CreateComboboxForField(field);
 
     if (!first_field_view_)
-      first_field_view_ = combobox;
+      first_field_view_ = combobox.get();
 
     // |combobox| will now be owned by |row|.
-    layout->AddView(combobox, 1, 1, views::GridLayout::FILL,
+    layout->AddView(combobox.release(), 1, 1, views::GridLayout::FILL,
                     views::GridLayout::FILL, 0, kInputFieldHeight);
   } else {
     // Custom field view will now be owned by |row|. And it must be valid since
     // the derived class specified a custom view for this field.
     std::unique_ptr<views::View> field_view = CreateCustomFieldView(field.type);
     DCHECK(field_view);
-    layout->AddView(field_view.release());
+
+    if (!first_field_view_)
+      first_field_view_ = field_view.get();
+
+    layout->AddView(field_view.release(), 1, 1, views::GridLayout::FILL,
+                    views::GridLayout::FILL, 0, kInputFieldHeight);
   }
 
   // If an extra view needs to go alongside the input field view, add it to the
@@ -355,7 +369,7 @@ void EditorViewController::CreateInputField(views::GridLayout* layout,
   std::unique_ptr<views::View> error_label_view =
       base::MakeUnique<views::View>();
   error_label_view->SetLayoutManager(new views::FillLayout);
-  error_labels_[field] = error_label_view.get();
+  error_labels_[field.type] = error_label_view.get();
   layout->AddView(error_label_view.release());
 
   // Bottom padding for the row.