[Payments] Add combobox support to editors.

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

Index: chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
diff --git a/chrome/browser/ui/views/payments/credit_card_editor_view_controller.h b/chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
index 15e07fc6eef20bce22e62b52e261eff10e3688c6..648dab5e90a1c1e81a3d8d074c9ba12c4e05097f 100644
--- a/chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
+++ b/chrome/browser/ui/views/payments/credit_card_editor_view_controller.h
@@ -7,7 +7,8 @@
 
 #include "base/macros.h"
 #include "chrome/browser/ui/views/payments/editor_view_controller.h"
-#include "chrome/browser/ui/views/payments/validating_textfield.h"
+#include "chrome/browser/ui/views/payments/validation_delegate.h"
+#include "ui/base/models/simple_combobox_model.h"
 
 namespace payments {
 
@@ -25,24 +26,33 @@ class CreditCardEditorViewController : public EditorViewController {
   // EditorViewController:
   std::vector<EditorField> GetFieldDefinitions() override;
   bool ValidateModelAndSave() override;
-  std::unique_ptr<ValidatingTextfield::Delegate> CreateValidationDelegate(
+  std::unique_ptr<ValidationDelegate> CreateValidationDelegate(
       const EditorField& field) override;
+  ui::ComboboxModel* GetComboboxModelForType(
+      const autofill::ServerFieldType& type) override;
 
  private:
-  class ValidationDelegate : public ValidatingTextfield::Delegate {
+  class CreditCardValidationDelegate : public ValidationDelegate {
    public:
-    explicit ValidationDelegate(const EditorField& field);
-    ~ValidationDelegate() override;
+    explicit CreditCardValidationDelegate(const EditorField& field);
+    ~CreditCardValidationDelegate() override;
 
-    // ValidatingTextfield::Delegate:
+    // ValidationDelegate:
     bool ValidateTextfield(views::Textfield* textfield) override;
+    bool ValidateCombobox(views::Combobox* combobox) override;
 
    private:
+    // Validates a specific |value|.
+    bool ValidateValue(const base::string16& value);
+
     EditorField field_;
 
-    DISALLOW_COPY_AND_ASSIGN(ValidationDelegate);
+    DISALLOW_COPY_AND_ASSIGN(CreditCardValidationDelegate);
   };
 
+  std::unique_ptr<ui::SimpleComboboxModel> exp_month_model_;

[Mathieu, 2017/02/14 21:23:54]

Under the current code, it's problematic to have this here because the
controller is destroyed before the Combobox, and ComboboxMenuModel needs a valid
reference to this model in its destructor [1]. I think the controller should be
destroyed after the view (see other comment)

[1]
https://cs.chromium.org/chromium/src/ui/views/controls/combobox/combobox.cc?r...

[sky, 2017/02/14 23:19:24]

Indeed. You'll have to tie the lifetime of the model to the combobox. Can you
explicitly delete the combobox before the model? If not, you'll need to transfer
ownership to an object that lives as long as the combobox. One option for that
moving ownership to PaymentRequestDialogView as it lasts at least as long as the
combobox. Another option is to use class properties so that the combobox
effectively owns the model. See
https://cs.chromium.org/chromium/src/ui/base/class_property.h?q=classproperty...
. A last option is to add a constructor to Combobox() that takes a
unique_ptr<ComboboxModel>. This is likely best option given others have
encountered this in the past and you won't be the last that wants the combobox
to own the model.

[Mathieu, 2017/02/15 03:15:57]

I've picked the last option, as I think that for most models it makes sense to
have the combobox hold on to it. PTAL

+  std::unique_ptr<ui::SimpleComboboxModel> exp_year_model_;
+
   DISALLOW_COPY_AND_ASSIGN(CreditCardEditorViewController);
 };