Make HoverButton a subclass of MenuButton

To have the correct behavior when a submenu
is opened via a HoverButton, the HoverButton
class is transformed into a subclass of
MenuButton.

HoverButton can now function as both, a normal
button and a menu button.

Visible changes in the user menu:
 - When the submenu is opened, the button stays
 highlighted.
 - When the submenu closes, the button loses its
 highlighting.

Video from before:
https://drive.google.com/file/d/1QONj3SZv-zAwQ2r7WqF4Ww7UKo0wVcJ2/view?usp=sharing
Video from after:
https://drive.google.com/file/d/1dwh_68A0cjiK2kiPnHwCvSdQX6RByVY5/view?usp=sharing

Bug: 814809
Change-Id: I6401cb39d20fca9c3590a67bd0fbeff5a2043b83
Reviewed-on: https://chromium-review.googlesource.com/953623Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543061}

chrome/browser/ui/views/hover_button.cc

@@ -67,9 +67,10 @@ void SetTooltipAndAccessibleName(views::Button* parent,
 
 HoverButton::HoverButton(views::ButtonListener* button_listener,
                          const base::string16& text)
-    : views::LabelButton(button_listener, text),
+    : views::MenuButton(text, this, false),
       title_(nullptr),
-      subtitle_(nullptr) {
+      subtitle_(nullptr),
+      listener_(button_listener) {
   SetFocusBehavior(FocusBehavior::ALWAYS);
   SetFocusPainter(nullptr);
 
@@ -147,7 +148,7 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
 
   // Make sure hovering over the icon also hovers the |HoverButton|.
   icon_view->set_can_process_events_within_subtree(false);
-  // Don't cover |icon_view| when the ink drops are being painted. |LabelButton|
+  // Don't cover |icon_view| when the ink drops are being painted. |MenuButton|
   // already does this with its own image.
   icon_view->SetPaintToLayer();
   icon_view->layer()->SetFillsBoundsOpaquely(false);
@@ -209,12 +210,16 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
 HoverButton::~HoverButton() {}
 
 void HoverButton::SetBorder(std::unique_ptr<views::Border> b) {
-  LabelButton::SetBorder(std::move(b));
+  MenuButton::SetBorder(std::move(b));
   // Make sure the minimum size is correct according to the layout (if any).
   if (GetLayoutManager())
     SetMinSize(GetLayoutManager()->GetPreferredSize(this));
 }
 
+void HoverButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  Button::GetAccessibleNodeData(node_data);
+}
+
 void HoverButton::SetSubtitleElideBehavior(gfx::ElideBehavior elide_behavior) {
   DCHECK(subtitle_);
   if (!subtitle_->text().empty())
@@ -245,7 +250,7 @@ views::Button::KeyClickAction HoverButton::GetKeyClickActionForEvent(
     // |PlatformStyle::kReturnClicksFocusedControl|.
     return CLICK_ON_KEY_PRESS;
   }
-  return LabelButton::GetKeyClickActionForEvent(event);
+  return MenuButton::GetKeyClickActionForEvent(event);
 }
 
 void HoverButton::SetHighlightingView(views::View* highlighting_view) {
@@ -253,11 +258,11 @@ void HoverButton::SetHighlightingView(views::View* highlighting_view) {
 }
 
 void HoverButton::StateChanged(ButtonState old_state) {
-  LabelButton::StateChanged(old_state);
+  MenuButton::StateChanged(old_state);
 
   // |HoverButtons| are designed for use in a list, so ensure only one button
   // can have a hover background at any time by requesting focus on hover.
-  if (state() == STATE_HOVERED || state() == STATE_PRESSED) {
+  if (state() == STATE_HOVERED && old_state != STATE_PRESSED) {
     RequestFocus();
   } else if (state() == STATE_NORMAL && HasFocus()) {
     GetFocusManager()->SetFocusedView(nullptr);
@@ -265,7 +270,7 @@ void HoverButton::StateChanged(ButtonState old_state) {
 }
 
 bool HoverButton::ShouldUseFloodFillInkDrop() const {
-  return views::LabelButton::ShouldUseFloodFillInkDrop() || title_ != nullptr;
+  return views::MenuButton::ShouldUseFloodFillInkDrop() || title_ != nullptr;
 }
 
 SkColor HoverButton::GetInkDropBaseColor() const {
@@ -299,7 +304,7 @@ std::unique_ptr<views::InkDropHighlight> HoverButton::CreateInkDropHighlight()
 }
 
 void HoverButton::Layout() {
-  LabelButton::Layout();
+  MenuButton::Layout();
 
   // Vertically center |title_| manually since it doesn't have a LayoutManager.
   if (title_) {
@@ -357,3 +362,10 @@ void HoverButton::SetTitleTextStyle(views::style::TextStyle text_style,
 void HoverButton::SetSubtitleColor(SkColor color) {
   subtitle_->SetEnabledColor(color);
 }
+
+void HoverButton::OnMenuButtonClicked(MenuButton* source,
+                                      const gfx::Point& point,
+                                      const ui::Event* event) {
+  if (listener_)
+    listener_->ButtonPressed(source, *event);
+}

chrome/browser/ui/views/hover_button.h

@@ -6,7 +6,8 @@
 #define CHROME_BROWSER_UI_VIEWS_HOVER_BUTTON_H_
 
 #include "base/strings/string16.h"
-#include "ui/views/controls/button/label_button.h"
+#include "ui/views/controls/button/menu_button.h"
+#include "ui/views/controls/button/menu_button_listener.h"
 
 namespace gfx {
 enum ElideBehavior;
@@ -22,7 +23,7 @@ class View;
 
 // A button taking the full width of its parent that shows a background color
 // when hovered over.
-class HoverButton : public views::LabelButton {
+class HoverButton : public views::MenuButton, public views::MenuButtonListener {
  public:
   enum Style { STYLE_PROMINENT, STYLE_ERROR };
 
@@ -47,8 +48,9 @@ class HoverButton : public views::LabelButton {
 
   ~HoverButton() override;
 
-  // views::LabelButton:
+  // views::MenuButton:
   void SetBorder(std::unique_ptr<views::Border> b) override;
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
   // Updates the title text, and applies the secondary style to the text
   // specified by |range|. If |range| is invalid, no style is applied. This
@@ -82,7 +84,12 @@ class HoverButton : public views::LabelButton {
   void SetHighlightingView(views::View* highlighting_view);
 
  protected:
-  // views::LabelButton:
+  // views::MenuButtonListener:
+  void OnMenuButtonClicked(MenuButton* source,
+                           const gfx::Point& point,
+                           const ui::Event* event) override;
+
+  // views::MenuButton:
   KeyClickAction GetKeyClickActionForEvent(const ui::KeyEvent& event) override;
   void StateChanged(ButtonState old_state) override;
   bool ShouldUseFloodFillInkDrop() const override;
@@ -113,6 +120,9 @@ class HoverButton : public views::LabelButton {
   // View that gets highlighted when this button is hovered.
   views::View* highlighting_view_ = this;
 
+  // Listener to be called when button is clicked.
+  views::ButtonListener* listener_;
+
   DISALLOW_COPY_AND_ASSIGN(HoverButton);
 };
 

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

@@ -86,7 +86,8 @@ DiceAccountsMenu::DiceAccountsMenu(const std::vector<AccountInfo>& accounts,
   menu_.AddSeparator(ui::SPACING_SEPARATOR);
 }
 
-void DiceAccountsMenu::Show(views::View* anchor_view) {
+void DiceAccountsMenu::Show(views::View* anchor_view,
+                            views::MenuButton* menu_button) {
   DCHECK(!runner_);
   runner_ =
       std::make_unique<views::MenuRunner>(&menu_, views::MenuRunner::COMBOBOX);
@@ -112,7 +113,7 @@ void DiceAccountsMenu::Show(views::View* anchor_view) {
   else
     anchor_bounds.Inset(anchor_bounds.width(), 0, 0, 0);
 
-  runner_->RunMenuAt(anchor_view->GetWidget(), nullptr, anchor_bounds,
+  runner_->RunMenuAt(anchor_view->GetWidget(), menu_button, anchor_bounds,
                      views::MENU_ANCHOR_TOPRIGHT, ui::MENU_SOURCE_MOUSE);
 }
 

chrome/browser/ui/views/profiles/dice_accounts_menu.h

@@ -37,9 +37,9 @@ class DiceAccountsMenu : public ui::SimpleMenuModel::Delegate {
                    Callback account_selected_callback);
   ~DiceAccountsMenu() override;
 
-  // Shows the accounts menu below |anchor_view|. This method can only be called
-  // once.
-  void Show(views::View* anchor_view);
+  // Shows the accounts menu below |anchor_view| and locks |menu_button| if
+  // given. This method can only be called once.
+  void Show(views::View* anchor_view, views::MenuButton* menu_button = nullptr);
 
  private:
   // Overridden from ui::SimpleMenuModel::Delegate:

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

@@ -693,7 +693,7 @@ void ProfileChooserView::ButtonPressed(views::Button* sender,
         accounts, GetImagesForAccounts(accounts, browser_->profile()),
         base::BindOnce(&ProfileChooserView::EnableSync,
                        base::Unretained(this)));
-    dice_accounts_menu_->Show(sender);
+    dice_accounts_menu_->Show(sender, sync_to_another_account_button_);
   } else {
     // Either one of the "other profiles", or one of the profile accounts
     // buttons was pressed.

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

@@ -36,6 +36,7 @@ class LabelButton;
 
 class Browser;
 class DiceSigninButtonView;
+class HoverButton;
 
 // This bubble view is displayed when the user clicks on the avatar button.
 // It displays a list of profiles and allows users to switch between profiles.
@@ -201,7 +202,7 @@ class ProfileChooserView : public content::WebContentsDelegate,
   views::Link* manage_accounts_link_;
   views::LabelButton* manage_accounts_button_;
   views::LabelButton* signin_current_profile_button_;
-  views::LabelButton* sync_to_another_account_button_;
+  HoverButton* sync_to_another_account_button_;
   views::LabelButton* signin_with_gaia_account_button_;
 
   // For material design user menu, the active profile card owns the profile

chrome/browser/ui/views/sync/dice_bubble_sync_promo_view.cc

@@ -92,7 +92,8 @@ void DiceBubbleSyncPromoView::ButtonPressed(views::Button* sender,
         accounts_for_submenu_, images_for_submenu_,
         base::BindOnce(&DiceBubbleSyncPromoView::EnableSync,
                        base::Unretained(this)));
-    dice_accounts_menu_->Show(signin_button_view_);
+    dice_accounts_menu_->Show(signin_button_view_,
+                              signin_button_view_->drop_down_arrow());
     return;
   }
 

chrome/browser/ui/views/sync/dice_signin_button_view.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
+#include "chrome/browser/ui/views/hover_button.h"
 #include "components/signin/core/browser/account_info.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/view.h"
@@ -37,7 +38,7 @@ class DiceSigninButtonView : public views::View {
   ~DiceSigninButtonView() override;
 
   views::LabelButton* signin_button() const { return signin_button_; }
-  views::LabelButton* drop_down_arrow() const { return arrow_; }
+  HoverButton* drop_down_arrow() const { return arrow_; }
   base::Optional<AccountInfo> account() const { return account_; }
 
  private:
@@ -46,7 +47,7 @@ class DiceSigninButtonView : public views::View {
   void Layout() override;
 
   views::LabelButton* signin_button_ = nullptr;
-  views::LabelButton* arrow_ = nullptr;
+  HoverButton* arrow_ = nullptr;
 
   const base::Optional<AccountInfo> account_;