Fix app menu button preferred size issue.

App menu now sets its label text context to toolbar button, like other
toolbar buttons.

Also prevent toolbar and app menu buttons from unnecessarily destroying
and recreating empty borders if the inset size has not changed.

Bug: 931575
Change-Id: Id2b7e146a97b1da56f08caa9b79b4c7072c9de04
Reviewed-on: https://chromium-review.googlesource.com/c/1474687Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632703}

chrome/browser/ui/views/frame/app_menu_button.cc

@@ -7,12 +7,15 @@
 #include <utility>
 
 #include "chrome/browser/ui/toolbar/app_menu_model.h"
+#include "chrome/browser/ui/views/chrome_typography.h"
 #include "chrome/browser/ui/views/frame/app_menu_button_observer.h"
 #include "chrome/browser/ui/views/toolbar/app_menu.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_ink_drop_util.h"
 
 AppMenuButton::AppMenuButton(views::MenuButtonListener* menu_button_listener)
-    : views::MenuButton(base::string16(), menu_button_listener) {}
+    : views::MenuButton(base::string16(),
+                        menu_button_listener,
+                        CONTEXT_TOOLBAR_BUTTON) {}
 
 AppMenuButton::~AppMenuButton() {}
 

chrome/browser/ui/views/toolbar/browser_app_menu_button.cc

@@ -303,8 +303,10 @@ const char* BrowserAppMenuButton::GetClassName() const {
 }
 
 void BrowserAppMenuButton::UpdateBorder() {
-  SetBorder(views::CreateEmptyBorder(GetLayoutInsets(TOOLBAR_BUTTON) +
-                                     *GetProperty(views::kInternalPaddingKey)));
+  gfx::Insets new_insets = GetLayoutInsets(TOOLBAR_BUTTON) +
+                           *GetProperty(views::kInternalPaddingKey);
+  if (!border() || border()->GetInsets() != new_insets)
+    SetBorder(views::CreateEmptyBorder(new_insets));
 }
 
 void BrowserAppMenuButton::OnBoundsChanged(const gfx::Rect& previous_bounds) {

chrome/browser/ui/views/toolbar/toolbar_button.cc

@@ -62,6 +62,8 @@ ToolbarButton::ToolbarButton(views::ButtonListener* listener,
   // make to the leading margin to handle Fitts' Law, it's easier to just
   // allocate the property once and modify the value.
   SetProperty(views::kInternalPaddingKey, new gfx::Insets());
+
+  UpdateHighlightBackgroundAndInsets();
 }
 
 ToolbarButton::~ToolbarButton() {}
@@ -101,21 +103,24 @@ void ToolbarButton::UpdateHighlightBackgroundAndInsets() {
     SetEnabledTextColors(*highlight_color_);
   }
 
-  gfx::Insets insets = GetLayoutInsets(TOOLBAR_BUTTON) + layout_inset_delta_ +
-                       *GetProperty(views::kInternalPaddingKey);
+  gfx::Insets new_insets = GetLayoutInsets(TOOLBAR_BUTTON) +
+                           layout_inset_delta_ +
+                           *GetProperty(views::kInternalPaddingKey);
+
   if (!GetText().empty()) {
     const int text_side_inset = highlight_radius / 2;
 
     // Some subclasses (AvatarToolbarButton) may be change alignment. This adds
     // an inset to the text-label side.
     if (horizontal_alignment() == gfx::ALIGN_RIGHT) {
-      insets += gfx::Insets(0, text_side_inset, 0, 0);
+      new_insets += gfx::Insets(0, text_side_inset, 0, 0);
     } else {
-      insets += gfx::Insets(0, 0, 0, text_side_inset);
+      new_insets += gfx::Insets(0, 0, 0, text_side_inset);
     }
   }
 
-  SetBorder(views::CreateEmptyBorder(insets));
+  if (!border() || new_insets != border()->GetInsets())
+    SetBorder(views::CreateEmptyBorder(new_insets));
 }
 
 void ToolbarButton::SetLayoutInsetDelta(const gfx::Insets& inset_delta) {

ui/views/controls/button/menu_button.cc

@@ -18,8 +18,9 @@ const int MenuButton::kMenuMarkerPaddingLeft = 3;
 const int MenuButton::kMenuMarkerPaddingRight = -1;
 
 MenuButton::MenuButton(const base::string16& text,
-                       MenuButtonListener* menu_button_listener)
-    : LabelButton(nullptr, text),
+                       MenuButtonListener* menu_button_listener,
+                       int button_context)
+    : LabelButton(nullptr, text, button_context),
       menu_button_event_handler_(this, menu_button_listener) {
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
 }
@@ -83,4 +84,4 @@ void MenuButton::NotifyClick(const ui::Event& event) {
   menu_button_event_handler_.NotifyClick(event);
 }
 
-}  // namespace views
\ No newline at end of file
+}  // namespace views

ui/views/controls/button/menu_button.h

@@ -31,7 +31,8 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
 
   // Create a Button.
   MenuButton(const base::string16& text,
-             MenuButtonListener* menu_button_listener);
+             MenuButtonListener* menu_button_listener,
+             int button_context = style::CONTEXT_BUTTON);
   ~MenuButton() override;
 
   bool Activate(const ui::Event* event);
@@ -57,7 +58,7 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
   void OnMouseMoved(const ui::MouseEvent& event) final;
 
   // Protected methods needed for MenuButtonEventHandler.
-  // TODO (cyan): Move these to a delegate interface.
+  // TODO(cyan): Move these to a delegate interface.
   using InkDropHostView::GetInkDrop;
   using View::InDrag;
   using View::GetDragOperations;
@@ -80,7 +81,7 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
 
  private:
   // All events get sent to this handler to be processed.
-  // TODO (cyan): This will be generalized into a ButtonEventHandler and moved
+  // TODO(cyan): This will be generalized into a ButtonEventHandler and moved
   // into the Button class.
   MenuButtonEventHandler menu_button_event_handler_;