Change ButtonPressed overrides to callbacks: ui/views/touchui/

Bug: 772945
Change-Id: Ibfb794373df3f30e88c645ae7678cd3ccb3f9fdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2485040Reviewed-by: Mohsen Izadi <mohsen@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819029}

chrome/browser/ui/views/touch_selection_menu_chromeos.cc

@@ -51,10 +51,10 @@ void TouchSelectionMenuChromeOS::SetActionsForTesting(
 
 void TouchSelectionMenuChromeOS::CreateButtons() {
   if (action_) {
-    views::LabelButton* button =
-        CreateButton(base::UTF8ToUTF16(action_->title));
-    button->set_tag(kSmartTextSelectionActionTag);
-
+    views::LabelButton* button = CreateButton(
+        base::UTF8ToUTF16(action_->title),
+        base::BindRepeating(&TouchSelectionMenuChromeOS::ActionButtonPressed,
+                            base::Unretained(this)));
     if (action_->bitmap_icon) {
       gfx::ImageSkia original(
           gfx::ImageSkia::CreateFrom1xBitmap(action_->bitmap_icon.value()));
@@ -63,20 +63,21 @@ void TouchSelectionMenuChromeOS::CreateButtons() {
           gfx::Size(kSmallIconSizeInDip, kSmallIconSizeInDip));
       button->SetImage(views::Button::ButtonState::STATE_NORMAL, icon);
     }
-
-    AddChildView(button);
   }
 
   views::TouchSelectionMenuViews::CreateButtons();
 }
 
-void TouchSelectionMenuChromeOS::ButtonPressed(views::Button* sender,
-                                               const ui::Event& event) {
-  if (sender->tag() != kSmartTextSelectionActionTag) {
-    views::TouchSelectionMenuViews::ButtonPressed(sender, event);
-    return;
-  }
+void TouchSelectionMenuChromeOS::OnBeforeBubbleWidgetInit(
+    views::Widget::InitParams* params,
+    views::Widget* widget) const {
+  ash_util::SetupWidgetInitParamsForContainer(
+      params, ash::kShellWindowId_SettingBubbleContainer);
+}
 
+TouchSelectionMenuChromeOS::~TouchSelectionMenuChromeOS() = default;
+
+void TouchSelectionMenuChromeOS::ActionButtonPressed() {
   auto* arc_service_manager = arc::ArcServiceManager::Get();
   if (!arc_service_manager)
     return;
@@ -88,12 +89,3 @@ void TouchSelectionMenuChromeOS::ButtonPressed(views::Button* sender,
   instance->HandleIntent(std::move(action_->action_intent),
                          std::move(action_->activity));
 }
-
-void TouchSelectionMenuChromeOS::OnBeforeBubbleWidgetInit(
-    views::Widget::InitParams* params,
-    views::Widget* widget) const {
-  ash_util::SetupWidgetInitParamsForContainer(
-      params, ash::kShellWindowId_SettingBubbleContainer);
-}
-
-TouchSelectionMenuChromeOS::~TouchSelectionMenuChromeOS() = default;

chrome/browser/ui/views/touch_selection_menu_chromeos.h

@@ -19,10 +19,6 @@ class TouchSelectionMenuRunnerViews;
 // user's text selection.
 class TouchSelectionMenuChromeOS : public views::TouchSelectionMenuViews {
  public:
-  // Tag used to mark added actions/buttons as generated by the smart text
-  // selection feature.
-  enum { kSmartTextSelectionActionTag = -2 };
-
   TouchSelectionMenuChromeOS(views::TouchSelectionMenuRunnerViews* owner,
                              ui::TouchSelectionMenuClient* client,
                              aura::Window* context,
@@ -34,13 +30,14 @@ class TouchSelectionMenuChromeOS : public views::TouchSelectionMenuViews {
  protected:
   // views:TouchSelectionMenuViews.
   void CreateButtons() override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   void OnBeforeBubbleWidgetInit(views::Widget::InitParams* params,
                                 views::Widget* widget) const override;
 
  private:
   ~TouchSelectionMenuChromeOS() override;
 
+  void ActionButtonPressed();
+
   arc::mojom::TextSelectionActionPtr action_;
   int64_t display_id_;
 

ui/views/touchui/touch_selection_menu_views.cc

@@ -5,6 +5,7 @@
 #include "ui/views/touchui/touch_selection_menu_views.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -123,27 +124,33 @@ TouchSelectionMenuViews::~TouchSelectionMenuViews() = default;
 
 void TouchSelectionMenuViews::CreateButtons() {
   for (const auto& command : kMenuCommands) {
-    if (!client_->IsCommandIdEnabled(command.command_id))
-      continue;
-
-    Button* button =
-        CreateButton(l10n_util::GetStringUTF16(command.message_id));
-    button->set_tag(command.command_id);
-    AddChildView(button);
+    if (client_->IsCommandIdEnabled(command.command_id)) {
+      CreateButton(
+          l10n_util::GetStringUTF16(command.message_id),
+          base::BindRepeating(&TouchSelectionMenuViews::ButtonPressed,
+                              base::Unretained(this), command.command_id));
+    }
   }
 
   // Finally, add ellipsis button.
-  LabelButton* ellipsis_button = CreateButton(base::ASCIIToUTF16("..."));
-  ellipsis_button->SetID(ButtonViewId::kEllipsisButton);
-  AddChildView(ellipsis_button);
+  CreateButton(base::ASCIIToUTF16("..."),
+               base::BindRepeating(
+                   [](TouchSelectionMenuViews* menu) {
+                     menu->CloseMenu();
+                     menu->client_->RunContextMenu();
+                   },
+                   base::Unretained(this)))
+      ->SetID(ButtonViewId::kEllipsisButton);
   InvalidateLayout();
 }
 
 LabelButton* TouchSelectionMenuViews::CreateButton(
-    const base::string16& title) {
+    const base::string16& title,
+    Button::PressedCallback callback) {
   base::string16 label =
       gfx::RemoveAcceleratorChar(title, '&', nullptr, nullptr);
-  LabelButton* button = new LabelButton(this, label, style::CONTEXT_TOUCH_MENU);
+  auto* button = AddChildView(std::make_unique<LabelButton>(
+      std::move(callback), label, style::CONTEXT_TOUCH_MENU));
   constexpr gfx::Size kMenuButtonMinSize = gfx::Size(63, 38);
   button->SetMinSize(kMenuButtonMinSize);
   button->SetFocusForPlatform();
@@ -179,13 +186,10 @@ void TouchSelectionMenuViews::WindowClosing() {
     DisconnectOwner();
 }
 
-void TouchSelectionMenuViews::ButtonPressed(Button* sender,
+void TouchSelectionMenuViews::ButtonPressed(int command,
                                             const ui::Event& event) {
   CloseMenu();
-  if (sender->GetID() != ButtonViewId::kEllipsisButton)
-    client_->ExecuteCommand(sender->tag(), event.flags());
-  else
-    client_->RunContextMenu();
+  client_->ExecuteCommand(command, event.flags());
 }
 
 BEGIN_METADATA(TouchSelectionMenuViews, BubbleDialogDelegateView)

ui/views/touchui/touch_selection_menu_views.h

@@ -7,7 +7,6 @@
 
 #include "base/macros.h"
 #include "ui/views/bubble/bubble_dialog_delegate_view.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/touchui/touch_selection_menu_runner_views.h"
 
 namespace ui {
@@ -19,8 +18,7 @@ class LabelButton;
 
 // A bubble that contains actions available for the selected text. An object of
 // this type, as a BubbleDialogDelegateView, manages its own lifetime.
-class VIEWS_EXPORT TouchSelectionMenuViews : public BubbleDialogDelegateView,
-                                             public ButtonListener {
+class VIEWS_EXPORT TouchSelectionMenuViews : public BubbleDialogDelegateView {
  public:
   METADATA_HEADER(TouchSelectionMenuViews);
 
@@ -47,14 +45,14 @@ class VIEWS_EXPORT TouchSelectionMenuViews : public BubbleDialogDelegateView,
   virtual void CreateButtons();
 
   // Helper method to create a single button.
-  LabelButton* CreateButton(const base::string16& title);
-
-  // ButtonListener:
-  void ButtonPressed(Button* sender, const ui::Event& event) override;
+  LabelButton* CreateButton(const base::string16& title,
+                            Button::PressedCallback callback);
 
  private:
   friend class TouchSelectionMenuRunnerViews::TestApi;
 
+  void ButtonPressed(int command, const ui::Event& event);
+
   // Helper to disconnect this menu object from its owning menu runner.
   void DisconnectOwner();