MenuButton: support Mac look & feel

ui/views/controls/button/menu_button.cc

Index: ui/views/controls/button/menu_button.cc
diff --git a/ui/views/controls/button/menu_button.cc b/ui/views/controls/button/menu_button.cc
index 4d1a1b9728157e8581a4ec725a94de28183a7715..00ec697d322adc6a5e9f1ef34c17be50d76a48bf 100644
--- a/ui/views/controls/button/menu_button.cc
+++ b/ui/views/controls/button/menu_button.cc
@@ -21,8 +21,10 @@
 #include "ui/views/animation/ink_drop_delegate.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
+#include "ui/views/controls/focusable_border.h"
 #include "ui/views/mouse_constants.h"
 #include "ui/views/resources/grit/views_resources.h"
+#include "ui/views/style/platform_style.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
@@ -37,8 +39,6 @@ static const int kDefaultMenuOffsetY = -4;
 
 // static
 const char MenuButton::kViewClassName[] = "MenuButton";
-const int MenuButton::kMenuMarkerPaddingLeft = 3;
-const int MenuButton::kMenuMarkerPaddingRight = -1;
 
 ////////////////////////////////////////////////////////////////////////////////
 //
@@ -73,15 +73,17 @@ MenuButton::MenuButton(const base::string16& text,
       menu_offset_(kDefaultMenuOffsetX, kDefaultMenuOffsetY),
       listener_(menu_button_listener),
       show_menu_marker_(show_menu_marker),
-      menu_marker_(ui::ResourceBundle::GetSharedInstance()
-                       .GetImageNamed(IDR_MENU_DROPARROW)
-                       .ToImageSkia()),
       destroyed_flag_(nullptr),
       pressed_lock_count_(0),
       increment_pressed_lock_called_(nullptr),
       should_disable_after_press_(false),
       weak_factory_(this) {
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  if (show_menu_marker_) {
+    menu_marker_ = PlatformStyle::CreateMenuButtonArrow(enabled());
+    set_background(PlatformStyle::CreateMenuButtonBackground(GetShoulderWidth())

[tapted, 2016/04/29 06:53:18]

The background will need similar treatment to the border to avoid a regression
like http://crbug.com/605161#c5

I think we should add some simple tests for this, something similar to
DialogButtonBorderMacTest.ChecksButtonStyle and friends

[Elly Fong-Jones, 2016/05/06 21:30:45]

Done.

+                       .release());
+  }
 }
 
 MenuButton::~MenuButton() {
@@ -198,11 +200,7 @@ void MenuButton::OnPaint(gfx::Canvas* canvas) {
 
 gfx::Size MenuButton::GetPreferredSize() const {
   gfx::Size prefsize = LabelButton::GetPreferredSize();
-  if (show_menu_marker_) {
-    prefsize.Enlarge(menu_marker_->width() + kMenuMarkerPaddingLeft +
-                         kMenuMarkerPaddingRight,
-                     0);
-  }
+  prefsize.Enlarge(GetShoulderWidth(), 0);
   return prefsize;
 }
 
@@ -311,29 +309,31 @@ void MenuButton::GetAccessibleState(ui::AXViewState* state) {
   state->AddStateFlag(ui::AX_STATE_HASPOPUP);
 }
 
+void MenuButton::OnEnabledChanged() {
+  if (show_menu_marker_)
+    menu_marker_ = PlatformStyle::CreateMenuButtonArrow(enabled());
+  LabelButton::OnEnabledChanged();
+}
+
+void MenuButton::OnNativeThemeChanged(const ui::NativeTheme* theme) {
+  if (ShouldUsePlatformStyleBorder())
+    SetBorder(PlatformStyle::CreateMenuButtonBorder());
+}
+
 void MenuButton::PaintMenuMarker(gfx::Canvas* canvas) {
   gfx::Insets insets = GetInsets();
 
   // Using the Views mirroring infrastructure incorrectly flips icon content.
   // Instead, manually mirror the position of the down arrow.
-  gfx::Rect arrow_bounds(width() - insets.right() -
-                         menu_marker_->width() - kMenuMarkerPaddingRight,
-                         height() / 2 - menu_marker_->height() / 2,
-                         menu_marker_->width(),
-                         menu_marker_->height());
+  gfx::Rect arrow_bounds(width() - GetShoulderWidth(), 0,
+                         GetShoulderWidth(), height());
+  arrow_bounds.ClampToCenteredSize(menu_marker_.size());

[tapted, 2016/04/29 06:53:18]

With kMenuMarkerPaddingLeft/Right gone, won't this change the position on
linux/win/chromeos?

[Elly Fong-Jones, 2016/05/06 21:30:45]

I *think* that I accounted for that in GetShoulderWidth()? I don't have Linux or
Windows machines handy to test against.

[tapted, 2016/05/09 08:01:35]

GetShoulderWidth() is the sum though. It might work for combobox, but MenuButton
is less evenly weighted. I'll try this on Windows. my last attempt at a build
ran out of disk space though :|

[tapted, 2016/05/10 08:03:23]

Added Windows screenshots to http://crbug.com/605161#c6 for this. The layout
changes quite a bit (and of course we need to ensure Windows doesn't change at
all).

   arrow_bounds.set_x(GetMirroredXForRect(arrow_bounds));
-  canvas->DrawImageInt(*menu_marker_, arrow_bounds.x(), arrow_bounds.y());
+  canvas->DrawImageInt(menu_marker_, arrow_bounds.x(), arrow_bounds.y());
 }
 
 gfx::Rect MenuButton::GetChildAreaBounds() {
-  gfx::Size s = size();
-
-  if (show_menu_marker_) {
-    s.set_width(s.width() - menu_marker_->width() - kMenuMarkerPaddingLeft -
-                kMenuMarkerPaddingRight);
-  }
-
-  return gfx::Rect(s);
+  return gfx::Rect(width() - GetShoulderWidth(), height());
 }
 
 bool MenuButton::IsTriggerableEvent(const ui::Event& event) {
@@ -372,6 +372,10 @@ void MenuButton::NotifyClick(const ui::Event& event) {
   Activate(&event);
 }
 
+bool MenuButton::ShouldUsePlatformStyleBorder() const {
+  return false;

[tapted, 2016/04/29 06:53:18]

nothing overrides this yet. I believe the current target use-case is the
TranslateBubble - we need to prepare a patch to ensure that will work correctly
when this is needed, and to ensure the structure still looks nice. Something in
views_examples would probably help too.

[Elly Fong-Jones, 2016/05/06 21:30:45]

I added some examples in views_examples for this.

+}
+
 void MenuButton::IncrementPressedLocked(bool snap_ink_drop_to_activated) {
   ++pressed_lock_count_;
   if (increment_pressed_lock_called_)
@@ -415,4 +419,11 @@ int MenuButton::GetMaximumScreenXCoordinate() {
   return monitor_bounds.right() - 1;
 }
 
+int MenuButton::GetShoulderWidth() const {
+  const int kPadding = 7;
+  if (!show_menu_marker_)
+    return 0;
+  return menu_marker_.width() + 2 * kPadding;
+}
+
 }  // namespace views