Reland "Remove menu_marker from MenuButton."

This reverts commit 5148d1fe.

Reason for revert: Speculative reland, root cause should be fixed with r607193.

Original change's description:
> Revert "Remove menu_marker from MenuButton."
>
> This reverts commit 32758758.
>
> Reason for revert: causes compile failures on windows similar to the ones that appeared in the CQ runs, e.g. https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%20x64%20Builder/58688
>
> Original change's description:
> > Remove menu_marker from MenuButton.
> >
> > This is always false except for in tests and menu_example.cc which is
> > not used. This helps simplify MenuButton for the Button cleanup task.
> >
> > Bug: 901183
> > Change-Id: If5a3cf7b6ba1ce5618548d112df1c4404b2ae181
> > Reviewed-on: https://chromium-review.googlesource.com/c/1324375
> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> > Reviewed-by: Robert Flack <flackr@chromium.org>
> > Reviewed-by: Trent Apted <tapted@chromium.org>
> > Reviewed-by: Michael Wasserman <msw@chromium.org>
> > Commit-Queue: Charlene Yan <cyan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#607061}
>
> TBR=msw@chromium.org,flackr@chromium.org,oshima@chromium.org,tapted@chromium.org,cyan@chromium.org
>
> Change-Id: I9c311a3c8cea56c7670b2dace3e0410618151034
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 901183
> Reviewed-on: https://chromium-review.googlesource.com/c/1330841
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Commit-Queue: John Budorick <jbudorick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#607072}

TBR=msw@chromium.org,flackr@chromium.org,oshima@chromium.org,tapted@chromium.org,jbudorick@chromium.org,cyan@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 901183
Change-Id: I8a54361c857ac0a18acfc41dedb9537180aae4ce
Reviewed-on: https://chromium-review.googlesource.com/c/1331137
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607294}

ash/shelf/login_shelf_view.cc

