Parametrize the secondary icon for HoverButton

This CL changes a ctor for HoverButton to take an optional View object
for showing an arbitrary secondary icon, instead of taking a boolean
for whether to show an arrow icon.

The HoverButton will be used by the Cast dialog to list devices.

Bug: 826089
Change-Id: Iac36c2bc8a60666bc6aa035fbcbbedd0f59c66cf
Reviewed-on: https://chromium-review.googlesource.com/989023
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547575}

chrome/browser/ui/views/hover_button.cc

@@ -95,7 +95,7 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
                          std::unique_ptr<views::View> icon_view,
                          const base::string16& title,
                          const base::string16& subtitle,
-                         bool show_submenu_arrow)
+                         std::unique_ptr<views::View> secondary_icon_view)
     : HoverButton(button_listener, base::string16()) {
   label()->SetHandlesTooltips(false);
   ChromeLayoutProvider* layout_provider = ChromeLayoutProvider::Get();
@@ -174,6 +174,19 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
   title_wrapper->set_can_process_events_within_subtree(false);
   grid_layout->AddView(title_wrapper);
 
+  if (secondary_icon_view) {
+    columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
+                       kFixed, views::GridLayout::USE_PREF, 0, 0);
+    // Make sure hovering over |secondary_icon_view| also hovers the
+    // |HoverButton|.
+    secondary_icon_view->set_can_process_events_within_subtree(false);
+    // |secondary_icon_view| needs a layer otherwise it's obscured by the layer
+    // used in drawing ink drops.
+    secondary_icon_view->SetPaintToLayer();
+    secondary_icon_view->layer()->SetFillsBoundsOpaquely(false);
+    grid_layout->AddView(secondary_icon_view.release(), 1, num_labels);
+  }
+
   if (!subtitle.empty()) {
     grid_layout->StartRow(0, kColumnSetId, row_height);
     subtitle_ = new views::Label(subtitle, views::style::CONTEXT_BUTTON,
@@ -184,23 +197,6 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
     grid_layout->AddView(subtitle_);
   }
 
-  if (show_submenu_arrow) {
-    constexpr int kSubmenuArrowSize = 12;
-    columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
-                       kFixed, views::GridLayout::USE_PREF, 0, 0);
-    views::ImageView* arrow = new views::ImageView();
-    arrow->SetImage(gfx::CreateVectorIcon(
-        kUserMenuRightArrowIcon, kSubmenuArrowSize, gfx::kChromeIconGrey));
-    // Make sure hovering over |arrow| also hovers the |HoverButton|.
-    arrow->set_can_process_events_within_subtree(false);
-    // |arrow| needs a layer otherwise it's obscured by the layer used in
-    // drawing ink drops.
-    arrow->SetPaintToLayer();
-    arrow->layer()->SetFillsBoundsOpaquely(false);
-    // Make sure that the arrow is flipped in RTL mode.
-    arrow->EnableCanvasFlippingForRTLUI(true);
-    grid_layout->AddView(arrow, 1, num_labels);
-  }
   SetTooltipAndAccessibleName(this, title_, subtitle_, GetLocalBounds(),
                               taken_width_, auto_compute_tooltip_);
 

chrome/browser/ui/views/hover_button.h

@@ -38,13 +38,13 @@ class HoverButton : public views::MenuButton, public views::MenuButtonListener {
 
   // Creates a HoverButton with custom subviews. |icon_view| replaces the
   // LabelButton icon, and titles appear on separate rows. An empty |subtitle|
-  // will vertically center |title|. If |show_submenu_arrow| is true, an arrow
-  // is shown, analogous to menu items with submenus.
+  // will vertically center |title|. |secondary_icon_view|, when set, is shown
+  // on the opposite side of the button from |icon_view|.
   HoverButton(views::ButtonListener* button_listener,
               std::unique_ptr<views::View> icon_view,
               const base::string16& title,
               const base::string16& subtitle,
-              bool show_submenu_arrow = false);
+              std::unique_ptr<views::View> secondary_icon_view = nullptr);
 
   ~HoverButton() override;
 

chrome/browser/ui/views/hover_button_unittest.cc

@@ -162,3 +162,18 @@ TEST_F(HoverButtonTest, SetStyleAndSubtitleElideBehavior) {
   button.SetStyle(HoverButton::STYLE_PROMINENT);
   button.SetSubtitleElideBehavior(gfx::ELIDE_EMAIL);
 }
+
+// Tests that a button with a subtitle and icons can be instantiated without a
+// crash.
+TEST_F(HoverButtonTest, CreateButtonWithSubtitleAndIcons) {
+  std::unique_ptr<views::View> primary_icon = CreateIcon();
+  views::View* primary_icon_raw = primary_icon.get();
+  std::unique_ptr<views::View> secondary_icon = CreateIcon();
+  views::View* secondary_icon_raw = secondary_icon.get();
+
+  HoverButton button(nullptr, std::move(primary_icon),
+                     base::ASCIIToUTF16("Title"),
+                     base::ASCIIToUTF16("Subtitle"), std::move(secondary_icon));
+  EXPECT_TRUE(button.Contains(primary_icon_raw));
+  EXPECT_TRUE(button.Contains(secondary_icon_raw));
+}

chrome/browser/ui/views/profiles/profile_chooser_view.cc

@@ -1133,12 +1133,20 @@ views::View* ProfileChooserView::CreateDiceSigninView() {
   // The accounts submenu is only needed when there are additional accounts to
   // list, i.e. when there is more than 1 account (the first account has it's
   // own button).
-  const bool show_submenu_arrow = dice_sync_promo_accounts_.size() > 1;
-  sync_to_another_account_button_ =
-      new HoverButton(this, std::move(switch_account_icon_view),
-                      l10n_util::GetStringUTF16(
-                          IDS_PROFILES_DICE_SIGNIN_WITH_ANOTHER_ACCOUNT_BUTTON),
-                      base::string16() /* subtitle */, show_submenu_arrow);
+  std::unique_ptr<views::ImageView> submenu_arrow_icon_view;
+  if (dice_sync_promo_accounts_.size() > 1) {
+    constexpr int kSubmenuArrowSize = 12;
+    submenu_arrow_icon_view = std::make_unique<views::ImageView>();
+    submenu_arrow_icon_view->SetImage(gfx::CreateVectorIcon(
+        kUserMenuRightArrowIcon, kSubmenuArrowSize, gfx::kChromeIconGrey));
+    // Make sure that the arrow is flipped in RTL mode.
+    submenu_arrow_icon_view->EnableCanvasFlippingForRTLUI(true);
+  }
+  sync_to_another_account_button_ = new HoverButton(
+      this, std::move(switch_account_icon_view),
+      l10n_util::GetStringUTF16(
+          IDS_PROFILES_DICE_SIGNIN_WITH_ANOTHER_ACCOUNT_BUTTON),
+      base::string16() /* subtitle */, std::move(submenu_arrow_icon_view));
   view->AddChildView(sync_to_another_account_button_);
   return view;
 }