Change ButtonPressed overrides to callbacks: ash/system/

Bug: 772945
Change-Id: I6623910c2a85b4b662e9bef1951f3c9f811eb0ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515100
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824033}

ash/system/audio/mic_gain_slider_controller.cc

@@ -41,20 +41,6 @@ views::View* MicGainSliderController::CreateView() {
   return nullptr;
 }
 
-void MicGainSliderController::ButtonPressed(views::Button* sender,
-                                            const ui::Event& event) {
-  bool is_muted = !CrasAudioHandler::Get()->IsInputMuted();
-
-  if (is_muted) {
-    base::RecordAction(base::UserMetricsAction("StatusArea_Mic_Muted"));
-  } else {
-    base::RecordAction(base::UserMetricsAction("StatusArea_Mic_Unmuted"));
-  }
-
-  CrasAudioHandler::Get()->SetMuteForDevice(
-      CrasAudioHandler::Get()->GetPrimaryActiveInputNode(), is_muted);
-}
-
 void MicGainSliderController::SliderValueChanged(
     views::Slider* sender,
     float value,
@@ -74,4 +60,15 @@ void MicGainSliderController::SliderValueChanged(
   CrasAudioHandler::Get()->SetInputGainPercent(value * 100);
 }
 
+void MicGainSliderController::SliderButtonPressed() {
+  auto* const audio_handler = CrasAudioHandler::Get();
+  const bool mute = !audio_handler->IsInputMuted();
+  if (mute)
+    base::RecordAction(base::UserMetricsAction("StatusArea_Mic_Muted"));
+  else
+    base::RecordAction(base::UserMetricsAction("StatusArea_Mic_Unmuted"));
+  audio_handler->SetMuteForDevice(audio_handler->GetPrimaryActiveInputNode(),
+                                  mute);
+}
+
 }  // namespace ash

ash/system/audio/mic_gain_slider_controller.h

@@ -31,11 +31,12 @@ class ASH_EXPORT MicGainSliderController : public UnifiedSliderListener {
 
   // UnifiedSliderListener:
   views::View* CreateView() override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   void SliderValueChanged(views::Slider* sender,
                           float value,
                           float old_value,
                           views::SliderChangeReason reason) override;
+
+  void SliderButtonPressed();
 };
 
 }  // namespace ash

ash/system/audio/mic_gain_slider_view.cc

