Change ButtonPressed overrides to callbacks: ash/search_box/

Bug: 772945
Change-Id: If7c1862ff6b88af98be69d09722e3a086bd030e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2516251
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824119}

ash/app_list/views/search_box_view.cc

@@ -227,6 +227,13 @@ int SearchBoxView::GetFocusRingSpacing() {
 
 void SearchBoxView::SetupCloseButton() {
   views::ImageButton* close = close_button();
+  close->SetCallback(base::BindRepeating(
+      [](SearchBoxView* view) {
+        view->view_delegate_->LogSearchAbandonHistogram();
+        view->SetSearchBoxActive(false, ui::ET_UNKNOWN);
+        view->ClearSearch();
+      },
+      this));
   close->SetImage(
       views::ImageButton::STATE_NORMAL,
       gfx::CreateVectorIcon(views::kIcCloseIcon, kSearchBoxIconSize,
@@ -748,15 +755,6 @@ bool SearchBoxView::HandleGestureEvent(views::Textfield* sender,
   return SearchBoxViewBase::HandleGestureEvent(sender, gesture_event);
 }
 
-void SearchBoxView::ButtonPressed(views::Button* sender,
-                                  const ui::Event& event) {
-  if (close_button() && sender == close_button()) {
-    view_delegate_->LogSearchAbandonHistogram();
-    SetSearchBoxActive(false, ui::ET_UNKNOWN);
-  }
-  SearchBoxViewBase::ButtonPressed(sender, event);
-}
-
 void SearchBoxView::UpdateSearchBoxTextForSelectedResult(
     SearchResult* selected_result) {
   if (selected_result->result_type() ==

ash/app_list/views/search_box_view.h

@@ -69,9 +69,6 @@ class APP_LIST_EXPORT SearchBoxView : public SearchBoxViewBase,
   void OnPaintBackground(gfx::Canvas* canvas) override;
   const char* GetClassName() const override;
 
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // Updates the search box's background corner radius and color based on the
   // state of AppListModel.
   void UpdateBackground(double progress,

ash/app_list/views/search_box_view_unittest.cc

@@ -32,6 +32,7 @@
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/test/widget_test.h"
 
 namespace ash {
@@ -211,33 +212,30 @@ TEST_F(SearchBoxViewTest, CloseButtonIVisibleInZeroStateSearchBox) {
 // Tests that the close button becomes invisible after close button is clicked.
 TEST_F(SearchBoxViewTest, CloseButtonInvisibleAfterCloseButtonClicked) {
   KeyPress(ui::VKEY_A);
-  view()->ButtonPressed(
-      view()->close_button(),
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                     base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON,
-                     ui::EF_LEFT_MOUSE_BUTTON));
+  views::test::ButtonTestApi(view()->close_button())
+      .NotifyClick(ui::MouseEvent(
+          ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), base::TimeTicks(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
   EXPECT_FALSE(view()->close_button()->GetVisible());
 }
 
 // Tests that the search box becomes empty after close button is clicked.
 TEST_F(SearchBoxViewTest, SearchBoxEmptyAfterCloseButtonClicked) {
   KeyPress(ui::VKEY_A);
-  view()->ButtonPressed(
-      view()->close_button(),
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                     base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON,
-                     ui::EF_LEFT_MOUSE_BUTTON));
+  views::test::ButtonTestApi(view()->close_button())
+      .NotifyClick(ui::MouseEvent(
+          ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), base::TimeTicks(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
   EXPECT_TRUE(view()->search_box()->GetText().empty());
 }
 
 // Tests that the search box is no longer active after close button is clicked.
 TEST_F(SearchBoxViewTest, SearchBoxActiveAfterCloseButtonClicked) {
   KeyPress(ui::VKEY_A);
-  view()->ButtonPressed(
-      view()->close_button(),
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                     base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON,
-                     ui::EF_LEFT_MOUSE_BUTTON));
+  views::test::ButtonTestApi(view()->close_button())
+      .NotifyClick(ui::MouseEvent(
+          ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), base::TimeTicks(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
   EXPECT_FALSE(view()->is_search_box_active());
 }
 

ash/search_box/search_box_view_base.cc

@@ -81,8 +81,8 @@ class SearchBoxBackground : public views::Background {
 // fullscreen launcher.
 class SearchBoxImageButton : public views::ImageButton {
  public:
-  explicit SearchBoxImageButton(views::ButtonListener* listener)
-      : ImageButton(listener) {
+  explicit SearchBoxImageButton(PressedCallback callback)
+      : ImageButton(std::move(callback)) {
     SetFocusBehavior(FocusBehavior::ALWAYS);
 
     // Avoid drawing default dashed focus and draw customized focus in
@@ -253,7 +253,8 @@ SearchBoxViewBase::SearchBoxViewBase(SearchBoxViewDelegate* delegate)
   search_box_->SetTextInputType(ui::TEXT_INPUT_TYPE_SEARCH);
   search_box_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
 
-  back_button_ = new SearchBoxImageButton(this);
+  back_button_ = new SearchBoxImageButton(base::BindRepeating(
+      &SearchBoxViewDelegate::BackButtonPressed, base::Unretained(delegate_)));
   content_container_->AddChildView(back_button_);
 
   search_icon_ = new views::ImageView();
@@ -272,13 +273,16 @@ SearchBoxViewBase::SearchBoxViewBase(SearchBoxViewDelegate* delegate)
   search_box_right_space_->SetPreferredSize(gfx::Size(kSearchBoxIconSize, 0));
   content_container_->AddChildView(search_box_right_space_);
 
-  assistant_button_ = new SearchBoxImageButton(this);
+  assistant_button_ = new SearchBoxImageButton(
+      base::BindRepeating(&SearchBoxViewDelegate::AssistantButtonPressed,
+                          base::Unretained(delegate_)));
   assistant_button_->SetFlipCanvasOnPaintForRTLUI(false);
   // Default hidden, child class should decide if it should shown.
   assistant_button_->SetVisible(false);
   content_container_->AddChildView(assistant_button_);
 
-  close_button_ = new SearchBoxImageButton(this);
+  close_button_ = new SearchBoxImageButton(base::BindRepeating(
+      &SearchBoxViewBase::ClearSearch, base::Unretained(this)));
   content_container_->AddChildView(close_button_);
 }
 
@@ -402,19 +406,6 @@ ax::mojom::Role SearchBoxViewBase::GetAccessibleWindowRole() {
   return ax::mojom::Role::kGroup;
 }
 
-void SearchBoxViewBase::ButtonPressed(views::Button* sender,
-                                      const ui::Event& event) {
-  if (assistant_button_ && sender == assistant_button_) {
-    delegate_->AssistantButtonPressed();
-  } else if (back_button_ && sender == back_button_) {
-    delegate_->BackButtonPressed();
-  } else if (close_button_ && sender == close_button_) {
-    ClearSearch();
-  } else {
-    NOTREACHED();
-  }
-}
-
 void SearchBoxViewBase::OnSearchBoxFocusedChanged() {
   UpdateSearchBoxBorder();
   Layout();

ash/search_box/search_box_view_base.h

@@ -51,8 +51,7 @@ enum class ActivationSource {
 // a close icon and a back icon for different functionalities. This class
 // provides common functions for the search box view across Chrome OS.
 class SearchBoxViewBase : public views::WidgetDelegateView,
-                          public views::TextfieldController,
-                          public views::ButtonListener {
+                          public views::TextfieldController {
  public:
   explicit SearchBoxViewBase(SearchBoxViewDelegate* delegate);
   ~SearchBoxViewBase() override;
@@ -96,9 +95,6 @@ class SearchBoxViewBase : public views::WidgetDelegateView,
   // Overridden from views::WidgetDelegate:
   ax::mojom::Role GetAccessibleWindowRole() override;
 
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // Used only in the tests to get the current search icon.
   views::ImageView* get_search_icon_for_test() { return search_icon_; }
 

ash/shortcut_viewer/BUILD.gn

@@ -54,5 +54,6 @@ source_set("unit_tests") {
     "//ui/events:test_support",
     "//ui/events/devices:test_support",
     "//ui/views",
+    "//ui/views:test_support",
   ]
 }

ash/shortcut_viewer/views/keyboard_shortcut_view_unittest.cc

@@ -21,6 +21,7 @@
 #include "ui/events/devices/device_data_manager_test_api.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/views/controls/textfield/textfield.h"
+#include "ui/views/test/button_test_api.h"
 #include "ui/views/widget/widget.h"
 
 namespace keyboard_shortcut_viewer {
@@ -189,11 +190,10 @@ TEST_F(KeyboardShortcutViewTest, FocusOnSearchBox) {
 
   // Case 2: Exit search mode by clicking |close_button|. The focus should be on
   // search box.
-  GetSearchBoxView()->ButtonPressed(
-      GetSearchBoxView()->close_button(),
-      ui::MouseEvent(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
-                     base::TimeTicks(), ui::EF_LEFT_MOUSE_BUTTON,
-                     ui::EF_LEFT_MOUSE_BUTTON));
+  views::test::ButtonTestApi(GetSearchBoxView()->close_button())
+      .NotifyClick(ui::MouseEvent(
+          ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), base::TimeTicks(),
+          ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON));
   EXPECT_TRUE(GetSearchBoxView()->search_box()->GetText().empty());
   EXPECT_TRUE(GetSearchBoxView()->search_box()->HasFocus());
 

ash/shortcut_viewer/views/ksv_search_box_view.cc

@@ -69,14 +69,6 @@ void KSVSearchBoxView::OnKeyEvent(ui::KeyEvent* event) {
     SetSearchBoxActive(false, event->type());
 }
 
-void KSVSearchBoxView::ButtonPressed(views::Button* sender,
-                                     const ui::Event& event) {
-  // Focus on the search box text field after clicking close button.
-  if (close_button() && sender == close_button())
-    search_box()->RequestFocus();
-  SearchBoxViewBase::ButtonPressed(sender, event);
-}
-
 void KSVSearchBoxView::SetAccessibleValue(const base::string16& value) {
   accessible_value_ = value;
   NotifyAccessibilityEvent(ax::mojom::Event::kValueChanged, true);
@@ -108,6 +100,13 @@ void KSVSearchBoxView::UpdateSearchBoxBorder() {
 
 void KSVSearchBoxView::SetupCloseButton() {
   views::ImageButton* close = close_button();
+  close->SetCallback(base::BindRepeating(
+      [](ash::SearchBoxViewBase* view) {
+        // Focus on the search box text field after clicking close button.
+        view->search_box()->RequestFocus();
+        view->ClearSearch();
+      },
+      this));
   close->SetHasInkDropActionOnClick(true);
   close->SetImage(
       views::ImageButton::STATE_NORMAL,

ash/shortcut_viewer/views/ksv_search_box_view.h

@@ -26,9 +26,6 @@ class KSVSearchBoxView : public ash::SearchBoxViewBase {
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   void OnKeyEvent(ui::KeyEvent* event) override;
 
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   void SetAccessibleValue(const base::string16& value);
 
   // SearchBoxViewBase: