MenuButton: support Mac look & feel

ui/views/controls/combobox/combobox.h

Index: ui/views/controls/combobox/combobox.h
diff --git a/ui/views/controls/combobox/combobox.h b/ui/views/controls/combobox/combobox.h
index 2705f7e22cc57e836ec64d03223a41bce479a585..8d61dda895de0963221ac51f93edac6e95a48982 100644
--- a/ui/views/controls/combobox/combobox.h
+++ b/ui/views/controls/combobox/combobox.h
@@ -54,6 +54,7 @@ class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
 
   // |model| is not owned by the combobox.
   explicit Combobox(ui::ComboboxModel* model);
+  Combobox(ui::ComboboxModel* model, Style style);

[tapted, 2016/04/26 05:05:58]

nit: move before the `explicit` constructor? Or reorder methods in .cc. Or (my
favourite): give `style` a default argument in the definition here (and add
`explicit` to this method). Default arguments are allowed now and this is a
pretty clear-cut case. It's good intrinsic documentation about exactly what
style you will get if you don't specify one.

[Elly Fong-Jones, 2016/04/26 17:41:25]

Done.

   ~Combobox() override;
 
   static const gfx::FontList& GetFontList();
@@ -61,8 +62,6 @@ class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
   // Sets the listener which will be called when a selection has been made.
   void set_listener(ComboboxListener* listener) { listener_ = listener; }
 
-  void SetStyle(Style style);
-
   // Informs the combobox that its model changed.
   void ModelChanged();
 
@@ -85,9 +84,9 @@ class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
   void SetInvalid(bool invalid);
   bool invalid() const { return invalid_; }
 
-  // Returns the width of the arrow button component of the combobox: the arrow
-  // button itself, and the padding on either side of it.
-  int GetArrowButtonWidth() const;
+  // Returns the width of the combobox's "shoulder", which is the part
+  // containing the arrow, including any padding space around it.

[tapted, 2016/04/26 05:05:57]

nit: remove "space"

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

+  int GetShoulderWidth() const;

[tapted, 2016/04/26 05:05:57]

Can these be in the private: section? Also make sure the method order in the .cc
matches

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

 
   // Overridden from View:
   gfx::Size GetPreferredSize() const override;
@@ -152,7 +151,7 @@ class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
   ui::ComboboxModel* model_;
 
   // The visual style of this combobox.
-  Style style_;
+  const Style style_;

[tapted, 2016/04/26 05:05:57]

nice - I like this :)

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

 
   // Our listener. Not owned. Notified when the selected index change.
   ComboboxListener* listener_;
@@ -203,6 +202,10 @@ class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
   // destroyed.
   std::unique_ptr<views::MenuRunner> menu_runner_;
 
+  // The image to be drawn for this combobox's arrow.
+  gfx::ImageSkia enabled_arrow_image_;
+  gfx::ImageSkia disabled_arrow_image_;

[tapted, 2016/04/26 05:05:57]

(per prior comment, just one image)

[Elly Fong-Jones, 2016/04/26 17:41:26]

Done.

+
   // Used for making calbacks.
   base::WeakPtrFactory<Combobox> weak_ptr_factory_;