@@ -20,9 +20,12 @@ namespace ash {
 MicGainSliderView::MicGainSliderView(MicGainSliderController* controller,
                                      uint64_t device_id,
                                      bool internal)
-    : UnifiedSliderView(controller,
-                        kImeMenuMicrophoneIcon,
-                        IDS_ASH_STATUS_TRAY_VOLUME_SLIDER_LABEL),
+    : UnifiedSliderView(
+          base::BindRepeating(&MicGainSliderController::SliderButtonPressed,
+                              base::Unretained(controller)),
+          controller,
+          kImeMenuMicrophoneIcon,
+          IDS_ASH_STATUS_TRAY_VOLUME_SLIDER_LABEL),
       device_id_(device_id),
       internal_(internal) {
   CrasAudioHandler::Get()->AddAudioObserver(this);

ash/system/audio/unified_volume_slider_controller.cc

@@ -25,24 +25,7 @@ UnifiedVolumeSliderController::UnifiedVolumeSliderController(
 UnifiedVolumeSliderController::~UnifiedVolumeSliderController() = default;
 
 views::View* UnifiedVolumeSliderController::CreateView() {
-  DCHECK(!slider_);
-  slider_ = new UnifiedVolumeView(this);
-  return slider_;
-}
-
-void UnifiedVolumeSliderController::ButtonPressed(views::Button* sender,
-                                                  const ui::Event& event) {
-  if (sender == slider_->button()) {
-    bool mute_on = !CrasAudioHandler::Get()->IsOutputMuted();
-    if (mute_on) {
-      base::RecordAction(base::UserMetricsAction("StatusArea_Audio_Muted"));
-    } else {
-      base::RecordAction(base::UserMetricsAction("StatusArea_Audio_Unmuted"));
-    }
-    CrasAudioHandler::Get()->SetOutputMute(mute_on);
-  } else if (sender == slider_->more_button()) {
-    delegate_->OnAudioSettingsButtonClicked();
-  }
+  return new UnifiedVolumeView(this, delegate_);
 }
 
 void UnifiedVolumeSliderController::SliderValueChanged(
@@ -69,4 +52,14 @@ void UnifiedVolumeSliderController::SliderValueChanged(
   }
 }
 
+void UnifiedVolumeSliderController::SliderButtonPressed() {
+  auto* const audio_handler = CrasAudioHandler::Get();
+  const bool mute = !audio_handler->IsOutputMuted();
+  if (mute)
+    base::RecordAction(base::UserMetricsAction("StatusArea_Audio_Muted"));
+  else
+    base::RecordAction(base::UserMetricsAction("StatusArea_Audio_Unmuted"));
+  audio_handler->SetOutputMute(mute);
+}
+
 }  // namespace ash

ash/system/audio/unified_volume_slider_controller.h

@@ -9,8 +9,6 @@
 
 namespace ash {
 
-class UnifiedVolumeView;
-
 // Controller of a slider that can change audio volume.
 class UnifiedVolumeSliderController : public UnifiedSliderListener {
  public:
@@ -25,15 +23,15 @@ class UnifiedVolumeSliderController : public UnifiedSliderListener {
 
   // UnifiedSliderListener:
   views::View* CreateView() override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   void SliderValueChanged(views::Slider* sender,
                           float value,
                           float old_value,
                           views::SliderChangeReason reason) override;
 
+  void SliderButtonPressed();
+
  private:
   Delegate* const delegate_;
-  UnifiedVolumeView* slider_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(UnifiedVolumeSliderController);
 };

ash/system/audio/unified_volume_view.cc

@@ -8,7 +8,6 @@
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/style/ash_color_provider.h"
-#include "ash/system/audio/unified_volume_slider_controller.h"
 #include "ash/system/tray/tray_popup_utils.h"
 #include "base/i18n/rtl.h"
 #include "base/stl_util.h"
@@ -61,8 +60,8 @@ SkColor GetBackgroundColorOfMoreButton() {
 
 class MoreButton : public views::Button {
  public:
-  explicit MoreButton(views::ButtonListener* listener)
-      : views::Button(listener) {
+  explicit MoreButton(PressedCallback callback)
+      : views::Button(std::move(callback)) {
     SetLayoutManager(std::make_unique<views::BoxLayout>(
         views::BoxLayout::Orientation::kHorizontal,
         gfx::Insets((kTrayItemSize -
@@ -133,11 +132,19 @@ class MoreButton : public views::Button {
 
 }  // namespace
 
-UnifiedVolumeView::UnifiedVolumeView(UnifiedVolumeSliderController* controller)
-    : UnifiedSliderView(controller,
+UnifiedVolumeView::UnifiedVolumeView(
+    UnifiedVolumeSliderController* controller,
+    UnifiedVolumeSliderController::Delegate* delegate)
+    : UnifiedSliderView(base::BindRepeating(
+                            &UnifiedVolumeSliderController::SliderButtonPressed,
+                            base::Unretained(controller)),
+                        controller,
                         kSystemMenuVolumeHighIcon,
                         IDS_ASH_STATUS_TRAY_VOLUME_SLIDER_LABEL),
-      more_button_(new MoreButton(controller)) {
+      more_button_(new MoreButton(
+          base::BindRepeating(&UnifiedVolumeSliderController::Delegate::
+                                  OnAudioSettingsButtonClicked,
+                              base::Unretained(delegate)))) {
   CrasAudioHandler::Get()->AddAudioObserver(this);
   AddChildView(more_button_);
   Update(false /* by_user */);

ash/system/audio/unified_volume_view.h

@@ -5,22 +5,20 @@
 #ifndef ASH_SYSTEM_AUDIO_UNIFIED_VOLUME_VIEW_H_
 #define ASH_SYSTEM_AUDIO_UNIFIED_VOLUME_VIEW_H_
 
+#include "ash/system/audio/unified_volume_slider_controller.h"
 #include "ash/system/unified/unified_slider_view.h"
 #include "chromeos/audio/cras_audio_handler.h"
 
 namespace ash {
 
-class UnifiedVolumeSliderController;
-
 // View of a slider that can change audio volume.
 class UnifiedVolumeView : public UnifiedSliderView,
                           public chromeos::CrasAudioHandler::AudioObserver {
  public:
-  explicit UnifiedVolumeView(UnifiedVolumeSliderController* controller);
+  UnifiedVolumeView(UnifiedVolumeSliderController* controller,
+                    UnifiedVolumeSliderController::Delegate* delegate);
   ~UnifiedVolumeView() override;
 
-  views::Button* more_button() { return more_button_; }
-
   // views::View:
   const char* GetClassName() const override;
 

ash/system/brightness/unified_brightness_slider_controller.cc

@@ -33,11 +33,6 @@ views::View* UnifiedBrightnessSliderController::CreateView() {
   return slider_;
 }
 
-void UnifiedBrightnessSliderController::ButtonPressed(views::Button* sender,
-                                                      const ui::Event& event) {
-  // The button in is UnifiedBrightnessView is no-op.
-}
-
 void UnifiedBrightnessSliderController::SliderValueChanged(
     views::Slider* sender,
     float value,

ash/system/brightness/unified_brightness_slider_controller.h

@@ -19,7 +19,6 @@ class UnifiedBrightnessSliderController : public UnifiedSliderListener {
 
   // UnifiedSliderListener:
   views::View* CreateView() override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   void SliderValueChanged(views::Slider* sender,
                           float value,
                           float old_value,

ash/system/brightness/unified_brightness_view.cc

@@ -14,7 +14,8 @@ namespace ash {
 UnifiedBrightnessView::UnifiedBrightnessView(
     UnifiedBrightnessSliderController* controller,
     UnifiedSystemTrayModel* model)
-    : UnifiedSliderView(controller,
+    : UnifiedSliderView(views::Button::PressedCallback(),
+                        controller,
                         kUnifiedMenuBrightnessIcon,
                         IDS_ASH_STATUS_TRAY_BRIGHTNESS),
       model_(model) {

ash/system/ime_menu/ime_menu_tray.cc

@@ -41,7 +41,6 @@
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/range/range.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/scroll_view.h"
@@ -66,12 +65,6 @@ gfx::Range GetImeListViewRange() {
   return gfx::Range(tray_item_height * min_items, tray_item_height * max_items);
 }
 
-// Shows language and input settings page.
-void ShowIMESettings() {
-  base::RecordAction(base::UserMetricsAction("StatusArea_IME_Detailed"));
-  Shell::Get()->system_tray_model()->client()->ShowIMESettings();
-}
-
 // Returns true if the current screen is login or lock screen.
 bool IsInLoginOrLockScreen() {
   using session_manager::SessionState;
@@ -120,15 +113,8 @@ class ImeMenuImageView : public views::ImageView {
   DISALLOW_COPY_AND_ASSIGN(ImeMenuImageView);
 };
 
-SystemMenuButton* CreateImeMenuButton(views::ButtonListener* listener,
-                                      const gfx::VectorIcon& icon,
-                                      int accessible_name_id,
-                                      int right_border) {
-  return new SystemMenuButton(listener, icon, accessible_name_id);
-}
-
 // The view that contains IME menu title.
-class ImeTitleView : public views::View, public views::ButtonListener {
+class ImeTitleView : public views::View {
  public:
   explicit ImeTitleView() {
     SetBorder(views::CreatePaddedBorder(
@@ -153,16 +139,15 @@ class ImeTitleView : public views::View, public views::ButtonListener {
     layout_ptr->SetFlexForView(title_label, 1);
 
     settings_button_ = AddChildView(std::make_unique<TopShortcutButton>(
-        this, kSystemMenuSettingsIcon, IDS_ASH_STATUS_TRAY_IME_SETTINGS));
+        base::BindRepeating([]() {
+          base::RecordAction(
+              base::UserMetricsAction("StatusArea_IME_Detailed"));
+          Shell::Get()->system_tray_model()->client()->ShowIMESettings();
+        }),
+        kSystemMenuSettingsIcon, IDS_ASH_STATUS_TRAY_IME_SETTINGS));
     settings_button_->SetEnabled(TrayPopupUtils::CanOpenWebUISettings());
   }
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override {
-    DCHECK_EQ(sender, settings_button_);
-    ShowIMESettings();
-  }
-
   ~ImeTitleView() override = default;
 
   // views::View:
@@ -175,7 +160,7 @@ class ImeTitleView : public views::View, public views::ButtonListener {
 };
 
 // The view that contains buttons shown on the bottom of IME menu.
-class ImeButtonsView : public views::View, public views::ButtonListener {
+class ImeButtonsView : public views::View {
  public:
   ImeButtonsView(ImeMenuTray* ime_menu_tray,
                  bool show_emoji,
@@ -189,28 +174,16 @@ class ImeButtonsView : public views::View, public views::ButtonListener {
 
   ~ImeButtonsView() override = default;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override {
-    // The |keyset| will be used for drawing input view keyset in IME
-    // extensions. ImeMenuTray::ShowKeyboardWithKeyset() will deal with
-    // the |keyset| string to generate the right input view url.
-    chromeos::input_method::ImeKeyset keyset =
-        chromeos::input_method::ImeKeyset::kNone;
-    if (sender == emoji_button_)
-      keyset = chromeos::input_method::ImeKeyset::kEmoji;
-    else if (sender == voice_button_)
-      keyset = chromeos::input_method::ImeKeyset::kVoice;
-    else if (sender == handwriting_button_)
-      keyset = chromeos::input_method::ImeKeyset::kHandwriting;
-    else
-      NOTREACHED();
-
+  void KeysetButtonPressed(chromeos::input_method::ImeKeyset keyset) {
     // TODO(dcheng): When https://crbug.com/742517 is fixed, Mojo will generate
     // a constant for the number of values in the enum. For now, we just define
     // it here and keep it in sync with the enum.
     const int kImeKeysetUmaBoundary = 4;
     UMA_HISTOGRAM_ENUMERATION("InputMethod.ImeMenu.EmojiHandwritingVoiceButton",
                               keyset, kImeKeysetUmaBoundary);
+    // The |keyset| will be used for drawing input view keyset in IME
+    // extensions. ImeMenuTray::ShowKeyboardWithKeyset() will deal with
+    // the |keyset| string to generate the right input view url.
     ime_menu_tray_->ShowKeyboardWithKeyset(keyset);
   }
 
@@ -231,26 +204,31 @@ class ImeButtonsView : public views::View, public views::ButtonListener {
         gfx::Insets(kMenuSeparatorVerticalPadding - kMenuSeparatorWidth,
                     kMenuExtraMarginFromLeftEdge)));
 
-    const int right_border = 1;
     if (show_emoji) {
-      emoji_button_ =
-          CreateImeMenuButton(this, kImeMenuEmoticonIcon,
-                              IDS_ASH_STATUS_TRAY_IME_EMOJI, right_border);
+      emoji_button_ = new SystemMenuButton(
+          base::BindRepeating(&ImeButtonsView::KeysetButtonPressed,
+                              base::Unretained(this),
+                              chromeos::input_method::ImeKeyset::kEmoji),
+          kImeMenuEmoticonIcon, IDS_ASH_STATUS_TRAY_IME_EMOJI);
       emoji_button_->SetID(kEmojiButtonId);
       AddChildView(emoji_button_);
     }
 
     if (show_handwriting) {
-      handwriting_button_ = CreateImeMenuButton(
-          this, kImeMenuWriteIcon, IDS_ASH_STATUS_TRAY_IME_HANDWRITING,
-          right_border);
+      handwriting_button_ = new SystemMenuButton(
+          base::BindRepeating(&ImeButtonsView::KeysetButtonPressed,
+                              base::Unretained(this),
+                              chromeos::input_method::ImeKeyset::kHandwriting),
+          kImeMenuWriteIcon, IDS_ASH_STATUS_TRAY_IME_HANDWRITING);
       AddChildView(handwriting_button_);
     }
 
     if (show_voice) {
-      voice_button_ =
-          CreateImeMenuButton(this, kImeMenuMicrophoneIcon,
-                              IDS_ASH_STATUS_TRAY_IME_VOICE, right_border);
+      voice_button_ = new SystemMenuButton(
+          base::BindRepeating(&ImeButtonsView::KeysetButtonPressed,
+                              base::Unretained(this),
+                              chromeos::input_method::ImeKeyset::kVoice),
+          kImeMenuMicrophoneIcon, IDS_ASH_STATUS_TRAY_IME_VOICE);
       AddChildView(voice_button_);
     }
   }

ash/system/keyboard_brightness/unified_keyboard_brightness_slider_controller.cc

@@ -18,7 +18,8 @@ class UnifiedKeyboardBrightnessView : public UnifiedSliderView,
   UnifiedKeyboardBrightnessView(
       UnifiedKeyboardBrightnessSliderController* controller,
       UnifiedSystemTrayModel* model)
-      : UnifiedSliderView(controller,
+      : UnifiedSliderView(views::Button::PressedCallback(),
+                          controller,
                           kUnifiedMenuKeyboardBrightnessIcon,
                           IDS_ASH_STATUS_TRAY_BRIGHTNESS,
                           true /* readonly*/),
@@ -55,12 +56,6 @@ views::View* UnifiedKeyboardBrightnessSliderController::CreateView() {
   return slider_;
 }
 
-void UnifiedKeyboardBrightnessSliderController::ButtonPressed(
-    views::Button* sender,
-    const ui::Event& event) {
-  // This slider is read-only.
-}
-
 void UnifiedKeyboardBrightnessSliderController::SliderValueChanged(
     views::Slider* sender,
     float value,

ash/system/keyboard_brightness/unified_keyboard_brightness_slider_controller.h

@@ -20,7 +20,6 @@ class UnifiedKeyboardBrightnessSliderController : public UnifiedSliderListener {
 
   // UnifiedSliderListener:
   views::View* CreateView() override;
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
   void SliderValueChanged(views::Slider* sender,
                           float value,
                           float old_value,

ash/system/tray/system_menu_button.cc

@@ -40,10 +40,10 @@ SystemMenuButton::SystemMenuButton(PressedCallback callback,
       AshColorProvider::ControlsLayerType::kFocusRingColor));
 }
 
-SystemMenuButton::SystemMenuButton(views::ButtonListener* listener,
+SystemMenuButton::SystemMenuButton(PressedCallback callback,
                                    const gfx::VectorIcon& icon,
                                    int accessible_name_id)
-    : SystemMenuButton(PressedCallback(listener, this),
+    : SystemMenuButton(std::move(callback),
                        gfx::ImageSkia(),
                        gfx::ImageSkia(),
                        accessible_name_id) {

ash/system/tray/system_menu_button.h

@@ -30,7 +30,7 @@ class SystemMenuButton : public views::ImageButton {
   // Similar to the above constructor. Just gets a single vector icon and
   // creates the normal and disabled icons based on that using default menu icon
   // colors.
-  SystemMenuButton(views::ButtonListener* listener,
+  SystemMenuButton(PressedCallback callback,
                    const gfx::VectorIcon& icon,
                    int accessible_name_id);
   ~SystemMenuButton() override;

ash/system/unified/unified_slider_view.cc

@@ -83,10 +83,10 @@ std::unique_ptr<views::Slider> CreateSlider(UnifiedSliderListener* listener,
 
 }  // namespace
 
-UnifiedSliderButton::UnifiedSliderButton(views::ButtonListener* listener,
+UnifiedSliderButton::UnifiedSliderButton(PressedCallback callback,
                                          const gfx::VectorIcon& icon,
                                          int accessible_name_id)
-    : views::ImageButton(listener) {
+    : views::ImageButton(std::move(callback)) {
   SetImageHorizontalAlignment(ALIGN_CENTER);
   SetImageVerticalAlignment(ALIGN_MIDDLE);
   if (accessible_name_id)
@@ -182,12 +182,13 @@ void UnifiedSliderButton::UpdateVectorIcon() {
       this, *icon_, toggled_, GetDefaultSizeOfVectorIcon(*icon_));
 }
 
-UnifiedSliderView::UnifiedSliderView(UnifiedSliderListener* listener,
+UnifiedSliderView::UnifiedSliderView(views::Button::PressedCallback callback,
+                                     UnifiedSliderListener* listener,
                                      const gfx::VectorIcon& icon,
                                      int accessible_name_id,
                                      bool readonly)
     : button_(AddChildView(
-          std::make_unique<UnifiedSliderButton>(listener,
+          std::make_unique<UnifiedSliderButton>(std::move(callback),
                                                 icon,
                                                 accessible_name_id))),
       slider_(AddChildView(CreateSlider(listener, readonly))) {

ash/system/unified/unified_slider_view.h

@@ -13,8 +13,7 @@
 
 namespace ash {
 
-class UnifiedSliderListener : public views::ButtonListener,
-                              public views::SliderListener {
+class UnifiedSliderListener : public views::SliderListener {
  public:
   // Instantiates UnifiedSliderView. The view will be onwed by views hierarchy.
   // The view should be always deleted after the controller is destructed.
@@ -26,7 +25,7 @@ class UnifiedSliderListener : public views::ButtonListener,
 // A button used in a slider row of UnifiedSystemTray. The button is togglable.
 class UnifiedSliderButton : public views::ImageButton {
  public:
-  UnifiedSliderButton(views::ButtonListener* listener,
+  UnifiedSliderButton(PressedCallback callback,
                       const gfx::VectorIcon& icon,
                       int accessible_name_id);
   ~UnifiedSliderButton() override;
@@ -64,7 +63,8 @@ class UnifiedSliderButton : public views::ImageButton {
 class UnifiedSliderView : public views::View {
  public:
   // If |readonly| is set, the slider will not accept any user events.
-  UnifiedSliderView(UnifiedSliderListener* listener,
+  UnifiedSliderView(views::Button::PressedCallback callback,
+                    UnifiedSliderListener* listener,
                     const gfx::VectorIcon& icon,
                     int accessible_name_id,
                     bool readonly = false);