Use callbacks instead of ButtonPressed overrides: ash/clipboard/

Bug: 772945
Change-Id: I1f347bc404e5b347159c64cdb9ff0e45f3903590
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425153
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#809894}

ash/clipboard/views/clipboard_history_item_view.cc

@@ -35,49 +35,10 @@ constexpr gfx::Insets kContentsInsets(/*vertical=*/4, /*horizontal=*/16);
 // The size of the `DeleteButton`.
 constexpr int kDeleteButtonSizeDip = 16;
 
-// The view responding to mouse click or gesture tap events.
-class MainButton : public views::Button {
- public:
-  explicit MainButton(ash::ClipboardHistoryItemView* container)
-      : Button(container), container_(container) {
-    SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
-    SetAccessibleName(base::ASCIIToUTF16(std::string(GetClassName())));
-  }
-  MainButton(const MainButton& rhs) = delete;
-  MainButton& operator=(const MainButton& rhs) = delete;
-  ~MainButton() override = default;
-
- private:
-  // views::Button:
-  const char* GetClassName() const override { return "MainButton"; }
-
-  void PaintButtonContents(gfx::Canvas* canvas) override {
-    if (!container_->IsSelected())
-      return;
-
-    // Highlight the background when the menu item is selected or pressed.
-    cc::PaintFlags flags;
-    flags.setAntiAlias(true);
-
-    const ui::NativeTheme::ColorId color_id =
-        ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor;
-    flags.setColor(GetNativeTheme()->GetSystemColor(color_id));
-
-    flags.setStyle(cc::PaintFlags::kFill_Style);
-    canvas->DrawRect(GetLocalBounds(), flags);
-  }
-
-  // The parent view.
-  ash::ClipboardHistoryItemView* const container_;
-};
-
 }  // namespace
 
 namespace ash {
 
-////////////////////////////////////////////////////////////////////////////////
-// ClipboardHistoryItemView::ContentsView
-
 ClipboardHistoryItemView::ContentsView::ContentsView(
     ClipboardHistoryItemView* container)
     : container_(container) {
@@ -111,12 +72,48 @@ bool ClipboardHistoryItemView::ContentsView::DoesIntersectRect(
       gfx::ToEnclosedRect(rect_in_delete_button));
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// ClipboardHistoryItemView::DeleteButton
+// The view responding to mouse click or gesture tap events.
+class ash::ClipboardHistoryItemView::MainButton : public views::Button {
+ public:
+  explicit MainButton(ClipboardHistoryItemView* container)
+      : Button(), container_(container) {
+    SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+    SetAccessibleName(base::ASCIIToUTF16(std::string(GetClassName())));
+  }
+  MainButton(const MainButton& rhs) = delete;
+  MainButton& operator=(const MainButton& rhs) = delete;
+  ~MainButton() override = default;
+
+ private:
+  // views::Button:
+  const char* GetClassName() const override { return "MainButton"; }
+
+  void PaintButtonContents(gfx::Canvas* canvas) override {
+    if (!container_->IsSelected())
+      return;
+
+    // Highlight the background when the menu item is selected or pressed.
+    cc::PaintFlags flags;
+    flags.setAntiAlias(true);
+
+    const ui::NativeTheme::ColorId color_id =
+        ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor;
+    flags.setColor(GetNativeTheme()->GetSystemColor(color_id));
+
+    flags.setStyle(cc::PaintFlags::kFill_Style);
+    canvas->DrawRect(GetLocalBounds(), flags);
+  }
+
+  // The parent view.
+  ash::ClipboardHistoryItemView* const container_;
+};
 
 ClipboardHistoryItemView::DeleteButton::DeleteButton(
-    views::ButtonListener* listener)
-    : views::ImageButton(listener) {
+    ClipboardHistoryItemView* listener)
+    : views::ImageButton() {
+  set_callback(base::BindRepeating(&ClipboardHistoryItemView::ExecuteCommand,
+                                   base::Unretained(listener),
+                                   ClipboardHistoryUtil::kDeleteCommandId));
   SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
   SetAccessibleName(base::ASCIIToUTF16(std::string(GetClassName())));
   SetImageHorizontalAlignment(views::ImageButton::ALIGN_CENTER);
@@ -134,9 +131,6 @@ const char* ClipboardHistoryItemView::DeleteButton::GetClassName() const {
   return "DeleteButton";
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// ClipboardHistoryItemView
-
 // static
 std::unique_ptr<ClipboardHistoryItemView>
 ClipboardHistoryItemView::CreateFromClipboardHistoryItem(
@@ -169,6 +163,12 @@ void ClipboardHistoryItemView::Init() {
 
   // Ensures that MainButton is below any other child views.
   main_button_ = AddChildView(std::make_unique<MainButton>(this));
+  main_button_->set_callback(base::BindRepeating(
+      [](ClipboardHistoryItemView* item, views::MenuItemView* container,
+         const ui::Event& event) {
+        item->ExecuteCommand(container->GetCommand(), event);
+      },
+      base::Unretained(this), container_));
 
   contents_view_ = AddChildView(CreateContentsView());
 
@@ -195,15 +195,11 @@ gfx::Size ClipboardHistoryItemView::CalculatePreferredSize() const {
   return gfx::Size(preferred_width, GetHeightForWidth(preferred_width));
 }
 
-void ClipboardHistoryItemView::ButtonPressed(views::Button* sender,
+void ClipboardHistoryItemView::ExecuteCommand(int command_id,
                                               const ui::Event& event) {
-  DCHECK(sender == contents_view_->delete_button() || sender == main_button_);
-  const int command_id = sender == contents_view_->delete_button()
-                             ? ClipboardHistoryUtil::kDeleteCommandId
-                             : container_->GetCommand();
   views::MenuDelegate* delegate = container_->GetDelegate();
   DCHECK(delegate->IsCommandEnabled(command_id));
-  delegate->ExecuteCommand(command_id, event.flags());
+  container_->GetDelegate()->ExecuteCommand(command_id, event.flags());
 }
 
 }  // namespace ash

ash/clipboard/views/clipboard_history_item_view.h

@@ -17,8 +17,7 @@ class ClipboardHistoryItem;
 class ClipboardHistoryResourceManager;
 
 // The base class for menu items of the clipboard history menu.
-class ClipboardHistoryItemView : public views::View,
-                                 public views::ButtonListener {
+class ClipboardHistoryItemView : public views::View {
  public:
   static std::unique_ptr<ClipboardHistoryItemView>
   CreateFromClipboardHistoryItem(
@@ -41,10 +40,12 @@ class ClipboardHistoryItemView : public views::View,
   void OnSelectionChanged();
 
  protected:
+  class MainButton;
+
   // The button to delete the menu item and its corresponding clipboard data.
   class DeleteButton : public views::ImageButton {
    public:
-    explicit DeleteButton(views::ButtonListener* listener);
+    explicit DeleteButton(ClipboardHistoryItemView* listener);
     DeleteButton(const DeleteButton& rhs) = delete;
     DeleteButton& operator=(const DeleteButton& rhs) = delete;
     ~DeleteButton() override;
@@ -98,14 +99,14 @@ class ClipboardHistoryItemView : public views::View,
   // views::View:
   gfx::Size CalculatePreferredSize() const override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // Executes |command_id| on the delegate.
+  void ExecuteCommand(int command_id, const ui::Event& event);
 
   views::MenuItemView* const container_;
 
   ContentsView* contents_view_ = nullptr;
 
-  views::View* main_button_ = nullptr;
+  MainButton* main_button_ = nullptr;
 
   views::PropertyChangedSubscription subscription_;
 };