Change ButtonPressed overrides to callbacks: c/b/ui/views/accessibility/

Bug: 772945
Change-Id: I929d7a955c9671748b2e7f7a370a39eecf1dbd80
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446653
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814727}

chrome/browser/ui/views/accessibility/caption_bubble.cc

@@ -348,16 +348,23 @@ void CaptionBubble::Init() {
       ->set_cross_axis_alignment(views::BoxLayout::CrossAxisAlignment::kCenter);
   error_message->SetVisible(false);
 
+  views::Button::PressedCallback expand_or_collapse_callback =
+      base::BindRepeating(&CaptionBubble::ExpandOrCollapseButtonPressed,
+                          base::Unretained(this));
   auto expand_button =
-      BuildImageButton(kCaretDownIcon, IDS_LIVE_CAPTION_BUBBLE_EXPAND);
+      BuildImageButton(expand_or_collapse_callback, kCaretDownIcon,
+                       IDS_LIVE_CAPTION_BUBBLE_EXPAND);
   expand_button->SetVisible(!is_expanded_);
 
   auto collapse_button =
-      BuildImageButton(kCaretUpIcon, IDS_LIVE_CAPTION_BUBBLE_COLLAPSE);
+      BuildImageButton(std::move(expand_or_collapse_callback), kCaretUpIcon,
+                       IDS_LIVE_CAPTION_BUBBLE_COLLAPSE);
   collapse_button->SetVisible(is_expanded_);
 
-  auto close_button = BuildImageButton(vector_icons::kCloseRoundedIcon,
-                                       IDS_LIVE_CAPTION_BUBBLE_CLOSE);
+  auto close_button = BuildImageButton(
+      base::BindRepeating(&CaptionBubble::CloseButtonPressed,
+                          base::Unretained(this)),
+      vector_icons::kCloseRoundedIcon, IDS_LIVE_CAPTION_BUBBLE_CLOSE);
 
   title_ = content_container->AddChildView(std::move(title));
   label_ = content_container->AddChildView(std::move(label));
@@ -378,9 +385,10 @@ void CaptionBubble::Init() {
 }
 
 std::unique_ptr<views::ImageButton> CaptionBubble::BuildImageButton(
+    views::Button::PressedCallback callback,
     const gfx::VectorIcon& icon,
     const int tooltip_text_id) {
-  auto button = views::CreateVectorImageButton(this);
+  auto button = views::CreateVectorImageButton(std::move(callback));
   views::SetImageFromVectorIcon(button.get(), icon, kButtonDip, SK_ColorWHITE);
   button->SetTooltipText(l10n_util::GetStringUTF16(tooltip_text_id));
   button->SetInkDropBaseColor(SkColor(gfx::kGoogleGrey600));
@@ -491,27 +499,27 @@ void CaptionBubble::AddedToWidget() {
                                static_cast<BubbleDialogDelegate*>(this));
 }
 
-void CaptionBubble::ButtonPressed(views::Button* sender,
-                                  const ui::Event& event) {
-  if (sender == close_button_) {
-    // TODO(crbug.com/1055150): This histogram currently only reports a single
-    // bucket, but it will eventually be extended to report session starts and
-    // natural session ends (when the audio stream ends).
-    UMA_HISTOGRAM_ENUMERATION(
-        "Accessibility.LiveCaption.Session",
-        CaptionController::SessionEvent::kCloseButtonClicked);
-    if (model_)
-      model_->Close();
-  } else if (sender == expand_button_ || sender == collapse_button_) {
-    is_expanded_ = !is_expanded_;
-    bool button_had_focus = sender->HasFocus();
-    views::Button* new_button =
-        is_expanded_ ? collapse_button_ : expand_button_;
-    OnIsExpandedChanged();
-    // TODO(crbug.com/1055150): Ensure that the button keeps focus on mac.
-    if (button_had_focus)
-      new_button->RequestFocus();
-  }
+void CaptionBubble::CloseButtonPressed() {
+  // TODO(crbug.com/1055150): This histogram currently only reports a single
+  // bucket, but it will eventually be extended to report session starts and
+  // natural session ends (when the audio stream ends).
+  UMA_HISTOGRAM_ENUMERATION(
+      "Accessibility.LiveCaption.Session",
+      CaptionController::SessionEvent::kCloseButtonClicked);
+  if (model_)
+    model_->Close();
+}
+
+void CaptionBubble::ExpandOrCollapseButtonPressed() {
+  is_expanded_ = !is_expanded_;
+  views::Button *old_button = collapse_button_, *new_button = expand_button_;
+  if (is_expanded_)
+    std::swap(old_button, new_button);
+  bool button_had_focus = old_button->HasFocus();
+  OnIsExpandedChanged();
+  // TODO(crbug.com/1055150): Ensure that the button keeps focus on mac.
+  if (button_had_focus)
+    new_button->RequestFocus();
 }
 
 void CaptionBubble::SetModel(CaptionBubbleModel* model) {

chrome/browser/ui/views/accessibility/caption_bubble.h

@@ -38,8 +38,7 @@ class CaptionBubbleFrameView;
 //  A caption bubble that floats above the BrowserView and shows automatically-
 //  generated text captions for audio and media streams from the current tab.
 //
-class CaptionBubble : public views::BubbleDialogDelegateView,
-                      public views::ButtonListener {
+class CaptionBubble : public views::BubbleDialogDelegateView {
  public:
   CaptionBubble(views::View* anchor,
                 BrowserView* browser_view,
@@ -89,13 +88,13 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   void AddedToWidget() override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
  private:
   friend class CaptionBubbleControllerViewsTest;
   friend class CaptionBubbleModel;
 
+  void CloseButtonPressed();
+  void ExpandOrCollapseButtonPressed();
+
   // Called by CaptionBubbleModel to notify this object that the model's text
   // has changed. Sets the text of the caption bubble to the model's text.
   void OnTextChanged();
@@ -120,6 +119,7 @@ class CaptionBubble : public views::BubbleDialogDelegateView,
   void UpdateContentSize();
   void Redraw();
   std::unique_ptr<views::ImageButton> BuildImageButton(
+      views::Button::PressedCallback callback,
       const gfx::VectorIcon& icon,
       const int tooltip_text_id);
 

chrome/browser/ui/views/accessibility/invert_bubble_view.cc

@@ -44,19 +44,7 @@ constexpr char kLearnMoreUrl[] =
     "https://groups.google.com/a/googleproductforums.com/d/topic/chrome/"
     "Xrco2HsXS-8/discussion";
 
-// Tag value used to uniquely identify the "learn more" (?) button.
-constexpr int kLearnMoreButton = 100;
-
-std::unique_ptr<views::View> CreateExtraView(views::ButtonListener* listener) {
-  auto learn_more = views::CreateVectorImageButtonWithNativeTheme(
-      listener, vector_icons::kHelpOutlineIcon);
-  learn_more->SetTooltipText(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
-  learn_more->set_tag(kLearnMoreButton);
-  return learn_more;
-}
-
-class InvertBubbleView : public views::BubbleDialogDelegateView,
-                         public views::ButtonListener {
+class InvertBubbleView : public views::BubbleDialogDelegateView {
  public:
   InvertBubbleView(Browser* browser, views::View* anchor_view);
   ~InvertBubbleView() override;
@@ -69,9 +57,6 @@ class InvertBubbleView : public views::BubbleDialogDelegateView,
   base::string16 GetWindowTitle() const override;
   bool ShouldShowCloseButton() const override;
 
-  // Overridden from views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   void OpenLink(const std::string& url, const ui::Event& event);
 
   Browser* browser_;
@@ -85,7 +70,11 @@ InvertBubbleView::InvertBubbleView(Browser* browser, views::View* anchor_view)
       browser_(browser) {
   SetButtons(ui::DIALOG_BUTTON_OK);
   SetButtonLabel(ui::DIALOG_BUTTON_OK, l10n_util::GetStringUTF16(IDS_DONE));
-  SetExtraView(::CreateExtraView(this));
+  SetExtraView(views::CreateVectorImageButtonWithNativeTheme(
+                   base::BindRepeating(&InvertBubbleView::OpenLink,
+                                       base::Unretained(this), kLearnMoreUrl),
+                   vector_icons::kHelpOutlineIcon))
+      ->SetTooltipText(l10n_util::GetStringUTF16(IDS_LEARN_MORE));
   set_margins(gfx::Insets());
   chrome::RecordDialogCreation(chrome::DialogIdentifier::INVERT);
 }
@@ -138,12 +127,6 @@ bool InvertBubbleView::ShouldShowCloseButton() const {
   return true;
 }
 
-void InvertBubbleView::ButtonPressed(views::Button* sender,
-                                     const ui::Event& event) {
-  if (sender->tag() == kLearnMoreButton)
-    OpenLink(kLearnMoreUrl, event);
-}
-
 void InvertBubbleView::OpenLink(const std::string& url,
                                 const ui::Event& event) {
   WindowOpenDisposition disposition = ui::DispositionFromEventFlags(