MenuButton: support Mac look & feel

ui/views/controls/button/menu_button.h

 // 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.
 
 #ifndef UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_H_
 #define UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_H_
 
 #include <string>
 
 #include "base/macros.h"
@@ skipping 26 matching lines @@
 
    private:
     base::WeakPtr<MenuButton> menu_button_;
 
     DISALLOW_COPY_AND_ASSIGN(PressedLock);
   };
 
   static const char kViewClassName[];
 
   // How much padding to put on the left and right of the menu marker.
   static const int kMenuMarkerPaddingLeft;

[tapted, 2016/04/27 03:25:11]

huh - looks like these are unused now and can be removed. But that might be bad.
They are not even, so I think there there will be a regression in the layout
using ClampToCenteredSize instead of these for non-Mac platforms, so that needs
to be fixed. If you still need values for left/right, they should move into an
anonymous namespace in the .cc.

[Elly Fong-Jones, 2016/04/28 17:03:12]

Done.

   static const int kMenuMarkerPaddingRight;
 
   // Create a Button.
   MenuButton(const base::string16& text,
              MenuButtonListener* menu_button_listener,
              bool show_menu_marker);
   ~MenuButton() override;
 
-  bool show_menu_marker() const { return show_menu_marker_; }
-  void set_menu_marker(const gfx::ImageSkia* menu_marker) {
-    menu_marker_ = menu_marker;
-  }
-  const gfx::ImageSkia* menu_marker() const { return menu_marker_; }
-
   const gfx::Point& menu_offset() const { return menu_offset_; }
   void set_menu_offset(int x, int y) { menu_offset_.SetPoint(x, y); }
 
   // Activate the button (called when the button is pressed). |event| is the
   // event triggering the activation, if any.
   bool Activate(const ui::Event* event);
 
   // Returns true if the event is of the proper type to potentially trigger an
   // action. Since MenuButtons have properties other than event type (like
   // last menu open time) to determine if an event is valid to activate the
   // menu, this is distinct from IsTriggerableEvent().
   virtual bool IsTriggerableEventType(const ui::Event& event);
 
   // Overridden from View:
   gfx::Size GetPreferredSize() const override;
   const char* GetClassName() const override;
   void OnPaint(gfx::Canvas* canvas) override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
   void OnMouseEntered(const ui::MouseEvent& event) override;
   void OnMouseExited(const ui::MouseEvent& event) override;
   void OnMouseMoved(const ui::MouseEvent& event) override;
   void OnGestureEvent(ui::GestureEvent* event) override;
   bool OnKeyPressed(const ui::KeyEvent& event) override;
   bool OnKeyReleased(const ui::KeyEvent& event) override;
   void GetAccessibleState(ui::AXViewState* state) override;
+  void OnEnabledChanged() override;
 
  protected:
   // Paint the menu marker image.
   void PaintMenuMarker(gfx::Canvas* canvas);
 
   // Overridden from LabelButton:
   gfx::Rect GetChildAreaBounds() override;
 
   // Overridden from CustomButton:
   bool IsTriggerableEvent(const ui::Event& event) override;
@@ skipping 11 matching lines @@
   // set the state accordingly. The ink drop is snapped to the final ACTIVATED
   // state if |snap_ink_drop_to_activated| is true, otherwise the ink drop will
   // be animated to the ACTIVATED state.
   void IncrementPressedLocked(bool snap_ink_drop_to_activated);
   void DecrementPressedLocked();
 
   // Compute the maximum X coordinate for the current screen. MenuButtons
   // use this to make sure a menu is never shown off screen.
   int GetMaximumScreenXCoordinate();
 
+  // Returns the width of the "shoulder" of the menubutton, which is the area

[tapted, 2016/04/27 03:25:11]

nit: menubutton -> MenuButton

[Elly Fong-Jones, 2016/04/28 17:03:12]

Done.

+  // containing the menu marker, and any padding surrounding it.
+  int GetShoulderWidth() const;
+
   // We use a time object in order to keep track of when the menu was closed.
   // The time is used for simulating menu behavior for the menu button; that
   // is, if the menu is shown and the button is pressed, we need to close the
   // menu. There is no clean way to get the second click event because the
   // menu is displayed using a modal loop and, unlike regular menus in Windows,
   // the button is not part of the displayed menu.
   base::TimeTicks menu_closed_time_;
 
   // Our listener. Not owned.
   MenuButtonListener* listener_;
 
+  // The images to draw for the menu marker, if |show_menu_marker_| is true.

[tapted, 2016/04/27 03:25:11]

I think we can keep the old comment, and variable name (menu_marker_) - they
don't really need to change. Also nudge down to the prior location (below the
bool). Changing it from a raw pointer to a (wrapper around) a ref-counted
pointer I'd keep

[Elly Fong-Jones, 2016/04/28 17:03:12]

Done.

+  gfx::ImageSkia marker_image_;
+
   // Whether or not we're showing a drop marker.
-  bool show_menu_marker_;
-
-  // The down arrow used to differentiate the menu button from normal buttons.
-  const gfx::ImageSkia* menu_marker_;
+  const bool show_menu_marker_;
 
   // If non-null the destructor sets this to true. This is set while the menu is
   // showing and used to detect if the menu was deleted while running.
   bool* destroyed_flag_;
 
   // The current number of "pressed" locks this button has.
   int pressed_lock_count_;
 
   // Used to let Activate() know if IncrementPressedLocked() was called.
   bool* increment_pressed_lock_called_;
 
   // True if the button was in a disabled state when a menu was run, and should
   // return to it once the press is complete. This can happen if, e.g., we
   // programmatically show a menu on a disabled button.
   bool should_disable_after_press_;
 
   base::WeakPtrFactory<MenuButton> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuButton);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_BUTTON_MENU_BUTTON_H_