[Dice] Refactor DiceAccountsMenu

The Dice accounts menu is migrated to
MenuItemViews to enable more customization.

Additional changes:
 - The font is set to match the avatar menu
   buttons font.
 - The additional spacing is calculated based on
   menu_config instead of setting it statically.

Mocks:
https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZWnUS9sdsb0Q/files/MCHtA7U1iMGr6z5qhd2cYMK0rnA0Z45fEIs

Change-Id: I85665ad62c885d5cdc3daa04228a9f9ec6452864
Reviewed-on: https://chromium-review.googlesource.com/1010722Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551647}

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

@@ -5,13 +5,13 @@
 #include "chrome/browser/ui/views/profiles/dice_accounts_menu.h"
 
 #include "base/strings/utf_string_conversions.h"
-#include "build/build_config.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
+#include "chrome/browser/ui/views/harmony/chrome_typography.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
-#include "ui/gfx/canvas.h"
-#include "ui/gfx/image/canvas_image_source.h"
+#include "ui/views/controls/menu/menu_config.h"
+#include "ui/views/layout/layout_provider.h"
 #include "ui/views/view.h"
 
 namespace {
@@ -24,17 +24,13 @@ constexpr int kUseAnotherAccountCmdId = std::numeric_limits<int>::max();
 // Anchor inset used to position the accounts menu.
 constexpr int kAnchorInset = 8;
 
-// TODO(tangltom): Calculate these values considering the existing menu config.
-constexpr int kVerticalImagePadding = 9;
-constexpr int kHorizontalImagePadding = 6;
+constexpr int kVerticalItemMargins = 12;
+constexpr int kHorizontalIconSpacing = 16;
 
-gfx::Image SizeAndCircleIcon(const gfx::Image& icon) {
+gfx::ImageSkia SizeAndCircleIcon(const gfx::Image& icon) {
   gfx::Image circled_icon = profiles::GetSizedAvatarIcon(
       icon, true, kAvatarIconSize, kAvatarIconSize, profiles::SHAPE_CIRCLE);
-
-  return gfx::Image(gfx::CanvasImageSource::CreatePadded(
-      *circled_icon.ToImageSkia(),
-      gfx::Insets(kVerticalImagePadding, kHorizontalImagePadding)));
+  return *circled_icon.ToImageSkia();
 }
 
 }  // namespace
@@ -42,35 +38,17 @@ gfx::Image SizeAndCircleIcon(const gfx::Image& icon) {
 DiceAccountsMenu::DiceAccountsMenu(const std::vector<AccountInfo>& accounts,
                                    const std::vector<gfx::Image>& icons,
                                    Callback account_selected_callback)
-    : menu_(this),
-      accounts_(accounts),
+    : accounts_(accounts),
+      icons_(icons),
       account_selected_callback_(std::move(account_selected_callback)) {
   DCHECK_EQ(accounts.size(), icons.size());
-  gfx::Image default_icon =
-      ui::ResourceBundle::GetSharedInstance().GetImageNamed(
-          profiles::GetPlaceholderAvatarIconResourceID());
-  // Add a menu item for each account.
-  menu_.AddSeparator(ui::SPACING_SEPARATOR);
-  for (size_t idx = 0; idx < accounts.size(); idx++) {
-    menu_.AddItem(idx, base::UTF8ToUTF16(accounts[idx].email));
-    menu_.SetIcon(
-        menu_.GetIndexOfCommandId(idx),
-        SizeAndCircleIcon(icons[idx].IsEmpty() ? default_icon : icons[idx]));
-  }
-  // Add the "Use another account" button at the bottom.
-  menu_.AddItem(
-      kUseAnotherAccountCmdId,
-      l10n_util::GetStringUTF16(IDS_PROFILES_DICE_USE_ANOTHER_ACCOUNT_BUTTON));
-  menu_.SetIcon(menu_.GetIndexOfCommandId(kUseAnotherAccountCmdId),
-                SizeAndCircleIcon(default_icon));
-  menu_.AddSeparator(ui::SPACING_SEPARATOR);
 }
 
 void DiceAccountsMenu::Show(views::View* anchor_view,
                             views::MenuButton* menu_button) {
   DCHECK(!runner_);
-  runner_ =
-      std::make_unique<views::MenuRunner>(&menu_, views::MenuRunner::COMBOBOX);
+  runner_ = std::make_unique<views::MenuRunner>(BuildMenu(),
+                                                views::MenuRunner::COMBOBOX);
   // Calculate custom anchor bounds to position the menu.
   // The menu is aligned along the right edge (left edge in RTL mode) of the
   // anchor, slightly shifted inside by |kAnchorInset| and overlapping
@@ -90,15 +68,38 @@ void DiceAccountsMenu::Show(views::View* anchor_view,
 
 DiceAccountsMenu::~DiceAccountsMenu() {}
 
-bool DiceAccountsMenu::IsCommandIdChecked(int command_id) const {
-  return false;
-}
-
-bool DiceAccountsMenu::IsCommandIdEnabled(int command_id) const {
-  return true;
+views::MenuItemView* DiceAccountsMenu::BuildMenu() {
+  views::MenuItemView* menu = new views::MenuItemView(this);
+  gfx::Image default_icon =
+      ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+          profiles::GetPlaceholderAvatarIconResourceID());
+  // Add spacing at top.
+  menu->AppendMenuItemImpl(
+      0, base::string16(), base::string16(), base::string16(), nullptr,
+      gfx::ImageSkia(), views::MenuItemView::SEPARATOR, ui::SPACING_SEPARATOR);
+  // Add a menu item for each account.
+  for (size_t idx = 0; idx < accounts_.size(); idx++) {
+    views::MenuItemView* item = menu->AppendMenuItemWithIcon(
+        idx, base::UTF8ToUTF16(accounts_[idx].email),
+        SizeAndCircleIcon(icons_[idx].IsEmpty() ? default_icon : icons_[idx]));
+    item->SetMargins(kVerticalItemMargins, kVerticalItemMargins);
+  }
+  // Add the "Use another account" button at the bottom.
+  views::MenuItemView* item = menu->AppendMenuItemWithIcon(
+      kUseAnotherAccountCmdId,
+      l10n_util::GetStringUTF16(IDS_PROFILES_DICE_USE_ANOTHER_ACCOUNT_BUTTON),
+      SizeAndCircleIcon(default_icon));
+  item->SetMargins(kVerticalItemMargins, kVerticalItemMargins);
+  // Add spacing at bottom.
+  menu->AppendMenuItemImpl(
+      0, base::string16(), base::string16(), base::string16(), nullptr,
+      gfx::ImageSkia(), views::MenuItemView::SEPARATOR, ui::SPACING_SEPARATOR);
+
+  menu->set_has_icons(true);
+  return menu;
 }
 
-void DiceAccountsMenu::ExecuteCommand(int id, int event_flags) {
+void DiceAccountsMenu::ExecuteCommand(int id) {
   DCHECK((0 <= id && static_cast<size_t>(id) < accounts_.size()) ||
          id == kUseAnotherAccountCmdId);
   base::Optional<AccountInfo> account;
@@ -106,3 +107,19 @@ void DiceAccountsMenu::ExecuteCommand(int id, int event_flags) {
     account = accounts_[id];
   std::move(account_selected_callback_).Run(account);
 }
+
+void DiceAccountsMenu::GetHorizontalIconMargins(int command_id,
+                                                int icon_size,
+                                                int* left_margin,
+                                                int* right_margin) const {
+  const views::MenuConfig& config = views::MenuConfig::instance();
+  *left_margin = kHorizontalIconSpacing - config.item_left_margin;
+  *right_margin = kHorizontalIconSpacing - config.icon_to_label_padding;
+}
+
+const gfx::FontList* DiceAccountsMenu::GetLabelFontList(int id) const {
+  const views::LayoutProvider* provider = views::LayoutProvider::Get();
+  // Match the font of the HoverButtons in the avatar bubble.
+  return &provider->GetTypographyProvider().GetFont(
+      views::style::CONTEXT_BUTTON, STYLE_SECONDARY);
+}

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

@@ -10,8 +10,9 @@
 #include "base/callback_forward.h"
 #include "base/optional.h"
 #include "components/signin/core/browser/account_info.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "ui/gfx/image/image.h"
+#include "ui/views/controls/menu/menu_delegate.h"
+#include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_runner.h"
 
 namespace views {
@@ -20,7 +21,7 @@ class View;
 
 // Menu allowing the user to select an account for enabling Sync when desktop
 // identity consistency (aka DICE) is enabled.
-class DiceAccountsMenu : public ui::SimpleMenuModel::Delegate {
+class DiceAccountsMenu : public views::MenuDelegate {
  public:
   // Callback to enable Sync for |account| if available. |account| corresponds
   // to the account that was selected or |nullopt| if the "Use another account"
@@ -42,15 +43,20 @@ class DiceAccountsMenu : public ui::SimpleMenuModel::Delegate {
   void Show(views::View* anchor_view, views::MenuButton* menu_button = nullptr);
 
  private:
-  // Overridden from ui::SimpleMenuModel::Delegate:
-  bool IsCommandIdChecked(int command_id) const override;
-  bool IsCommandIdEnabled(int command_id) const override;
-  void ExecuteCommand(int command_id, int event_flags) override;
+  views::MenuItemView* BuildMenu();
+
+  // Overridden from views::MenuDelegate:
+  void ExecuteCommand(int id) override;
+  void GetHorizontalIconMargins(int command_id,
+                                int icon_size,
+                                int* left_margin,
+                                int* right_margin) const override;
+  const gfx::FontList* GetLabelFontList(int id) const override;
 
-  ui::SimpleMenuModel menu_;
   std::unique_ptr<views::MenuRunner> runner_;
 
   std::vector<AccountInfo> accounts_;
+  std::vector<gfx::Image> icons_;
 
   Callback account_selected_callback_;