[Payments] Add combobox support to editors.

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

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_UI_VIEWS_PAYMENTS_CREDIT_CARD_EDITOR_VIEW_CONTROLLER_H_
 #define CHROME_BROWSER_UI_VIEWS_PAYMENTS_CREDIT_CARD_EDITOR_VIEW_CONTROLLER_H_
 
 #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 {
 
 class PaymentRequest;
 class PaymentRequestDialogView;
 
 // Credit card editor screen of the Payment Request flow.
 class CreditCardEditorViewController : public EditorViewController {
  public:
   // Does not take ownership of the arguments, which should outlive this object.
   CreditCardEditorViewController(PaymentRequest* request,
                                  PaymentRequestDialogView* dialog);
   ~CreditCardEditorViewController() override;
 
   // 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);
 };
 
 }  // namespace payments
 
 #endif  // CHROME_BROWSER_UI_VIEWS_PAYMENTS_CREDIT_CARD_EDITOR_VIEW_CONTROLLER_H_