Use button pressed callback instead of listener.

Bug: 772945
Change-Id: I23bf2495c96f87471b6975595f03a787b375ac30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466205
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816589}

ash/app_list/views/page_switcher.cc

@@ -11,6 +11,7 @@
 #include "ash/app_list/app_list_metrics.h"
 #include "ash/public/cpp/app_list/app_list_color_provider.h"
 #include "ash/public/cpp/pagination/pagination_model.h"
+#include "base/bind.h"
 #include "base/i18n/number_formatting.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
@@ -49,15 +50,15 @@ constexpr int kHorizontalButtonPadding = 0;
 
 class PageSwitcherButton : public views::Button {
  public:
-  PageSwitcherButton(views::ButtonListener* listener,
-                     bool is_root_app_grid_page_switcher)
-      : views::Button(listener),
-        is_root_app_grid_page_switcher_(is_root_app_grid_page_switcher) {
+  explicit PageSwitcherButton(bool is_root_app_grid_page_switcher)
+      : is_root_app_grid_page_switcher_(is_root_app_grid_page_switcher) {
     SetInkDropMode(InkDropMode::ON);
     views::InstallFixedSizeCircleHighlightPathGenerator(
         this, is_root_app_grid_page_switcher ? kInkDropRadiusForRootGrid
                                              : kInkDropRadiusForFolderGrid);
   }
+  PageSwitcherButton(const PageSwitcherButton&) = delete;
+  PageSwitcherButton& operator=(const PageSwitcherButton&) = delete;
 
   ~PageSwitcherButton() override {}
 
@@ -167,8 +168,6 @@ class PageSwitcherButton : public views::Button {
 
   // True if the page switcher root is the app grid.
   const bool is_root_app_grid_page_switcher_;
-
-  DISALLOW_COPY_AND_ASSIGN(PageSwitcherButton);
 };
 
 // Gets PageSwitcherButton at |index| in |buttons|.
@@ -234,8 +233,8 @@ const char* PageSwitcher::GetClassName() const {
   return "PageSwitcher";
 }
 
-void PageSwitcher::ButtonPressed(views::Button* sender,
-                                 const ui::Event& event) {
+void PageSwitcher::OnButtonPressed(views::Button* sender,
+                                   const ui::Event& event) {
   if (!model_ || ignore_button_press_)
     return;
 
@@ -260,13 +259,15 @@ void PageSwitcher::TotalPagesChanged(int previous_page_count,
 
   buttons_->RemoveAllChildViews(true);
   for (int i = 0; i < model_->total_pages(); ++i) {
-    PageSwitcherButton* button =
-        new PageSwitcherButton(this, is_root_app_grid_page_switcher_);
+    PageSwitcherButton* button = buttons_->AddChildView(
+        std::make_unique<PageSwitcherButton>(is_root_app_grid_page_switcher_));
+    button->set_callback(base::BindRepeating(&PageSwitcher::OnButtonPressed,
+                                             base::Unretained(this),
+                                             base::Unretained(button)));
     button->SetAccessibleName(l10n_util::GetStringFUTF16(
         IDS_APP_LIST_PAGE_SWITCHER, base::FormatNumber(i + 1),
         base::FormatNumber(model_->total_pages())));
     button->SetSelected(i == model_->selected_page() ? true : false);
-    buttons_->AddChildView(button);
   }
   buttons_->SetVisible(model_->total_pages() > 1);
   Layout();

ash/app_list/views/page_switcher.h

@@ -7,7 +7,11 @@
 
 #include "ash/public/cpp/pagination/pagination_model_observer.h"
 #include "base/macros.h"
-#include "ui/views/controls/button/button.h"
+#include "ui/views/view.h"
+
+namespace views {
+class Button;
+}
 
 namespace ash {
 class PaginationModel;
@@ -16,7 +20,6 @@ class PaginationModel;
 // strip. Each page in the PageinationModel has a button in the strip and
 // when the button is clicked, the corresponding page becomes selected.
 class PageSwitcher : public views::View,
-                     public views::ButtonListener,
                      public PaginationModelObserver {
  public:
   static constexpr int kMaxButtonRadiusForRootGrid = 16;
@@ -25,6 +28,8 @@ class PageSwitcher : public views::View,
   PageSwitcher(PaginationModel* model,
                bool is_root_app_grid_page_switcher,
                bool is_tablet_mode);
+  PageSwitcher(const PageSwitcher&) = delete;
+  PageSwitcher& operator=(const PageSwitcher&) = delete;
   ~PageSwitcher() override;
 
   // Overridden from views::View:
@@ -36,8 +41,8 @@ class PageSwitcher : public views::View,
   void set_is_tablet_mode(bool started) { is_tablet_mode_ = started; }
 
  private:
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
+  // Button pressed callback.
+  void OnButtonPressed(views::Button* sender, const ui::Event& event);
 
   // Overridden from PaginationModelObserver:
   void TotalPagesChanged(int previous_page_count, int new_page_count) override;
@@ -54,8 +59,6 @@ class PageSwitcher : public views::View,
 
   // Whether tablet mode is enabled.
   bool is_tablet_mode_;
-
-  DISALLOW_COPY_AND_ASSIGN(PageSwitcher);
 };
 
 }  // namespace ash

ash/app_list/views/privacy_info_view.cc

@@ -210,10 +210,8 @@ void PrivacyInfoView::NotifyA11yResultSelected() {
   }
 }
 
-void PrivacyInfoView::ButtonPressed(views::Button* sender,
-                                    const ui::Event& event) {
-  if (sender == close_button_)
-    CloseButtonPressed();
+void PrivacyInfoView::OnButtonPressed() {
+  CloseButtonPressed();
 }
 
 void PrivacyInfoView::InitLayout() {
@@ -284,7 +282,9 @@ void PrivacyInfoView::InitText() {
 }
 
 void PrivacyInfoView::InitCloseButton() {
-  auto close_button = std::make_unique<views::ImageButton>(this);
+  auto close_button = std::make_unique<views::ImageButton>();
+  close_button->set_callback(base::BindRepeating(
+      &PrivacyInfoView::OnButtonPressed, base::Unretained(this)));
   close_button->SetImage(views::ImageButton::STATE_NORMAL,
                          gfx::CreateVectorIcon(views::kCloseIcon, kIconSizeDip,
                                                gfx::kGoogleGrey700));

ash/app_list/views/privacy_info_view.h

@@ -35,9 +35,6 @@ class PrivacyInfoView : public SearchResultBaseView {
   void OnGestureEvent(ui::GestureEvent* event) override;
   void OnKeyEvent(ui::KeyEvent* event) override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // SearchResultBaseView:
   void SelectInitialResultAction(bool reverse_tab_order) override;
   bool SelectNextResultAction(bool reverse_tab_order) override;
@@ -52,6 +49,9 @@ class PrivacyInfoView : public SearchResultBaseView {
  private:
   enum class Action { kNone, kTextLink, kCloseButton };
 
+  // Button pressed callback.
+  void OnButtonPressed();
+
   void InitLayout();
   void InitInfoIcon();
   void InitText();

ash/app_list/views/result_selection_controller_unittest.cc

@@ -36,10 +36,6 @@ class TestResultView : public SearchResultBaseView {
     return nullptr;
   }
 
-  void ButtonPressed(Button* sender, const ui::Event& event) override {
-    // Do nothing for test.
-  }
-
  private:
   DISALLOW_COPY_AND_ASSIGN(TestResultView);
 };

ash/app_list/views/search_result_base_view.cc

@@ -12,7 +12,7 @@
 
 namespace ash {
 
-SearchResultBaseView::SearchResultBaseView() : Button(this) {
+SearchResultBaseView::SearchResultBaseView() {
   SetInstallFocusRingOnFocus(false);
 }
 

ash/app_list/views/search_result_base_view.h

@@ -17,7 +17,6 @@ class SearchResultActionsView;
 
 // Base class for views that observe and display a search result
 class APP_LIST_EXPORT SearchResultBaseView : public views::Button,
-                                             public views::ButtonListener,
                                              public SearchResultObserver {
  public:
   SearchResultBaseView();

ash/app_list/views/search_result_suggestion_chip_view.cc

@@ -54,10 +54,11 @@ void LogAppLaunch(int index_in_container) {
 
 SearchResultSuggestionChipView::SearchResultSuggestionChipView(
     AppListViewDelegate* view_delegate)
-    : view_delegate_(view_delegate),
-      icon_view_(new views::ImageView()),
-      text_view_(new views::Label()) {
+    : view_delegate_(view_delegate) {
   SetFocusBehavior(FocusBehavior::ALWAYS);
+  set_callback(
+      base::BindRepeating(&SearchResultSuggestionChipView::OnButtonPressed,
+                          base::Unretained(this)));
 
   SetInstallFocusRingOnFocus(true);
   focus_ring()->SetColor(kFocusRingColor);
@@ -96,19 +97,6 @@ void SearchResultSuggestionChipView::OnMetadataChanged() {
   UpdateSuggestionChipView();
 }
 
-void SearchResultSuggestionChipView::ButtonPressed(views::Button* sender,
-                                                   const ui::Event& event) {
-  DCHECK(result());
-  LogAppLaunch(index_in_container());
-  RecordSearchResultOpenSource(result(), view_delegate_->GetModel(),
-                               view_delegate_->GetSearchModel());
-  view_delegate_->OpenSearchResult(
-      result()->id(), event.flags(),
-      AppListLaunchedFrom::kLaunchedFromSuggestionChip,
-      AppListLaunchType::kAppSearchResult, index_in_container(),
-      false /* launch_as_default */);
-}
-
 const char* SearchResultSuggestionChipView::GetClassName() const {
   return "SearchResultSuggestionChipView";
 }
@@ -232,20 +220,32 @@ void SearchResultSuggestionChipView::InitLayout() {
   // Icon.
   const int icon_size =
       AppListConfig::instance().suggestion_chip_icon_dimension();
+  icon_view_ = AddChildView(std::make_unique<views::ImageView>());
   icon_view_->SetImageSize(gfx::Size(icon_size, icon_size));
   icon_view_->SetPreferredSize(gfx::Size(icon_size, icon_size));
 
   icon_view_->SetVisible(false);
-  AddChildView(icon_view_);
 
   // Text.
+  text_view_ = AddChildView(std::make_unique<views::Label>());
   text_view_->SetAutoColorReadabilityEnabled(false);
   text_view_->SetSubpixelRenderingEnabled(false);
   text_view_->SetFontList(AppListConfig::instance().app_title_font());
   SetText(base::string16());
   text_view_->SetEnabledColor(
       AppListColorProvider::Get()->GetSuggestionChipTextColor());
-  AddChildView(text_view_);
+}
+
+void SearchResultSuggestionChipView::OnButtonPressed(const ui::Event& event) {
+  DCHECK(result());
+  LogAppLaunch(index_in_container());
+  RecordSearchResultOpenSource(result(), view_delegate_->GetModel(),
+                               view_delegate_->GetSearchModel());
+  view_delegate_->OpenSearchResult(
+      result()->id(), event.flags(),
+      AppListLaunchedFrom::kLaunchedFromSuggestionChip,
+      AppListLaunchType::kAppSearchResult, index_in_container(),
+      false /* launch_as_default */);
 }
 
 void SearchResultSuggestionChipView::SetRoundedCornersForLayer(

ash/app_list/views/search_result_suggestion_chip_view.h

@@ -28,6 +28,10 @@ class APP_LIST_EXPORT SearchResultSuggestionChipView
     : public SearchResultBaseView {
  public:
   explicit SearchResultSuggestionChipView(AppListViewDelegate* view_delegate);
+  SearchResultSuggestionChipView(const SearchResultSuggestionChipView&) =
+      delete;
+  SearchResultSuggestionChipView& operator=(
+      const SearchResultSuggestionChipView&) = delete;
   ~SearchResultSuggestionChipView() override;
 
   // Enables background blur for folder icon if |enabled| is true.
@@ -38,9 +42,6 @@ class APP_LIST_EXPORT SearchResultSuggestionChipView
   // SearchResultObserver:
   void OnMetadataChanged() override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // views::View:
   const char* GetClassName() const override;
   void ChildVisibilityChanged(views::View* child) override;
@@ -67,19 +68,19 @@ class APP_LIST_EXPORT SearchResultSuggestionChipView
 
   void InitLayout();
 
+  void OnButtonPressed(const ui::Event& event);
+
   // Sets rounded corners for the layer with |corner_radius| to clip the chip.
   void SetRoundedCornersForLayer(int corner_radius);
 
   AppListViewDelegate* const view_delegate_;  // Owned by AppListView.
 
-  views::ImageView* icon_view_;  // Owned by view hierarchy.
-  views::Label* text_view_;      // Owned by view hierarchy.
+  views::ImageView* icon_view_ = nullptr;  // Owned by view hierarchy.
+  views::Label* text_view_ = nullptr;      // Owned by view hierarchy.
 
   views::BoxLayout* layout_manager_;  // Owned by view hierarchy.
 
   base::WeakPtrFactory<SearchResultSuggestionChipView> weak_ptr_factory_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(SearchResultSuggestionChipView);
 };
 
 }  // namespace ash

ash/app_list/views/search_result_tile_item_list_view.h

@@ -28,6 +28,9 @@ class APP_LIST_EXPORT SearchResultTileItemListView
  public:
   SearchResultTileItemListView(views::Textfield* search_box,
                                AppListViewDelegate* view_delegate);
+  SearchResultTileItemListView(const SearchResultTileItemListView&) = delete;
+  SearchResultTileItemListView& operator=(const SearchResultTileItemListView&) =
+      delete;
   ~SearchResultTileItemListView() override;
 
   // Overridden from SearchResultContainerView:
@@ -77,8 +80,6 @@ class APP_LIST_EXPORT SearchResultTileItemListView
   const bool is_app_reinstall_recommendation_enabled_;
 
   const size_t max_search_result_tiles_;
-
-  DISALLOW_COPY_AND_ASSIGN(SearchResultTileItemListView);
 };
 
 }  // namespace ash

ash/app_list/views/search_result_tile_item_view.cc

@@ -74,6 +74,8 @@ SearchResultTileItemView::SearchResultTileItemView(
       is_app_reinstall_recommendation_enabled_(
           app_list_features::IsAppReinstallZeroStateEnabled()),
       show_in_apps_page_(show_in_apps_page) {
+  set_callback(base::BindRepeating(&SearchResultTileItemView::OnButtonPressed,
+                                   base::Unretained(this)));
   SetFocusBehavior(FocusBehavior::ALWAYS);
 
   // When |result_| is null, the tile is invisible. Calling SetSearchResult with
@@ -83,50 +85,44 @@ SearchResultTileItemView::SearchResultTileItemView(
   GetViewAccessibility().OverrideIsLeaf(true);
 
   // Prevent the icon view from interfering with our mouse events.
-  icon_ = new views::ImageView;
+  icon_ = AddChildView(std::make_unique<views::ImageView>());
   icon_->SetCanProcessEventsWithinSubtree(false);
   icon_->SetVerticalAlignment(views::ImageView::Alignment::kLeading);
-  AddChildView(icon_);
 
-  badge_ = new views::ImageView;
+  badge_ = AddChildView(std::make_unique<views::ImageView>());
   badge_->SetCanProcessEventsWithinSubtree(false);
   badge_->SetVerticalAlignment(views::ImageView::Alignment::kLeading);
   badge_->SetVisible(false);
-  AddChildView(badge_);
 
-  title_ = new views::Label;
+  title_ = AddChildView(std::make_unique<views::Label>());
   title_->SetAutoColorReadabilityEnabled(false);
   title_->SetEnabledColor(AppListColorProvider::Get()->GetSearchBoxTextColor());
   title_->SetLineHeight(kTileTextLineHeight);
   title_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
   title_->SetHandlesTooltips(false);
   title_->SetAllowCharacterBreak(true);
-  AddChildView(title_);
 
-  rating_ = new views::Label;
+  rating_ = AddChildView(std::make_unique<views::Label>());
   rating_->SetEnabledColor(
       AppListColorProvider::Get()->GetSearchBoxSecondaryTextColor());
   rating_->SetLineHeight(kTileTextLineHeight);
   rating_->SetHorizontalAlignment(gfx::ALIGN_RIGHT);
   rating_->SetVisible(false);
-  AddChildView(rating_);
 
-  rating_star_ = new views::ImageView;
+  rating_star_ = AddChildView(std::make_unique<views::ImageView>());
   rating_star_->SetCanProcessEventsWithinSubtree(false);
   rating_star_->SetVerticalAlignment(views::ImageView::Alignment::kLeading);
   rating_star_->SetImage(gfx::CreateVectorIcon(
       kBadgeRatingIcon, kSearchRatingStarSize,
       AppListColorProvider::Get()->GetSearchBoxSecondaryTextColor()));
   rating_star_->SetVisible(false);
-  AddChildView(rating_star_);
 
-  price_ = new views::Label;
+  price_ = AddChildView(std::make_unique<views::Label>());
   price_->SetEnabledColor(
       AppListColorProvider::Get()->GetSearchBoxSecondaryTextColor());
   price_->SetLineHeight(kTileTextLineHeight);
   price_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   price_->SetVisible(false);
-  AddChildView(price_);
 
   set_context_menu_controller(this);
 }
@@ -249,11 +245,6 @@ void SearchResultTileItemView::SetParentBackgroundColor(SkColor color) {
   UpdateBackgroundColor();
 }
 
-void SearchResultTileItemView::ButtonPressed(views::Button* sender,
-                                             const ui::Event& event) {
-  ActivateResult(event.flags(), true /* by_button_press */);
-}
-
 void SearchResultTileItemView::GetAccessibleNodeData(
     ui::AXNodeData* node_data) {
   views::Button::GetAccessibleNodeData(node_data);
@@ -389,6 +380,10 @@ void SearchResultTileItemView::OnMenuClosed() {
   }
 }
 
+void SearchResultTileItemView::OnButtonPressed(const ui::Event& event) {
+  ActivateResult(event.flags(), true /* by_button_press */);
+}
+
 void SearchResultTileItemView::ActivateResult(int event_flags,
                                               bool by_button_press) {
   const bool launch_as_default = is_default_result() && !by_button_press;

ash/app_list/views/search_result_tile_item_view.h

@@ -51,9 +51,6 @@ class APP_LIST_EXPORT SearchResultTileItemView
     return group_index_in_container_view_;
   }
 
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // Overridden from views::Button:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   bool OnKeyPressed(const ui::KeyEvent& event) override;
@@ -83,6 +80,8 @@ class APP_LIST_EXPORT SearchResultTileItemView
   // The callback used when a menu closes.
   void OnMenuClosed();
 
+  void OnButtonPressed(const ui::Event& event);
+
   void SetIcon(const gfx::ImageSkia& icon);
   void SetBadgeIcon(const gfx::ImageSkia& badge_icon);
   void SetTitle(const base::string16& title);

ash/app_list/views/search_result_view.cc

@@ -62,6 +62,8 @@ SearchResultView::SearchResultView(SearchResultListView* list_view,
                                    AppListViewDelegate* view_delegate)
     : list_view_(list_view), view_delegate_(view_delegate) {
   SetFocusBehavior(FocusBehavior::ALWAYS);
+  set_callback(base::BindRepeating(&SearchResultView::OnButtonPressed,
+                                   base::Unretained(this)));
 
   icon_ = AddChildView(std::make_unique<views::ImageView>());
   display_icon_ = AddChildView(std::make_unique<views::ImageView>());
@@ -350,13 +352,6 @@ void SearchResultView::OnGestureEvent(ui::GestureEvent* event) {
     Button::OnGestureEvent(event);
 }
 
-void SearchResultView::ButtonPressed(views::Button* sender,
-                                     const ui::Event& event) {
-  DCHECK(sender == this);
-  list_view_->SearchResultActivated(this, event.flags(),
-                                    true /* by_button_press */);
-}
-
 void SearchResultView::OnMetadataChanged() {
   // Updates |icon_|.
   // Note: this might leave the view with an old icon. But it is needed to avoid
@@ -385,6 +380,11 @@ void SearchResultView::OnMetadataChanged() {
                                       : SearchResult::Actions());
 }
 
+void SearchResultView::OnButtonPressed(const ui::Event& event) {
+  list_view_->SearchResultActivated(this, event.flags(),
+                                    true /* by_button_press */);
+}
+
 void SearchResultView::SetIconImage(const gfx::ImageSkia& source,
                                     views::ImageView* const icon,
                                     const int icon_dimension) {

ash/app_list/views/search_result_view.h

@@ -83,9 +83,6 @@ class APP_LIST_EXPORT SearchResultView
   // ui::EventHandler overrides:
   void OnGestureEvent(ui::GestureEvent* event) override;
 
-  // views::ButtonListener overrides:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // views::ContextMenuController overrides:
   void ShowContextMenuForViewImpl(views::View* source,
                                   const gfx::Point& point,
@@ -100,6 +97,8 @@ class APP_LIST_EXPORT SearchResultView
   // SearchResultObserver overrides:
   void OnMetadataChanged() override;
 
+  void OnButtonPressed(const ui::Event& event);
+
   void SetIconImage(const gfx::ImageSkia& source,
                     views::ImageView* const icon,
                     const int icon_dimension);