MenuButton: support Mac look & feel

ui/views/controls/combobox/combobox.cc

Index: ui/views/controls/combobox/combobox.cc
diff --git a/ui/views/controls/combobox/combobox.cc b/ui/views/controls/combobox/combobox.cc
index af0caa6be92b60086508c9893db05a0bed266e65..0b28237eed534ac043de65c27732222b3a359ed7 100644
--- a/ui/views/controls/combobox/combobox.cc
+++ b/ui/views/controls/combobox/combobox.cc
@@ -54,12 +54,6 @@ const int kMenuBorderWidthRight = 1;
 // Limit how small a combobox can be.
 const int kMinComboboxWidth = 25;
 
-// Size of the combobox arrow margins
-const int kDisclosureArrowLeftPadding = 7;
-const int kDisclosureArrowRightPadding = 7;
-const int kDisclosureArrowButtonLeftPadding = 11;
-const int kDisclosureArrowButtonRightPadding = 12;
-
 // Define the id of the first item in the menu (since it needs to be > 0)
 const int kFirstMenuItemId = 1000;
 
@@ -342,21 +336,26 @@ class Combobox::ComboboxMenuModelAdapter : public ui::MenuModel,
 ////////////////////////////////////////////////////////////////////////////////
 // Combobox, public:
 
-Combobox::Combobox(ui::ComboboxModel* model)
+Combobox::Combobox(ui::ComboboxModel* model) : Combobox(model, STYLE_NORMAL) {}
+
+Combobox::Combobox(ui::ComboboxModel* model, Style style)
     : model_(model),
-      style_(STYLE_NORMAL),
+      style_(style),
       listener_(NULL),
       selected_index_(model_->GetDefaultIndex()),
       invalid_(false),
       menu_model_adapter_(new ComboboxMenuModelAdapter(this, model)),
       text_button_(new TransparentButton(this)),
       arrow_button_(new TransparentButton(this)),
+      enabled_arrow_image_(PlatformStyle::CreateComboboxArrow(true, style)),
+      disabled_arrow_image_(PlatformStyle::CreateComboboxArrow(false, style)),
       weak_ptr_factory_(this) {
   ModelChanged();
   SetFocusable(true);
   UpdateBorder();
   // set_background() takes ownership but takes a raw pointer.
-  std::unique_ptr<Background> b = PlatformStyle::CreateComboboxBackground();
+  std::unique_ptr<Background> b =
+      PlatformStyle::CreateComboboxBackground(GetShoulderWidth());
   set_background(b.release());
 
   // Initialize the button images.
@@ -385,6 +384,12 @@ Combobox::Combobox(ui::ComboboxModel* model)
   arrow_button_->SetFocusable(false);
   AddChildView(text_button_);
   AddChildView(arrow_button_);
+
+  if (style_ == STYLE_ACTION)

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

Move this to the top of the constructor.

Then the content_size_ = and PreferredSizeChanged() calls are unnecessary, since
they will be called by ModelChanged()

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

Done.

+    selected_index_ = 0;
+
+  content_size_ = GetContentSize();
+  PreferredSizeChanged();
 }
 
 Combobox::~Combobox() {
@@ -400,19 +405,6 @@ const gfx::FontList& Combobox::GetFontList() {
   return rb.GetFontListWithDelta(ui::kLabelFontSizeDelta);
 }
 
-void Combobox::SetStyle(Style style) {
-  if (style_ == style)
-    return;
-
-  style_ = style;
-  if (style_ == STYLE_ACTION)
-    selected_index_ = 0;
-
-  UpdateBorder();
-  content_size_ = GetContentSize();
-  PreferredSizeChanged();
-}
-
 void Combobox::ModelChanged() {
   // If the selection is no longer valid (or the model is empty), restore the
   // default index.
@@ -461,10 +453,11 @@ void Combobox::SetInvalid(bool invalid) {
   SchedulePaint();
 }
 
-int Combobox::GetArrowButtonWidth() const {
-  return GetDisclosureArrowLeftPadding() +
-         ArrowSize().width() +
-         GetDisclosureArrowRightPadding();
+int Combobox::GetShoulderWidth() const {
+  const int kNormalPadding = 7;
+  const int kActionPadding = 11;
+  int padding = style_ == STYLE_NORMAL ? kNormalPadding : kActionPadding;
+  return ArrowSize().width() + 2 * padding;
 }
 
 void Combobox::Layout() {
@@ -480,9 +473,7 @@ void Combobox::Layout() {
       break;
     }
     case STYLE_ACTION: {
-      arrow_button_width = GetDisclosureArrowLeftPadding() +
-          ArrowSize().width() +
-          GetDisclosureArrowRightPadding();
+      arrow_button_width = GetShoulderWidth();
       text_button_width = width() - arrow_button_width;
       break;
     }
@@ -525,8 +516,7 @@ gfx::Size Combobox::GetPreferredSize() const {
                         Textfield::kTextPadding,
                         Textfield::kTextPadding);
   int total_width = std::max(kMinComboboxWidth, content_size_.width()) +
-                    insets.width() + GetDisclosureArrowLeftPadding() +
-                    ArrowSize().width() + GetDisclosureArrowRightPadding();
+                    insets.width() + GetShoulderWidth();
   return gfx::Size(total_width, content_size_.height() + insets.height());
 }
 
@@ -733,9 +723,7 @@ void Combobox::PaintText(gfx::Canvas* canvas) {
     selected_index_ = 0;
   base::string16 text = model()->GetItemAt(selected_index_);
 
-  gfx::Size arrow_size = ArrowSize();
-  int disclosure_arrow_offset = width() - arrow_size.width() -
-      GetDisclosureArrowLeftPadding() - GetDisclosureArrowRightPadding();
+  int disclosure_arrow_offset = width() - GetShoulderWidth();
 
   const gfx::FontList& font_list = Combobox::GetFontList();
   int text_width = gfx::GetStringWidth(text, font_list);
@@ -746,26 +734,20 @@ void Combobox::PaintText(gfx::Canvas* canvas) {
   AdjustBoundsForRTLUI(&text_bounds);
   canvas->DrawStringRect(text, font_list, text_color, text_bounds);
 
-  int arrow_x = disclosure_arrow_offset + GetDisclosureArrowLeftPadding();
-  gfx::Rect arrow_bounds(arrow_x,
-                         height() / 2 - arrow_size.height() / 2,
-                         arrow_size.width(),
-                         arrow_size.height());
+  gfx::ImageSkia arrow_image =
+      enabled() ? enabled_arrow_image_ : disabled_arrow_image_;
+  gfx::Rect arrow_bounds(disclosure_arrow_offset, 0, GetShoulderWidth(),
+                         height());
+  arrow_bounds.ClampToCenteredSize(ArrowSize());
   AdjustBoundsForRTLUI(&arrow_bounds);
 
-  gfx::ImageSkia arrow_image = PlatformStyle::CreateComboboxArrow(
-      enabled(), style_);
   canvas->DrawImageInt(arrow_image, arrow_bounds.x(), arrow_bounds.y());
 }
 
 void Combobox::PaintButtons(gfx::Canvas* canvas) {
   DCHECK(style_ == STYLE_ACTION);
 
-  gfx::ScopedCanvas scoped_canvas(canvas);
-  if (base::i18n::IsRTL()) {
-    canvas->Translate(gfx::Vector2d(width(), 0));
-    canvas->Scale(-1, 1);
-  }
+  gfx::ScopedRTLFlipCanvas scoped_canvas(canvas, bounds());
 
   bool focused = HasFocus();
   const std::vector<const gfx::ImageSkia*>& arrow_button_images =
@@ -873,30 +855,8 @@ void Combobox::OnPerformAction() {
     selected_index_ = 0;
 }
 
-int Combobox::GetDisclosureArrowLeftPadding() const {
-  switch (style_) {
-    case STYLE_NORMAL:
-      return kDisclosureArrowLeftPadding;
-    case STYLE_ACTION:
-      return kDisclosureArrowButtonLeftPadding;
-  }
-  NOTREACHED();
-  return 0;
-}
-
-int Combobox::GetDisclosureArrowRightPadding() const {
-  switch (style_) {
-    case STYLE_NORMAL:
-      return kDisclosureArrowRightPadding;
-    case STYLE_ACTION:
-      return kDisclosureArrowButtonRightPadding;
-  }
-  NOTREACHED();
-  return 0;
-}
-
 gfx::Size Combobox::ArrowSize() const {
-  return PlatformStyle::CreateComboboxArrow(enabled(), style_).size();
+  return enabled() ? enabled_arrow_image_.size() : disabled_arrow_image_.size();

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

same here - this should DCHECK that the sizes are equal

(edit: except that won't be necessary)

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

Done.

 }
 
 gfx::Size Combobox::GetContentSize() const {