Change ButtonPressed overrides to callbacks: ash/system/message_center/

Bug: 772945
Change-Id: I438b94516567877c93f1357db189826e72a71470
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515272
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823802}

ash/system/message_center/notification_swipe_control_view.cc

@@ -115,7 +115,9 @@ void NotificationSwipeControlView::UpdateCornerRadius(int top_radius,
 
 void NotificationSwipeControlView::ShowSettingsButton(bool show) {
   if (show && !settings_button_) {
-    settings_button_ = new views::ImageButton(this);
+    settings_button_ = new views::ImageButton(
+        base::BindRepeating(&NotificationSwipeControlView::ButtonPressed,
+                            base::Unretained(this), ButtonId::kSettings));
     settings_button_->SetImage(
         views::Button::STATE_NORMAL,
         gfx::CreateVectorIcon(
@@ -149,7 +151,9 @@ void NotificationSwipeControlView::ShowSettingsButton(bool show) {
 
 void NotificationSwipeControlView::ShowSnoozeButton(bool show) {
   if (show && !snooze_button_) {
-    snooze_button_ = new views::ImageButton(this);
+    snooze_button_ = new views::ImageButton(
+        base::BindRepeating(&NotificationSwipeControlView::ButtonPressed,
+                            base::Unretained(this), ButtonId::kSnooze));
     snooze_button_->SetImage(
         views::Button::STATE_NORMAL,
         gfx::CreateVectorIcon(
@@ -184,18 +188,17 @@ const char* NotificationSwipeControlView::GetClassName() const {
   return kViewClassName;
 }
 
-void NotificationSwipeControlView::ButtonPressed(views::Button* sender,
+void NotificationSwipeControlView::ButtonPressed(ButtonId button,
                                                  const ui::Event& event) {
-  DCHECK(sender);
-  std::string notification_id = message_view_->notification_id();
   auto weak_this = weak_factory_.GetWeakPtr();
 
-  if (sender == settings_button_) {
+  const std::string notification_id = message_view_->notification_id();
+  if (button == ButtonId::kSettings) {
     message_view_->OnSettingsButtonPressed(event);
     metrics_utils::LogSettingsShown(notification_id,
                                     /*is_slide_controls=*/true,
                                     /*is_popup=*/false);
-  } else if (sender == snooze_button_) {
+  } else {
     message_view_->OnSnoozeButtonPressed(event);
     metrics_utils::LogSnoozed(notification_id,
                               /*is_slide_controls=*/true,

ash/system/message_center/notification_swipe_control_view.h

@@ -11,7 +11,6 @@
 #include "base/observer_list_types.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/animation/animation_delegate.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/view.h"
 
@@ -22,8 +21,7 @@ class MessageView;
 namespace ash {
 
 // View containing 2 buttons that appears behind notification by swiping.
-class ASH_EXPORT NotificationSwipeControlView : public views::View,
-                                                public views::ButtonListener {
+class ASH_EXPORT NotificationSwipeControlView : public views::View {
  public:
   // Physical positions to show buttons in the swipe control. This is invariant
   // across RTL/LTR languages because buttons should be shown on one side which
@@ -43,9 +41,6 @@ class ASH_EXPORT NotificationSwipeControlView : public views::View,
   // views::View
   const char* GetClassName() const override;
 
-  // views::ButtonListener
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // Update the visibility of control buttons.
   void UpdateButtonsVisibility();
 
@@ -58,6 +53,11 @@ class ASH_EXPORT NotificationSwipeControlView : public views::View,
   FRIEND_TEST_ALL_PREFIXES(NotificationSwipeControlViewTest,
                            DeleteOnSnoozeButtonPressed);
 
+  enum class ButtonId {
+    kSettings,
+    kSnooze,
+  };
+
   // Change the visibility of the settings button.
   void ShowButtons(ButtonPosition button_position,
                    bool has_settings,
@@ -70,6 +70,8 @@ class ASH_EXPORT NotificationSwipeControlView : public views::View,
   // Change the visibility of the snooze button. True to show, false to hide.
   void ShowSnoozeButton(bool show);
 
+  void ButtonPressed(ButtonId button, const ui::Event& event);
+
   message_center::MessageView* const message_view_;
 
   // Owned by views hierarchy.

ash/system/message_center/notification_swipe_control_view_unittest.cc

@@ -18,6 +18,7 @@
 #include "ui/message_center/public/cpp/notification.h"
 #include "ui/message_center/views/message_view.h"
 #include "ui/message_center/views/notification_control_buttons_view.h"
+#include "ui/views/test/button_test_api.h"
 #include "url/gurl.h"
 
 namespace {
@@ -107,16 +108,16 @@ TEST_F(NotificationSwipeControlViewTest, DeleteOnSettingsButtonPressed) {
   swipe_control_view->ShowButtons(
       NotificationSwipeControlView::ButtonPosition::LEFT,
       /*has_settings_button=*/true, /*has_snooze_button=*/true);
-  swipe_control_view->ButtonPressed(swipe_control_view->settings_button_,
-                                    press);
+  views::test::ButtonTestApi(swipe_control_view->settings_button_)
+      .NotifyClick(press);
   EXPECT_TRUE(swipe_control_view);
 
   // Second click deletes |swipe_control_view| in the handler.
   swipe_control_view->ShowButtons(
       NotificationSwipeControlView::ButtonPosition::LEFT,
       /*has_settings_button=*/true, /*has_snooze_button=*/true);
-  swipe_control_view->ButtonPressed(swipe_control_view->settings_button_,
-                                    press);
+  views::test::ButtonTestApi(swipe_control_view->settings_button_)
+      .NotifyClick(press);
   EXPECT_FALSE(swipe_control_view);
 }
 
@@ -137,14 +138,16 @@ TEST_F(NotificationSwipeControlViewTest, DeleteOnSnoozeButtonPressed) {
   swipe_control_view->ShowButtons(
       NotificationSwipeControlView::ButtonPosition::LEFT,
       /*has_settings_button=*/true, /*has_snooze_button=*/true);
-  swipe_control_view->ButtonPressed(swipe_control_view->snooze_button_, press);
+  views::test::ButtonTestApi(swipe_control_view->snooze_button_)
+      .NotifyClick(press);
   EXPECT_TRUE(swipe_control_view);
 
   // Second click deletes |swipe_control_view| in the handler.
   swipe_control_view->ShowButtons(
       NotificationSwipeControlView::ButtonPosition::LEFT,
       /*has_settings_button=*/true, /*has_snooze_button=*/true);
-  swipe_control_view->ButtonPressed(swipe_control_view->snooze_button_, press);
+  views::test::ButtonTestApi(swipe_control_view->snooze_button_)
+      .NotifyClick(press);
   EXPECT_FALSE(swipe_control_view);
 }
 

ash/system/message_center/notifier_settings_view.cc

@@ -320,8 +320,8 @@ NotifierSettingsView::NotifierButton::NotifierButton(
       base::BindRepeating(
           [](NotifierButton* button, const ui::Event& event) {
             // The checkbox state has already changed at this point, but we'll
-            // update the state on NotifierSettingsView::ButtonPressed() too, so
-            // here change back to the previous state.
+            // update the state on NotifierSettingsView::NotifierButtonPressed()
+            // too, so here change back to the previous state.
             button->checkbox_->SetChecked(!button->checkbox_->GetChecked());
             button->NotifyClick(event);
           },

ash/system/message_center/stacked_notification_bar.cc

@@ -35,10 +35,10 @@ namespace {
 // or "See all notifications" button.
 class StackingBarLabelButton : public views::LabelButton {
  public:
-  StackingBarLabelButton(views::ButtonListener* listener,
+  StackingBarLabelButton(PressedCallback callback,
                          const base::string16& text,
                          UnifiedMessageCenterView* message_center_view)
-      : views::LabelButton(listener, text),
+      : views::LabelButton(std::move(callback), text),
         message_center_view_(message_center_view) {
     SetEnabledTextColors(message_center_style::kUnifiedMenuButtonColorActive);
     SetHorizontalAlignment(gfx::ALIGN_CENTER);
@@ -241,12 +241,14 @@ StackedNotificationBar::StackedNotificationBar(
     : message_center_view_(message_center_view),
       count_label_(new views::Label),
       clear_all_button_(new StackingBarLabelButton(
-          this,
+          base::BindRepeating(&UnifiedMessageCenterView::ClearAllNotifications,
+                              base::Unretained(message_center_view_)),
           l10n_util::GetStringUTF16(
               IDS_ASH_MESSAGE_CENTER_CLEAR_ALL_BUTTON_LABEL),
           message_center_view)),
       expand_all_button_(new StackingBarLabelButton(
-          this,
+          base::BindRepeating(&UnifiedMessageCenterView::ExpandMessageCenter,
+                              base::Unretained(message_center_view_)),
           l10n_util::GetStringUTF16(
               IDS_ASH_MESSAGE_CENTER_EXPAND_ALL_NOTIFICATIONS_BUTTON_LABEL),
           message_center_view)) {
@@ -507,15 +509,6 @@ void StackedNotificationBar::UpdateVisibility() {
   }
 }
 
-void StackedNotificationBar::ButtonPressed(views::Button* sender,
-                                           const ui::Event& event) {
-  if (sender == clear_all_button_) {
-    message_center_view_->ClearAllNotifications();
-  } else if (sender == expand_all_button_) {
-    message_center_view_->ExpandMessageCenter();
-  }
-}
-
 void StackedNotificationBar::OnNotificationAdded(const std::string& id) {
   // Reset the stacked icons bar if a notification is added since we don't
   // know the position where it may have been added.

ash/system/message_center/stacked_notification_bar.h

@@ -13,7 +13,6 @@
 #include "ui/gfx/animation/animation_delegate.h"
 #include "ui/message_center/message_center_observer.h"
 #include "ui/views/background.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/focus/focus_manager.h"
@@ -29,7 +28,6 @@ namespace ash {
 // notifications. There are currently two UI implementations toggled by the
 // NotificationStackedBarRedesign feature flag.
 class StackedNotificationBar : public views::View,
-                               public views::ButtonListener,
                                public message_center::MessageCenterObserver {
  public:
   explicit StackedNotificationBar(
@@ -59,9 +57,6 @@ class StackedNotificationBar : public views::View,
   void OnPaint(gfx::Canvas* canvas) override;
   const char* GetClassName() const override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // message_center::MessageCenterObserver:
   void OnNotificationAdded(const std::string& id) override;
   void OnNotificationRemoved(const std::string& id, bool by_user) override;