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

This also reorders the definitions in one file to match the declarations.

Bug: 772945
Change-Id: I74f04421cdab8acd02e380a317155bd1bdcc2a62
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454789
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Reviewed-by: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814702}

chrome/browser/ui/views/md_text_button_with_down_arrow.cc

@@ -12,9 +12,9 @@
 
 namespace views {
 
-MdTextButtonWithDownArrow::MdTextButtonWithDownArrow(ButtonListener* listener,
+MdTextButtonWithDownArrow::MdTextButtonWithDownArrow(PressedCallback callback,
                                                      const base::string16& text)
-    : MdTextButton(listener, text) {
+    : MdTextButton(std::move(callback), text) {
   SetFocusForPlatform();
   SetHorizontalAlignment(gfx::ALIGN_RIGHT);
   SetImageLabelSpacing(LayoutProvider::Get()->GetDistanceMetric(
@@ -29,6 +29,10 @@ MdTextButtonWithDownArrow::MdTextButtonWithDownArrow(ButtonListener* listener,
                                   DISTANCE_DROPDOWN_BUTTON_RIGHT_MARGIN)));
 }
 
+MdTextButtonWithDownArrow::MdTextButtonWithDownArrow(ButtonListener* listener,
+                                                     const base::string16& text)
+    : MdTextButton(PressedCallback(listener, this), text) {}
+
 MdTextButtonWithDownArrow::~MdTextButtonWithDownArrow() = default;
 
 void MdTextButtonWithDownArrow::OnThemeChanged() {

chrome/browser/ui/views/md_text_button_with_down_arrow.h

@@ -17,6 +17,8 @@ class ButtonListener;
 // right side.
 class MdTextButtonWithDownArrow : public MdTextButton {
  public:
+  MdTextButtonWithDownArrow(PressedCallback callback,
+                            const base::string16& text);
   MdTextButtonWithDownArrow(ButtonListener* listener,
                             const base::string16& text);
   ~MdTextButtonWithDownArrow() override;

chrome/browser/ui/views/webauthn/authenticator_request_dialog_view.h

@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "chrome/browser/webauthn/authenticator_request_dialog_model.h"
 #include "content/public/browser/web_contents_observer.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/window/dialog_delegate.h"
 
@@ -37,8 +36,7 @@ class AuthenticatorRequestSheetView;
 class AuthenticatorRequestDialogView
     : public views::DialogDelegateView,
       public AuthenticatorRequestDialogModel::Observer,
-      public content::WebContentsObserver,
-      public views::ButtonListener {
+      public content::WebContentsObserver {
  public:
   ~AuthenticatorRequestDialogView() override;
 
@@ -80,9 +78,6 @@ class AuthenticatorRequestDialogView
   void OnStepTransition() override;
   void OnSheetModelChanged() override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   void OnVisibilityChanged(content::Visibility visibility) override;
 
  private:
@@ -99,6 +94,8 @@ class AuthenticatorRequestDialogView
   // Shows the dialog after creation or after being hidden.
   void Show();
 
+  void OtherTransportsButtonPressed();
+
   void OnDialogClosing();
 
   std::unique_ptr<AuthenticatorRequestDialogModel> model_;

chrome/browser/ui/views/webauthn/authenticator_request_sheet_view.cc

@@ -72,12 +72,6 @@ AuthenticatorRequestSheetView::BuildStepSpecificContent() {
   return nullptr;
 }
 
-void AuthenticatorRequestSheetView::ButtonPressed(views::Button* sender,
-                                                  const ui::Event& event) {
-  DCHECK_EQ(sender, back_arrow_button_);
-  model()->OnBack();
-}
-
 std::unique_ptr<views::View>
 AuthenticatorRequestSheetView::CreateIllustrationWithOverlays() {
   const int illustration_width = ChromeLayoutProvider::Get()->GetDistanceMetric(
@@ -108,7 +102,8 @@ AuthenticatorRequestSheetView::CreateIllustrationWithOverlays() {
   }
 
   if (model()->IsBackButtonVisible()) {
-    auto back_arrow = views::CreateVectorImageButton(this);
+    auto back_arrow = views::CreateVectorImageButton(base::BindRepeating(
+        &AuthenticatorRequestSheetModel::OnBack, base::Unretained(model())));
     back_arrow->SetFocusForPlatform();
     back_arrow->SetAccessibleName(l10n_util::GetStringUTF16(
         IDS_BACK_BUTTON_AUTHENTICATOR_REQUEST_DIALOG));

chrome/browser/ui/views/webauthn/authenticator_request_sheet_view.h

@@ -9,7 +9,6 @@
 
 #include "base/macros.h"
 #include "base/strings/string16.h"
-#include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/view.h"
 
@@ -57,8 +56,7 @@ class NonAccessibleImageView;
 // TODO(https://crbug.com/852352): The Web Authentication and Web Payment APIs
 // both use the concept of showing multiple "sheets" in a single dialog. To
 // avoid code duplication, consider factoring out common parts.
-class AuthenticatorRequestSheetView : public views::View,
-                                      public views::ButtonListener {
+class AuthenticatorRequestSheetView : public views::View {
  public:
   explicit AuthenticatorRequestSheetView(
       std::unique_ptr<AuthenticatorRequestSheetModel> model);
@@ -81,9 +79,6 @@ class AuthenticatorRequestSheetView : public views::View,
   // Returns the step-specific view the derived sheet wishes to provide, if any.
   virtual std::unique_ptr<views::View> BuildStepSpecificContent();
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
  private:
   // Creates the upper half of the sheet, consisting of a pretty illustration
   // overlayed with absolutely positioned controls (the activity indicator and

chrome/browser/ui/views/webauthn/hover_list_view.cc

@@ -27,8 +27,6 @@
 
 namespace {
 
-constexpr int kPlaceHolderItemTag = -1;
-
 enum class ItemType {
   kButton,
   kPlaceholder,
@@ -36,14 +34,12 @@ enum class ItemType {
 };
 
 std::unique_ptr<WebAuthnHoverButton> CreateHoverButtonForListItem(
-    int item_tag,
     const gfx::VectorIcon* vector_icon,
     base::string16 item_title,
     base::string16 item_description,
-    views::ButtonListener* listener,
+    views::Button::PressedCallback callback,
     bool is_two_line_item,
     ItemType item_type = ItemType::kButton) {
-
   auto item_image = std::make_unique<views::ImageView>();
   // TODO - Icon color should be set and updated in OnThemeChanged
   const SkColor icon_color = color_utils::DeriveDefaultIconColor(
@@ -83,9 +79,8 @@ std::unique_ptr<WebAuthnHoverButton> CreateHoverButtonForListItem(
   }
 
   auto hover_button = std::make_unique<WebAuthnHoverButton>(
-      listener, std::move(item_image), item_title, item_description,
+      std::move(callback), std::move(item_image), item_title, item_description,
       std::move(secondary_view), is_two_line_item);
-  hover_button->set_tag(item_tag);
 
   switch (item_type) {
     case ItemType::kPlaceholder: {
@@ -136,8 +131,11 @@ HoverListView::HoverListView(std::unique_ptr<HoverListModel> model)
 
   for (const auto item_tag : model_->GetThrobberTags()) {
     auto button = CreateHoverButtonForListItem(
-        item_tag, model_->GetItemIcon(item_tag), model_->GetItemText(item_tag),
-        model_->GetDescriptionText(item_tag), this, true, ItemType::kThrobber);
+        model_->GetItemIcon(item_tag), model_->GetItemText(item_tag),
+        model_->GetDescriptionText(item_tag),
+        base::BindRepeating(&HoverListModel::OnListItemSelected,
+                            base::Unretained(model_.get()), item_tag),
+        true, ItemType::kThrobber);
     throbber_views_.push_back(button.get());
     item_container_->AddChildView(button.release());
     AddSeparatorAsChild(item_container_);
@@ -172,7 +170,10 @@ void HoverListView::AppendListItemView(const gfx::VectorIcon* icon,
                                        base::string16 description_text,
                                        int item_tag) {
   auto hover_button = CreateHoverButtonForListItem(
-      item_tag, icon, item_text, description_text, this, is_two_line_list_);
+      icon, item_text, description_text,
+      base::BindRepeating(&HoverListModel::OnListItemSelected,
+                          base::Unretained(model_.get()), item_tag),
+      is_two_line_list_);
 
   auto* list_item_view_ptr = hover_button.release();
   item_container_->AddChildView(list_item_view_ptr);
@@ -183,8 +184,8 @@ void HoverListView::AppendListItemView(const gfx::VectorIcon* icon,
 
 void HoverListView::CreateAndAppendPlaceholderItem() {
   auto placeholder_item = CreateHoverButtonForListItem(
-      kPlaceHolderItemTag, model_->GetPlaceholderIcon(),
-      model_->GetPlaceholderText(), base::string16(), nullptr,
+      model_->GetPlaceholderIcon(), model_->GetPlaceholderText(),
+      base::string16(), views::Button::PressedCallback(),
       /*is_two_line_list=*/false, ItemType::kPlaceholder);
   item_container_->AddChildView(placeholder_item.get());
   auto* separator = AddSeparatorAsChild(item_container_);
@@ -273,11 +274,6 @@ void HoverListView::OnListItemChanged(int changed_list_item_tag,
   }
 }
 
-void HoverListView::ButtonPressed(views::Button* sender,
-                                  const ui::Event& event) {
-  model_->OnListItemSelected(sender->tag());
-}
-
 int HoverListView::GetPreferredViewHeight() const {
   constexpr int kMaxViewHeight = 300;
 
@@ -296,8 +292,8 @@ int HoverListView::GetPreferredViewHeight() const {
       model_->GetPreferredItemCount() - tags_to_list_item_views_.size();
   if (reserved_items > 0) {
     auto dummy_hover_button = CreateHoverButtonForListItem(
-        -1 /* tag */, &gfx::kNoneIcon, base::string16(), base::string16(),
-        nullptr /* listener */, is_two_line_list_);
+        &gfx::kNoneIcon, base::string16(), base::string16(),
+        views::Button::PressedCallback(), is_two_line_list_);
     const auto list_item_height =
         separator_height + dummy_hover_button->GetPreferredSize().height();
     size += list_item_height * reserved_items;

chrome/browser/ui/views/webauthn/hover_list_view.h

@@ -40,7 +40,6 @@ class WebAuthnHoverButton;
 //  +----------------------------------+
 //
 class HoverListView : public views::View,
-                      public views::ButtonListener,
                       public HoverListModel::Observer {
  public:
   explicit HoverListView(std::unique_ptr<HoverListModel> model);
@@ -72,9 +71,6 @@ class HoverListView : public views::View,
   void OnListItemChanged(int changed_list_item_tag,
                          HoverListModel::ListItemChangeType type) override;
 
-  // views::ButtonListener:
-  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
-
   std::unique_ptr<HoverListModel> model_;
   std::map<int, ListItemViews> tags_to_list_item_views_;
   std::vector<WebAuthnHoverButton*> throbber_views_;

chrome/browser/ui/views/webauthn/webauthn_hover_button.cc

@@ -38,13 +38,13 @@ class IconWrapper : public views::View {
 }  // namespace
 
 WebAuthnHoverButton::WebAuthnHoverButton(
-    views::ButtonListener* listener,
+    PressedCallback callback,
     std::unique_ptr<views::ImageView> icon,
     const base::string16& title_text,
     const base::string16& subtitle_text,
     std::unique_ptr<views::View> secondary_icon,
     bool force_two_line)
-    : HoverButton(listener, base::string16()) {
+    : HoverButton(std::move(callback), base::string16()) {
   ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get();
 
   views::GridLayout* grid_layout =

chrome/browser/ui/views/webauthn/webauthn_hover_button.h

@@ -11,7 +11,6 @@
 #include "chrome/browser/ui/views/hover_button.h"
 
 namespace views {
-class ButtonListener;
 class ImageView;
 class Label;
 class View;
@@ -36,7 +35,7 @@ class WebAuthnHoverButton : public HoverButton {
   //
   // |icon| and |secondary_icon| are also optional. If either is null, the
   // middle column resizes to fill the space.
-  WebAuthnHoverButton(views::ButtonListener* listener,
+  WebAuthnHoverButton(PressedCallback callback,
                       std::unique_ptr<views::ImageView> icon,
                       const base::string16& title,
                       const base::string16& subtitle,