MenuButton: support Mac look & feel

ui/views/controls/button/menu_button.cc

 // Copyright (c) 2012 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.
 
 #include "ui/views/controls/button/menu_button.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/ui_base_switches_util.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/screen.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/strings/grit/ui_strings.h"
 #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"
 
 using base::TimeTicks;
 using base::TimeDelta;
 
 namespace views {
 
 // Default menu offset.
 static const int kDefaultMenuOffsetX = -2;
 static const int kDefaultMenuOffsetY = -4;
 
 // static
 const char MenuButton::kViewClassName[] = "MenuButton";
-const int MenuButton::kMenuMarkerPaddingLeft = 3;
-const int MenuButton::kMenuMarkerPaddingRight = -1;
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MenuButton::PressedLock
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 MenuButton::PressedLock::PressedLock(MenuButton* menu_button)
     : PressedLock(menu_button, false) {}
 
@@ skipping 14 matching lines @@
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 MenuButton::MenuButton(const base::string16& text,
                        MenuButtonListener* menu_button_listener,
                        bool show_menu_marker)
     : LabelButton(nullptr, 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_) {

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

just call OnEnabledChanged

+    menu_marker_ = PlatformStyle::CreateMenuButtonArrow(enabled());
+  }
 }
 
 MenuButton::~MenuButton() {
   if (destroyed_flag_)
     *destroyed_flag_ = true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MenuButton - Public APIs
@@ skipping 96 matching lines @@
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // MenuButton - Events
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 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;
 }
 
 const char* MenuButton::GetClassName() const {
   return kViewClassName;
 }
 
 bool MenuButton::OnMousePressed(const ui::MouseEvent& event) {
   if (request_focus_on_press())
     RequestFocus();
@@ skipping 88 matching lines @@
   return false;
 }
 
 void MenuButton::GetAccessibleState(ui::AXViewState* state) {
   CustomButton::GetAccessibleState(state);
   state->role = ui::AX_ROLE_POP_UP_BUTTON;
   state->default_action = l10n_util::GetStringUTF16(IDS_APP_ACCACTION_PRESS);
   state->AddStateFlag(ui::AX_STATE_HASPOPUP);
 }
 
+void MenuButton::OnEnabledChanged() {
+  if (show_menu_marker_)
+    menu_marker_ = PlatformStyle::CreateMenuButtonArrow(enabled());

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

So now the background is not unexpectedly blue, there is another problem for
permission_selector_view.cc's PermissionMenuButton which I didn't realise would
happen before. No background == white background. But the default "enabled"
arrow is white, so you can't see it on a white background.

I think the right way to fix this is to pass the entire MenuButton into
PlatformStyle -- let PlatformStyle decide whether to create a native or
light-coloured arrow based on whether the button already has a background/border
it recognises. Make sure you call OnEnabledChanged when the background/border
may have changed due to the native theme. (note: unlike SetBorder, there is no
overridable SetBackground -- just set_background -- so I think this is more
reason to use SetBorder in the same way as LabelButton.

+  LabelButton::OnEnabledChanged();
+}
+
+void MenuButton::OnNativeThemeChanged(const ui::NativeTheme* theme) {
+  LabelButton::OnNativeThemeChanged(theme);
+  if (ShouldUsePlatformStyleBackground()) {
+    set_background(PlatformStyle::CreateMenuButtonBackground(GetShoulderWidth())
+                       .release());
+  }
+  if (ShouldUsePlatformStyleBorder())

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

There's still an issue with this. MenuButton inherits from LabelButton which
already has a way for instances to effectively override
`ShouldUsePlatformStyleBorder`. Except they do it simply by calling
SetBorder(anything). E.g. the Chrome menu on the toolbar calls
SetBorder(nullptr) to avoid getting a border. With this line here, it will now
get an outline and a focus ring.

An earlier comment has some ideas about this:

On 2016/04/26 05:05:58, tapted wrote:
I'm still confident that having views::MenuButton to override
LabelButton::UpdateThemedBorder() will lead to a way to land this without
regressions in the permission selector.

+    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());
   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());
+}
+
+bool MenuButton::ShouldUsePlatformStyleBackground() const {
+  return true;
+}
+
+bool MenuButton::ShouldUsePlatformStyleBorder() const {
+  return true;
 }
 
 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) {
   if (!IsTriggerableEventType(event))
     return false;
 
   TimeDelta delta = TimeTicks::Now() - menu_closed_time_;
   if (delta.InMilliseconds() < kMinimumMsBetweenButtonClicks)
     return false;  // Not enough time since the menu closed.
 
@@ skipping 61 matching lines @@
 int MenuButton::GetMaximumScreenXCoordinate() {
   if (!GetWidget()) {
     NOTREACHED();
     return 0;
   }
 
   gfx::Rect monitor_bounds = GetWidget()->GetWorkAreaBoundsInScreen();
   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