@@ -193,10 +193,7 @@ class KioskAppsButton : public views::MenuButton,
                         public ui::SimpleMenuModel::Delegate {
  public:
   KioskAppsButton(const base::string16& text, const gfx::ImageSkia& image)
-      : MenuButton(text, this, true), ui::SimpleMenuModel(this) {
-    // We don't want a menu marker for the apps button.
-    set_menu_marker(&empty_menu_marker_);
-
+      : MenuButton(text, this), ui::SimpleMenuModel(this) {
     SetFocusBehavior(FocusBehavior::ALWAYS);
     SetFocusPainter(views::Painter::CreateSolidFocusPainter(
         kFocusBorderColor, kFocusBorderThickness, gfx::InsetsF()));
@@ -324,8 +321,6 @@ class KioskAppsButton : public views::MenuButton,
  private:
   std::unique_ptr<views::MenuRunner> menu_runner_;
   std::vector<mojom::KioskAppInfoPtr> kiosk_apps_;
-  // Passed to set_menu_marker to remove menu marker
-  gfx::ImageSkia empty_menu_marker_;
   bool is_launch_enabled_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(KioskAppsButton);

chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc

@@ -320,9 +320,8 @@ const char ShortcutButton::kViewClassName[] = "ShortcutButton";
 class BookmarkMenuButtonBase : public views::MenuButton {
  public:
   BookmarkMenuButtonBase(const base::string16& title,
-                         views::MenuButtonListener* menu_button_listener,
-                         bool show_menu_marker)
-      : MenuButton(title, menu_button_listener, show_menu_marker) {
+                         views::MenuButtonListener* menu_button_listener)
+      : MenuButton(title, menu_button_listener) {
     SetImageLabelSpacing(ChromeLayoutProvider::Get()->GetDistanceMetric(
         DISTANCE_RELATED_LABEL_HORIZONTAL_LIST));
     SetInkDropMode(InkDropMode::ON);
@@ -365,9 +364,8 @@ class BookmarkMenuButtonBase : public views::MenuButton {
 class BookmarkFolderButton : public BookmarkMenuButtonBase {
  public:
   BookmarkFolderButton(const base::string16& title,
-                       views::MenuButtonListener* menu_button_listener,
-                       bool show_menu_marker)
-      : BookmarkMenuButtonBase(title, menu_button_listener, show_menu_marker) {
+                       views::MenuButtonListener* menu_button_listener)
+      : BookmarkMenuButtonBase(title, menu_button_listener) {
     show_animation_.reset(new gfx::SlideAnimation(this));
     if (!animations_enabled) {
       // For some reason during testing the events generated by animating
@@ -415,7 +413,7 @@ class BookmarkFolderButton : public BookmarkMenuButtonBase {
 class OverflowButton : public BookmarkMenuButtonBase {
  public:
   explicit OverflowButton(BookmarkBarView* owner)
-      : BookmarkMenuButtonBase(base::string16(), owner, false), owner_(owner) {}
+      : BookmarkMenuButtonBase(base::string16(), owner), owner_(owner) {}
 
   bool OnMousePressed(const ui::MouseEvent& e) override {
     owner_->StopThrobbing(true);
@@ -1565,7 +1563,7 @@ int BookmarkBarView::GetFirstHiddenNodeIndex() {
 
 MenuButton* BookmarkBarView::CreateOtherBookmarksButton() {
   // Title is set in Loaded.
-  MenuButton* button = new BookmarkFolderButton(base::string16(), this, false);
+  MenuButton* button = new BookmarkFolderButton(base::string16(), this);
   button->set_id(VIEW_ID_OTHER_BOOKMARKS);
   button->set_context_menu_controller(this);
   return button;
@@ -1573,7 +1571,7 @@ MenuButton* BookmarkBarView::CreateOtherBookmarksButton() {
 
 MenuButton* BookmarkBarView::CreateManagedBookmarksButton() {
   // Title is set in Loaded.
-  MenuButton* button = new BookmarkFolderButton(base::string16(), this, false);
+  MenuButton* button = new BookmarkFolderButton(base::string16(), this);
   button->set_id(VIEW_ID_MANAGED_BOOKMARKS);
   button->set_context_menu_controller(this);
   return button;
@@ -1605,7 +1603,7 @@ views::View* BookmarkBarView::CreateBookmarkButton(const BookmarkNode* node) {
   if (node->is_url()) {
     button = new BookmarkButton(this, node->url(), node->GetTitle());
   } else {
-    button = new BookmarkFolderButton(node->GetTitle(), this, false);
+    button = new BookmarkFolderButton(node->GetTitle(), this);
   }
   ConfigureButton(node, button);
   bookmark_buttons_.insert(bookmark_buttons_.cbegin() + index, button);

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

@@ -12,7 +12,7 @@
 #include "ui/views/controls/menu/menu_listener.h"
 
 AppMenuButton::AppMenuButton(views::MenuButtonListener* menu_button_listener)
-    : views::MenuButton(base::string16(), menu_button_listener, false) {}
+    : views::MenuButton(base::string16(), menu_button_listener) {}
 
 AppMenuButton::~AppMenuButton() {}
 

chrome/browser/ui/views/hover_button.cc

@@ -65,7 +65,7 @@ void SetTooltipAndAccessibleName(views::Button* parent,
 
 HoverButton::HoverButton(views::ButtonListener* button_listener,
                          const base::string16& text)
-    : views::MenuButton(text, this, false),
+    : views::MenuButton(text, this),
       title_(nullptr),
       subtitle_(nullptr),
       icon_view_(nullptr),

chrome/browser/ui/views/menu_model_adapter_test.cc

@@ -66,12 +66,12 @@ class CommonMenuModel : public ui::MenuModel {
   bool GetIconAt(int index, gfx::Image* icon) override { return false; }
 
   ui::ButtonMenuItemModel* GetButtonMenuItemAt(int index) const override {
-    return NULL;
+    return nullptr;
   }
 
   bool IsEnabledAt(int index) const override { return true; }
 
-  ui::MenuModel* GetSubmenuModelAt(int index) const override { return NULL; }
+  ui::MenuModel* GetSubmenuModelAt(int index) const override { return nullptr; }
 
   void HighlightChangedTo(int index) override {}
 
@@ -79,7 +79,9 @@ class CommonMenuModel : public ui::MenuModel {
 
   void SetMenuModelDelegate(ui::MenuModelDelegate* delegate) override {}
 
-  ui::MenuModelDelegate* GetMenuModelDelegate() const override { return NULL; }
+  ui::MenuModelDelegate* GetMenuModelDelegate() const override {
+    return nullptr;
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CommonMenuModel);
@@ -162,18 +164,17 @@ class MenuModelAdapterTest : public ViewEventTestBase,
  public:
   MenuModelAdapterTest()
       : ViewEventTestBase(),
-        button_(NULL),
+        button_(nullptr),
         menu_model_adapter_(&top_menu_model_),
-        menu_(NULL) {
-  }
+        menu_(nullptr) {}
 
   ~MenuModelAdapterTest() override {}
 
   // ViewEventTestBase implementation.
 
   void SetUp() override {
-    button_ = new views::MenuButton(base::ASCIIToUTF16("Menu Adapter Test"),
-                                    this, true);
+    button_ =
+        new views::MenuButton(base::ASCIIToUTF16("Menu Adapter Test"), this);
 
     menu_ = menu_model_adapter_.CreateMenu();
     menu_runner_.reset(
@@ -183,8 +184,8 @@ class MenuModelAdapterTest : public ViewEventTestBase,
   }
 
   void TearDown() override {
-    menu_runner_.reset(NULL);
-    menu_ = NULL;
+    menu_runner_ = nullptr;
+    menu_ = nullptr;
     ViewEventTestBase::TearDown();
   }
 

chrome/browser/ui/views/menu_test_base.cc

@@ -46,7 +46,7 @@ int MenuTestBase::GetMenuRunnerFlags() {
 void MenuTestBase::SetUp() {
   views::test::DisableMenuClosureAnimations();
 
-  button_ = new views::MenuButton(base::ASCIIToUTF16("Menu Test"), this, true);
+  button_ = new views::MenuButton(base::ASCIIToUTF16("Menu Test"), this);
   menu_ = new views::MenuItemView(this);
   BuildMenu(menu_);
   menu_runner_.reset(new views::MenuRunner(menu_, GetMenuRunnerFlags()));

chrome/browser/ui/views/page_info/permission_selector_row.cc

@@ -180,9 +180,7 @@ PermissionSelectorRow::PermissionSelectorRow(
     const GURL& url,
     const PageInfoUI::PermissionInfo& permission,
     views::GridLayout* layout)
-    : profile_(profile),
-      icon_(nullptr),
-      combobox_(nullptr) {
+    : profile_(profile), icon_(nullptr), combobox_(nullptr) {
   const int list_item_padding = ChromeLayoutProvider::Get()->GetDistanceMetric(
                                     DISTANCE_CONTROL_LIST_VERTICAL) /
                                 2;

chrome/browser/ui/views/tab_icon_view.cc

@@ -56,7 +56,7 @@ void TabIconView::InitializeIfNeeded() {
 
 TabIconView::TabIconView(TabIconViewModel* model,
                          views::MenuButtonListener* listener)
-    : views::MenuButton(base::string16(), listener, false),
+    : views::MenuButton(base::string16(), listener),
       model_(model),
       is_light_(false) {
   InitializeIfNeeded();

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

@@ -51,7 +51,7 @@ const int kBorderInset = 0;
 ToolbarActionView::ToolbarActionView(
     ToolbarActionViewController* view_controller,
     ToolbarActionView::Delegate* delegate)
-    : MenuButton(base::string16(), this, false),
+    : MenuButton(base::string16(), this),
       view_controller_(view_controller),
       delegate_(delegate),
       called_register_command_(false),

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

@@ -272,7 +272,7 @@ TEST_F(ToolbarActionViewUnitTest, BasicToolbarActionViewTest) {
   EXPECT_FALSE(view.wants_to_run_for_testing());
 
   // Create an overflow button.
-  views::MenuButton overflow_button(base::string16(), nullptr, false);
+  views::MenuButton overflow_button(base::string16(), nullptr);
   overflow_button.set_owned_by_client();
   action_view_delegate.set_overflow_reference_view(&overflow_button);
 

ui/resources/ui_resources.grd

@@ -118,7 +118,6 @@
       <if expr="is_macosx or is_ios">
         <structure type="chrome_scaled_image" name="IDR_MENU_HIERARCHY_ARROW" file="mac/menu_hierarchy_arrow.png" />
       </if>
-      <structure type="chrome_scaled_image" name="IDR_MENU_DROPARROW" file="cros/menu_droparrow.png" />
       <if expr="toolkit_views or is_ios">
         <if expr="is_win">
           <structure type="chrome_scaled_image" name="IDR_NOTIFICATION_CLOSE" file="win/notification_close.png"/>

ui/views/controls/button/button_unittest.cc

@@ -364,10 +364,10 @@ TEST_F(ButtonTest, GestureEventsSetState) {
 TEST_F(ButtonTest, AsButton) {
   base::string16 text;
 
-  LabelButton label_button(NULL, text);
+  LabelButton label_button(nullptr, text);
   EXPECT_TRUE(Button::AsButton(&label_button));
 
-  ImageButton image_button(NULL);
+  ImageButton image_button(nullptr);
   EXPECT_TRUE(Button::AsButton(&image_button));
 
   Checkbox checkbox(text);
@@ -376,10 +376,10 @@ TEST_F(ButtonTest, AsButton) {
   RadioButton radio_button(text, 0);
   EXPECT_TRUE(Button::AsButton(&radio_button));
 
-  MenuButton menu_button(text, NULL, false);
+  MenuButton menu_button(text, nullptr);
   EXPECT_TRUE(Button::AsButton(&menu_button));
 
-  ToggleButton toggle_button(NULL);
+  ToggleButton toggle_button(nullptr);
   EXPECT_TRUE(Button::AsButton(&toggle_button));
 
   Label label;

ui/views/controls/button/menu_button.cc

@@ -7,15 +7,12 @@
 #include "base/strings/utf_string_conversions.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/display/screen.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/image/image.h"
 #include "ui/gfx/text_constants.h"
-#include "ui/resources/grit/ui_resources.h"
 #include "ui/views/animation/ink_drop.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
@@ -65,15 +62,10 @@ MenuButton::PressedLock::~PressedLock() {
 ////////////////////////////////////////////////////////////////////////////////
 
 MenuButton::MenuButton(const base::string16& text,
-                       MenuButtonListener* menu_button_listener,
-                       bool show_menu_marker)
+                       MenuButtonListener* menu_button_listener)
     : 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()),
       weak_factory_(this) {
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
 }
@@ -176,16 +168,6 @@ bool MenuButton::IsTriggerableEventType(const ui::Event& event) {
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-gfx::Size MenuButton::CalculatePreferredSize() const {
-  gfx::Size prefsize = LabelButton::CalculatePreferredSize();
-  if (show_menu_marker_) {
-    prefsize.Enlarge(menu_marker_->width() + kMenuMarkerPaddingLeft +
-                         kMenuMarkerPaddingRight,
-                     0);
-  }
-  return prefsize;
-}
-
 const char* MenuButton::GetClassName() const {
   return kViewClassName;
 }
@@ -293,31 +275,6 @@ void MenuButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
     node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kOpen);
 }
 
-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());
-  arrow_bounds.set_x(GetMirroredXForRect(arrow_bounds));
-  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);
-}
-
 bool MenuButton::IsTriggerableEvent(const ui::Event& event) {
   if (!IsTriggerableEventType(event))
     return false;
@@ -354,11 +311,6 @@ void MenuButton::NotifyClick(const ui::Event& event) {
   Activate(&event);
 }
 
-void MenuButton::PaintButtonContents(gfx::Canvas* canvas) {
-  if (show_menu_marker_)
-    PaintMenuMarker(canvas);
-}
-
 void MenuButton::IncrementPressedLocked(bool snap_ink_drop_to_activated,
                                         const ui::LocatedEvent* event) {
   ++pressed_lock_count_;

ui/views/controls/button/menu_button.h

@@ -52,16 +52,9 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
 
   // Create a Button.
   MenuButton(const base::string16& text,
-             MenuButtonListener* menu_button_listener,
-             bool show_menu_marker);
+             MenuButtonListener* menu_button_listener);
   ~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); }
 
@@ -76,7 +69,6 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
   virtual bool IsTriggerableEventType(const ui::Event& event);
 
   // Overridden from View:
-  gfx::Size CalculatePreferredSize() const override;
   const char* GetClassName() const override;
   bool OnMousePressed(const ui::MouseEvent& event) override;
   void OnMouseReleased(const ui::MouseEvent& event) override;
@@ -89,18 +81,12 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
  protected:
-  // Paint the menu marker image.
-  void PaintMenuMarker(gfx::Canvas* canvas);
-
-  // Overridden from LabelButton:
-  gfx::Rect GetChildAreaBounds() override;
 
   // Overridden from Button:
   bool IsTriggerableEvent(const ui::Event& event) override;
   bool ShouldEnterPushedState(const ui::Event& event) override;
   void StateChanged(ButtonState old_state) override;
   void NotifyClick(const ui::Event& event) override;
-  void PaintButtonContents(gfx::Canvas* canvas) override;
 
   // Offset of the associated menu position.
   gfx::Point menu_offset_;
@@ -132,12 +118,6 @@ class VIEWS_EXPORT MenuButton : public LabelButton {
   // Our listener. Not owned.
   MenuButtonListener* listener_;
 
-  // 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_;
-
   // The current number of "pressed" locks this button has.
   int pressed_lock_count_ = 0;
 

ui/views/controls/button/menu_button_unittest.cc

@@ -38,8 +38,7 @@ class TestMenuButton : public MenuButton {
  public:
   explicit TestMenuButton(MenuButtonListener* menu_button_listener)
       : MenuButton(base::string16(ASCIIToUTF16("button")),
-                   menu_button_listener,
-                   false) {}
+                   menu_button_listener) {}
 
   ~TestMenuButton() override {}
 
@@ -693,7 +692,7 @@ TEST_F(MenuButtonTest,
 class DestroyButtonInGestureListener : public MenuButtonListener {
  public:
   DestroyButtonInGestureListener() {
-    menu_button_ = std::make_unique<MenuButton>(base::string16(), this, true);
+    menu_button_ = std::make_unique<MenuButton>(base::string16(), this);
   }
 
   ~DestroyButtonInGestureListener() override = default;

ui/views/examples/menu_example.cc

@@ -171,10 +171,9 @@ void ExampleMenuModel::ExecuteCommand(int command_id, int event_flags) {
 // ExampleMenuButton -----------------------------------------------------------
 
 ExampleMenuButton::ExampleMenuButton(const base::string16& test)
-    : MenuButton(test, this, true) {}
+    : MenuButton(test, this) {}
 
-ExampleMenuButton::~ExampleMenuButton() {
-}
+ExampleMenuButton::~ExampleMenuButton() {}
 
 void ExampleMenuButton::OnMenuButtonClicked(MenuButton* source,
                                             const gfx::Point& point,