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

Bug: 772945
Change-Id: I4a3a8090b3b03a68c9d214f8ef0b9c26eebe2dfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2508333
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822936}

ash/system/phonehub/bluetooth_disabled_view.cc

@@ -47,7 +47,8 @@ BluetoothDisabledView::BluetoothDisabledView() {
 
   // Add "Learn more" and "Ok, got it" buttons.
   auto learn_more = std::make_unique<InterstitialViewButton>(
-      this,
+      base::BindRepeating(&BluetoothDisabledView::LearnMoreButtonPressed,
+                          base::Unretained(this)),
       l10n_util::GetStringUTF16(
           IDS_ASH_PHONE_HUB_BLUETOOTH_DISABLED_DIALOG_LEARN_MORE_BUTTON),
       /*paint_background=*/false);
@@ -58,7 +59,8 @@ BluetoothDisabledView::BluetoothDisabledView() {
   content_view_->AddButton(std::move(learn_more));
 
   auto confirm = std::make_unique<InterstitialViewButton>(
-      this,
+      base::BindRepeating(&BluetoothDisabledView::ConfirmButtonPressed,
+                          base::Unretained(this)),
       l10n_util::GetStringUTF16(
           IDS_ASH_PHONE_HUB_BLUETOOTH_DISABLED_DIALOG_OK_BUTTON),
       /*paint_background=*/true);
@@ -75,24 +77,20 @@ phone_hub_metrics::Screen BluetoothDisabledView::GetScreenForMetrics() const {
   return Screen::kBluetoothOrWifiDisabled;
 }
 
-void BluetoothDisabledView::ButtonPressed(views::Button* sender,
-                                          const ui::Event& event) {
-  switch (sender->GetID()) {
-    case kBluetoothDisabledLearnMoreButton:
-      LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                 InterstitialScreenEvent::kLearnMore);
-      NewWindowDelegate::GetInstance()->NewTabWithUrl(
-          GURL(kLearnMoreUrl), /*from_user_interaction=*/true);
-      return;
-    case kBluetoothDisabledConfirmButton:
-      LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                 InterstitialScreenEvent::kConfirm);
-      Shell::GetPrimaryRootWindowController()
-          ->GetStatusAreaWidget()
-          ->phone_hub_tray()
-          ->CloseBubble();
-      return;
-  }
+void BluetoothDisabledView::LearnMoreButtonPressed() {
+  LogInterstitialScreenEvent(GetScreenForMetrics(),
+                             InterstitialScreenEvent::kLearnMore);
+  NewWindowDelegate::GetInstance()->NewTabWithUrl(
+      GURL(kLearnMoreUrl), /*from_user_interaction=*/true);
+}
+
+void BluetoothDisabledView::ConfirmButtonPressed() {
+  LogInterstitialScreenEvent(GetScreenForMetrics(),
+                             InterstitialScreenEvent::kConfirm);
+  Shell::GetPrimaryRootWindowController()
+      ->GetStatusAreaWidget()
+      ->phone_hub_tray()
+      ->CloseBubble();
 }
 
 BEGIN_METADATA(BluetoothDisabledView, views::View)

ash/system/phonehub/bluetooth_disabled_view.h

@@ -7,7 +7,6 @@
 
 #include "ash/ash_export.h"
 #include "ash/system/phonehub/phone_hub_content_view.h"
-#include "ui/views/controls/button/button.h"
 
 namespace ash {
 
@@ -15,8 +14,7 @@ class PhoneHubInterstitialView;
 
 // An interstitial view representing an error state where the Phone Hub
 // feature is not available because Bluetooth is turned off on this device.
-class ASH_EXPORT BluetoothDisabledView : public PhoneHubContentView,
-                                         public views::ButtonListener {
+class ASH_EXPORT BluetoothDisabledView : public PhoneHubContentView {
  public:
   METADATA_HEADER(BluetoothDisabledView);
 
@@ -28,10 +26,10 @@ class ASH_EXPORT BluetoothDisabledView : public PhoneHubContentView,
   // PhoneHubInterstitialView:
   phone_hub_metrics::Screen GetScreenForMetrics() const override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
  private:
+  void LearnMoreButtonPressed();
+  void ConfirmButtonPressed();
+
   PhoneHubInterstitialView* content_view_ = nullptr;
 };
 

ash/system/phonehub/connection_error_view.cc

@@ -61,7 +61,13 @@ ConnectionErrorView::ConnectionErrorView(
 
   // Add "Learn more" and "Refresh" buttons only for disconnected state.
   auto learn_more = std::make_unique<InterstitialViewButton>(
-      this,
+      base::BindRepeating(
+          &ConnectionErrorView::ButtonPressed, base::Unretained(this),
+          InterstitialScreenEvent::kLearnMore,
+          base::BindRepeating(
+              &NewWindowDelegate::NewTabWithUrl,
+              base::Unretained(NewWindowDelegate::GetInstance()),
+              GURL(kLearnMoreUrl), /*from_user_interaction=*/true)),
       l10n_util::GetStringUTF16(
           IDS_ASH_PHONE_HUB_CONNECTION_ERROR_DIALOG_LEARN_MORE_BUTTON),
       /*paint_background=*/false);
@@ -72,7 +78,12 @@ ConnectionErrorView::ConnectionErrorView(
   content_view_->AddButton(std::move(learn_more));
 
   auto refresh = std::make_unique<InterstitialViewButton>(
-      this,
+      base::BindRepeating(
+          &ConnectionErrorView::ButtonPressed, base::Unretained(this),
+          InterstitialScreenEvent::kConfirm,
+          base::BindRepeating(
+              &chromeos::phonehub::ConnectionScheduler::ScheduleConnectionNow,
+              base::Unretained(connection_scheduler_))),
       l10n_util::GetStringUTF16(
           IDS_ASH_PHONE_HUB_CONNECTION_ERROR_DIALOG_REFRESH_BUTTON),
       /*paint_background=*/true);
@@ -91,22 +102,10 @@ phone_hub_metrics::Screen ConnectionErrorView::GetScreenForMetrics() const {
              : Screen::kConnectionError;
 }
 
-void ConnectionErrorView::ButtonPressed(views::Button* sender,
-                                        const ui::Event& event) {
-  switch (sender->GetID()) {
-    case PhoneHubViewID::kDisconnectedRefreshButton:
-      LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                 InterstitialScreenEvent::kConfirm);
-      // Retry the connection attempt.
-      connection_scheduler_->ScheduleConnectionNow();
-      return;
-    case PhoneHubViewID::kDisconnectedLearnMoreButton:
-      LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                 InterstitialScreenEvent::kLearnMore);
-      NewWindowDelegate::GetInstance()->NewTabWithUrl(
-          GURL(kLearnMoreUrl), /*from_user_interaction=*/true);
-      return;
-  }
+void ConnectionErrorView::ButtonPressed(InterstitialScreenEvent event,
+                                        base::RepeatingClosure callback) {
+  LogInterstitialScreenEvent(GetScreenForMetrics(), event);
+  std::move(callback).Run();
 }
 
 BEGIN_METADATA(ConnectionErrorView, views::View)

ash/system/phonehub/connection_error_view.h

@@ -7,7 +7,6 @@
 
 #include "ash/ash_export.h"
 #include "ash/system/phonehub/phone_hub_content_view.h"
-#include "ui/views/controls/button/button.h"
 
 namespace chromeos {
 namespace phonehub {
@@ -19,10 +18,13 @@ namespace ash {
 
 class PhoneHubInterstitialView;
 
+namespace phone_hub_metrics {
+enum class InterstitialScreenEvent;
+}
+
 // An interstitial view represeting that the Phone Hub feature is not available
 // due to connection issues.
-class ASH_EXPORT ConnectionErrorView : public PhoneHubContentView,
-                                       public views::ButtonListener {
+class ASH_EXPORT ConnectionErrorView : public PhoneHubContentView {
  public:
   METADATA_HEADER(ConnectionErrorView);
 
@@ -42,10 +44,10 @@ class ASH_EXPORT ConnectionErrorView : public PhoneHubContentView,
   // PhoneHubContentView:
   phone_hub_metrics::Screen GetScreenForMetrics() const override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
  private:
+  void ButtonPressed(phone_hub_metrics::InterstitialScreenEvent event,
+                     base::RepeatingClosure callback);
+
   chromeos::phonehub::ConnectionScheduler* connection_scheduler_ = nullptr;
 
   PhoneHubInterstitialView* content_view_ = nullptr;

ash/system/phonehub/continue_browsing_chip.cc

@@ -35,7 +35,10 @@ constexpr int kTitleMaxLines = 2;
 ContinueBrowsingChip::ContinueBrowsingChip(
     const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata,
     int index)
-    : views::Button(this), url_(metadata.url), index_(index) {
+    : views::Button(base::BindRepeating(&ContinueBrowsingChip::ButtonPressed,
+                                        base::Unretained(this))),
+      url_(metadata.url),
+      index_(index) {
   auto* color_provider = AshColorProvider::Get();
   SetFocusBehavior(FocusBehavior::ALWAYS);
   focus_ring()->SetColor(color_provider->GetControlsLayerColor(
@@ -100,8 +103,13 @@ void ContinueBrowsingChip::OnPaintBackground(gfx::Canvas* canvas) {
   views::View::OnPaintBackground(canvas);
 }
 
-void ContinueBrowsingChip::ButtonPressed(views::Button* sender,
-                                         const ui::Event& event) {
+ContinueBrowsingChip::~ContinueBrowsingChip() = default;
+
+const char* ContinueBrowsingChip::GetClassName() const {
+  return "ContinueBrowsingChip";
+}
+
+void ContinueBrowsingChip::ButtonPressed() {
   PA_LOG(INFO) << "Opening browser tab: " << url_;
   phone_hub_metrics::LogTabContinuationChipClicked(index_);
 
@@ -114,10 +122,4 @@ void ContinueBrowsingChip::ButtonPressed(views::Button* sender,
       ->CloseBubble();
 }
 
-ContinueBrowsingChip::~ContinueBrowsingChip() = default;
-
-const char* ContinueBrowsingChip::GetClassName() const {
-  return "ContinueBrowsingChip";
-}
-
 }  // namespace ash

ash/system/phonehub/continue_browsing_chip.h

@@ -14,8 +14,7 @@ namespace ash {
 
 // A chip containing a web page info (title, web URL, etc.) that users left off
 // from their phone.
-class ASH_EXPORT ContinueBrowsingChip : public views::Button,
-                                        public views::ButtonListener {
+class ASH_EXPORT ContinueBrowsingChip : public views::Button {
  public:
   ContinueBrowsingChip(
       const chromeos::phonehub::BrowserTabsModel::BrowserTabMetadata& metadata,
@@ -27,10 +26,11 @@ class ASH_EXPORT ContinueBrowsingChip : public views::Button,
 
   // views::ButtonListener:
   void OnPaintBackground(gfx::Canvas* canvas) override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   const char* GetClassName() const override;
 
  private:
+  void ButtonPressed();
+
   // The URL of the tab to open.
   GURL url_;
 

ash/system/phonehub/interstitial_view_button.cc

@@ -10,10 +10,10 @@
 
 namespace ash {
 
-InterstitialViewButton::InterstitialViewButton(views::ButtonListener* listener,
+InterstitialViewButton::InterstitialViewButton(Button::PressedCallback callback,
                                                const base::string16& text,
                                                bool paint_background)
-    : RoundedLabelButton(PressedCallback(listener, this), text),
+    : RoundedLabelButton(std::move(callback), text),
       paint_background_(paint_background) {}
 
 InterstitialViewButton::~InterstitialViewButton() = default;

ash/system/phonehub/interstitial_view_button.h

@@ -7,10 +7,7 @@
 
 #include "ash/ash_export.h"
 #include "ash/system/unified/rounded_label_button.h"
-
-namespace views {
-class ButtonListener;
-}  // namespace views
+#include "ui/views/controls/button/button.h"
 
 namespace ash {
 
@@ -21,7 +18,7 @@ class ASH_EXPORT InterstitialViewButton : public RoundedLabelButton {
  public:
   METADATA_HEADER(InterstitialViewButton);
 
-  InterstitialViewButton(views::ButtonListener* listener,
+  InterstitialViewButton(Button::PressedCallback callback,
                          const base::string16& text,
                          bool paint_background);
   InterstitialViewButton(const InterstitialViewButton&) = delete;

ash/system/phonehub/notification_opt_in_view.cc

@@ -44,10 +44,6 @@ constexpr int kTextLabelLineHeightDip = 20;
 // Typography.
 constexpr int kLabelTextFontSizeDip = 14;
 
-// Tag value used to uniquely identify the "Dismiss" and "Get started" buttons.
-constexpr int kDismissButtonTag = 1;
-constexpr int kSetUpButtonTag = 2;
-
 // URL of the multidevice settings page with the URL parameter that will
 // start up the opt-in-flow.
 constexpr char kMultideviceSettingsUrl[] =
@@ -68,24 +64,19 @@ NotificationOptInView::NotificationOptInView(
 
 NotificationOptInView::~NotificationOptInView() = default;
 
-void NotificationOptInView::ButtonPressed(views::Button* sender,
-                                          const ui::Event& event) {
-  switch (sender->tag()) {
-    case kDismissButtonTag:
-      // Dismiss this view if user chose to opt out and update the bubble size.
-      LogNotificationOptInEvent(InterstitialScreenEvent::kDismiss);
-      SetVisible(false);
-      bubble_view_->UpdateBubble();
-      notification_access_manager_->DismissSetupRequiredUi();
-      break;
-    case kSetUpButtonTag:
-      // Opens the notification set up dialog in settings to start the opt in
-      // flow.
-      LogNotificationOptInEvent(InterstitialScreenEvent::kConfirm);
-      NewWindowDelegate::GetInstance()->NewTabWithUrl(
-          GURL(kMultideviceSettingsUrl), /*from_user_interaction=*/true);
-      break;
-  }
+void NotificationOptInView::SetUpButtonPressed() {
+  // Opens the notification set up dialog in settings to start the opt in flow.
+  LogNotificationOptInEvent(InterstitialScreenEvent::kConfirm);
+  NewWindowDelegate::GetInstance()->NewTabWithUrl(
+      GURL(kMultideviceSettingsUrl), /*from_user_interaction=*/true);
+}
+
+void NotificationOptInView::DismissButtonPressed() {
+  // Dismiss this view if user chose to opt out and update the bubble size.
+  LogNotificationOptInEvent(InterstitialScreenEvent::kDismiss);
+  SetVisible(false);
+  bubble_view_->UpdateBubble();
+  notification_access_manager_->DismissSetupRequiredUi();
 }
 
 void NotificationOptInView::InitLayout() {
@@ -134,21 +125,21 @@ void NotificationOptInView::InitLayout() {
       views::CreateEmptyBorder(kButtonContainerBorderInsets));
   dismiss_button_ =
       button_container->AddChildView(std::make_unique<InterstitialViewButton>(
-          this,
+          base::BindRepeating(&NotificationOptInView::DismissButtonPressed,
+                              base::Unretained(this)),
           l10n_util::GetStringUTF16(
               IDS_ASH_PHONE_HUB_NOTIFICATION_OPT_IN_DISMISS_BUTTON),
           /*paint_background=*/false));
-  dismiss_button_->set_tag(kDismissButtonTag);
   dismiss_button_->SetEnabledTextColors(
       AshColorProvider::Get()->GetContentLayerColor(
           AshColorProvider::ContentLayerType::kTextColorPrimary));
   set_up_button_ =
       button_container->AddChildView(std::make_unique<InterstitialViewButton>(
-          this,
+          base::BindRepeating(&NotificationOptInView::SetUpButtonPressed,
+                              base::Unretained(this)),
           l10n_util::GetStringUTF16(
               IDS_ASH_PHONE_HUB_NOTIFICATION_OPT_IN_SET_UP_BUTTON),
           /*paint_background=*/true));
-  set_up_button_->set_tag(kSetUpButtonTag);
 }
 
 BEGIN_METADATA(NotificationOptInView, views::View)

ash/system/phonehub/notification_opt_in_view.h

@@ -27,8 +27,7 @@ class TrayBubbleView;
 
 // An additional entry point shown on the Phone Hub bubble for the user to grant
 // access or opt out for notifications from the phone.
-class ASH_EXPORT NotificationOptInView : public views::View,
-                                         public views::ButtonListener {
+class ASH_EXPORT NotificationOptInView : public views::View {
  public:
   METADATA_HEADER(NotificationOptInView);
 
@@ -39,15 +38,15 @@ class ASH_EXPORT NotificationOptInView : public views::View,
   NotificationOptInView& operator=(const NotificationOptInView&) = delete;
   ~NotificationOptInView() override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   views::View* set_up_button_for_testing() { return set_up_button_; }
   views::View* dismiss_button_for_testing() { return dismiss_button_; }
 
  private:
   void InitLayout();
 
+  void SetUpButtonPressed();
+  void DismissButtonPressed();
+
   // Main components of this view. Owned by view hierarchy.
   views::Label* text_label_ = nullptr;
   InterstitialViewButton* set_up_button_ = nullptr;

ash/system/phonehub/onboarding_view.cc

@@ -29,7 +29,6 @@
 #include "chromeos/components/phonehub/onboarding_ui_tracker.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/metadata/metadata_impl_macros.h"
 
@@ -43,8 +42,7 @@ using phone_hub_metrics::Screen;
 // Main onboarding screen with Phone Hub feature description and two buttons
 // (Get Started and Dismiss), where user can either choose to grant permission
 // to enable this feature or dismiss the screen.
-class OnboardingMainView : public PhoneHubInterstitialView,
-                           public views::ButtonListener {
+class OnboardingMainView : public PhoneHubInterstitialView {
  public:
   OnboardingMainView(
       chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker,
@@ -56,23 +54,6 @@ class OnboardingMainView : public PhoneHubInterstitialView,
     InitLayout();
   }
 
-  // views::ButtonListener:
-  // TODO(crbug.com/1141629): deprecated, replace with |PressedCallback|.
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override {
-    switch (sender->GetID()) {
-      case PhoneHubViewID::kOnboardingGetStartedButton:
-        LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                   InterstitialScreenEvent::kConfirm);
-        onboarding_ui_tracker_->HandleGetStarted();
-        return;
-      case PhoneHubViewID::kOnboardingDismissButton:
-        LogInterstitialScreenEvent(GetScreenForMetrics(),
-                                   InterstitialScreenEvent::kDismiss);
-        parent_view_->ShowDismissPrompt();
-        return;
-    }
-  }
-
   // PhoneHubInterstitialView:
   Screen GetScreenForMetrics() const override {
     // TODO(tengs): Distinguish between the two different onboarding flows.
@@ -93,7 +74,8 @@ class OnboardingMainView : public PhoneHubInterstitialView,
 
     // Add "Dismiss" and "Get started" buttons.
     auto dismiss = std::make_unique<InterstitialViewButton>(
-        this,
+        base::BindRepeating(&OnboardingMainView::DismissButtonPressed,
+                            base::Unretained(this)),
         l10n_util::GetStringUTF16(
             IDS_ASH_PHONE_HUB_ONBOARDING_DIALOG_DISMISS_BUTTON),
         /*paint_background=*/false);
@@ -103,7 +85,8 @@ class OnboardingMainView : public PhoneHubInterstitialView,
     AddButton(std::move(dismiss));
 
     auto get_started = std::make_unique<InterstitialViewButton>(
-        this,
+        base::BindRepeating(&OnboardingMainView::GetStartedButtonPressed,
+                            base::Unretained(this)),
         l10n_util::GetStringUTF16(
             IDS_ASH_PHONE_HUB_ONBOARDING_DIALOG_GET_STARTED_BUTTON),
         /*paint_background=*/true);
@@ -111,6 +94,18 @@ class OnboardingMainView : public PhoneHubInterstitialView,
     AddButton(std::move(get_started));
   }
 
+  void GetStartedButtonPressed() {
+    LogInterstitialScreenEvent(GetScreenForMetrics(),
+                               InterstitialScreenEvent::kConfirm);
+    onboarding_ui_tracker_->HandleGetStarted();
+  }
+
+  void DismissButtonPressed() {
+    LogInterstitialScreenEvent(GetScreenForMetrics(),
+                               InterstitialScreenEvent::kDismiss);
+    parent_view_->ShowDismissPrompt();
+  }
+
   chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker_ = nullptr;
   OnboardingView* parent_view_ = nullptr;
 };
@@ -119,8 +114,7 @@ class OnboardingMainView : public PhoneHubInterstitialView,
 // A follow-up prompt screen that pops up when the user has chosen to dismiss
 // the main onboarding screen. It should not be shown again after being
 // dismissed manually by either clicking the ack button or outside the bubble.
-class OnboardingDismissPromptView : public PhoneHubInterstitialView,
-                                    public views::ButtonListener {
+class OnboardingDismissPromptView : public PhoneHubInterstitialView {
  public:
   explicit OnboardingDismissPromptView(
       chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker)
@@ -144,7 +138,8 @@ class OnboardingDismissPromptView : public PhoneHubInterstitialView,
 
     // Adds "Ok, got it" button.
     auto ack_button = std::make_unique<InterstitialViewButton>(
-        this,
+        base::BindRepeating(&OnboardingDismissPromptView::ButtonPressed,
+                            base::Unretained(this)),
         l10n_util::GetStringUTF16(
             IDS_ASH_PHONE_HUB_ONBOARDING_DISMISS_DIALOG_OK_BUTTON),
         /*paint_background=*/true);
@@ -152,17 +147,7 @@ class OnboardingDismissPromptView : public PhoneHubInterstitialView,
     AddButton(std::move(ack_button));
   }
 
-  // PhoneHubInterstitialView:
-  void OnBubbleClose() override { onboarding_ui_tracker_->DismissSetupUi(); }
-
-  Screen GetScreenForMetrics() const override {
-    return Screen::kOnboardingDismissPrompt;
-  }
-
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override {
-    DCHECK_EQ(sender->GetID(), PhoneHubViewID::kOnboardingDismissAckButton);
-
+  void ButtonPressed() {
     LogInterstitialScreenEvent(GetScreenForMetrics(),
                                InterstitialScreenEvent::kConfirm);
 
@@ -172,6 +157,13 @@ class OnboardingDismissPromptView : public PhoneHubInterstitialView,
         ->CloseBubble();
   }
 
+  // PhoneHubInterstitialView:
+  void OnBubbleClose() override { onboarding_ui_tracker_->DismissSetupUi(); }
+
+  Screen GetScreenForMetrics() const override {
+    return Screen::kOnboardingDismissPrompt;
+  }
+
   chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker_ = nullptr;
 };
 

ash/system/phonehub/phone_status_view.cc

@@ -64,12 +64,13 @@ PhoneStatusView::PhoneStatusView(chromeos::phonehub::PhoneModel* phone_model,
                                  Delegate* delegate)
     : TriView(kTitleContainerSpacing),
       phone_model_(phone_model),
-      delegate_(delegate),
       phone_name_label_(new views::Label),
       signal_icon_(new views::ImageView),
       mobile_provider_label_(new views::Label),
       battery_icon_(new views::ImageView),
       battery_label_(new views::Label) {
+  DCHECK(delegate);
+
   SetID(PhoneHubViewID::kPhoneStatusView);
 
   SetBorder(views::CreateEmptyBorder(kBorderInsets));
@@ -110,13 +111,14 @@ PhoneStatusView::PhoneStatusView(chromeos::phonehub::PhoneModel* phone_model,
   separator->SetPreferredHeight(kSeparatorHeight);
   AddView(TriView::Container::END, separator);
 
-  settings_button_ = new TopShortcutButton(this, kSystemMenuSettingsIcon,
-                                           IDS_ASH_STATUS_TRAY_SETTINGS);
+  settings_button_ = new TopShortcutButton(
+      base::BindRepeating(&Delegate::OpenConnectedDevicesSettings,
+                          base::Unretained(delegate)),
+      kSystemMenuSettingsIcon, IDS_ASH_STATUS_TRAY_SETTINGS);
   AddView(TriView::Container::END, settings_button_);
 
-  DCHECK(delegate_);
-  separator->SetVisible(delegate_->CanOpenConnectedDeviceSettings());
-  settings_button_->SetVisible(delegate_->CanOpenConnectedDeviceSettings());
+  separator->SetVisible(delegate->CanOpenConnectedDeviceSettings());
+  settings_button_->SetVisible(delegate->CanOpenConnectedDeviceSettings());
 
   Update();
 }
@@ -125,13 +127,6 @@ PhoneStatusView::~PhoneStatusView() {
   phone_model_->RemoveObserver(this);
 }
 
-void PhoneStatusView::ButtonPressed(views::Button* sender,
-                                    const ui::Event& event) {
-  DCHECK_EQ(settings_button_, sender);
-  DCHECK(delegate_);
-  delegate_->OpenConnectedDevicesSettings();
-}
-
 void PhoneStatusView::OnModelChanged() {
   Update();
 }

ash/system/phonehub/phone_status_view.h

@@ -10,7 +10,6 @@
 #include "ash/system/tray/tri_view.h"
 #include "ash/system/unified/top_shortcut_button.h"
 #include "chromeos/components/phonehub/phone_model.h"
-#include "ui/views/controls/button/button.h"
 
 namespace views {
 class ImageView;
@@ -23,7 +22,6 @@ namespace ash {
 // status (wifi, volime, etc.).
 class ASH_EXPORT PhoneStatusView
     : public TriView,
-      public views::ButtonListener,
       public chromeos::phonehub::PhoneModel::Observer {
  public:
   class Delegate {
@@ -38,9 +36,6 @@ class ASH_EXPORT PhoneStatusView
   PhoneStatusView(PhoneStatusView&) = delete;
   PhoneStatusView operator=(PhoneStatusView&) = delete;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   // chromeos::phonehub::PhoneHubModel::Observer:
   void OnModelChanged() override;
 

ash/system/phonehub/phone_status_view_unittest.cc

@@ -12,6 +12,7 @@
 #include "chromeos/constants/chromeos_features.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
+#include "ui/views/test/button_test_api.h"
 
 namespace ash {
 
@@ -150,7 +151,8 @@ TEST_F(PhoneStatusViewTest, ClickOnSettings) {
   EXPECT_TRUE(status_view_->settings_button_->GetVisible());
 
   // Click on the settings button.
-  status_view_->ButtonPressed(status_view_->settings_button_, DummyEvent());
+  views::test::ButtonTestApi(status_view_->settings_button_)
+      .NotifyClick(DummyEvent());
   EXPECT_TRUE(connected_device_settings_opened_);
 }
 

ash/system/phonehub/task_continuation_view_unittest.cc

@@ -12,6 +12,7 @@
 #include "chromeos/components/phonehub/phone_model_test_util.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "ui/views/test/button_test_api.h"
 
 namespace ash {
 
@@ -119,7 +120,7 @@ TEST_F(TaskContinuationViewTest, TaskChipsView) {
           EXPECT_TRUE(from_user_interaction);
         });
     // Simulate clicking button using dummy event.
-    chip->ButtonPressed(nullptr, DummyEvent());
+    views::test::ButtonTestApi(chip).NotifyClick(DummyEvent());
   }
 }