Removed remaining use of GetInstanceForNativeUi() from profile Menu.

Removed remaining use of global NativeTheme accessor from profile
menu and added color update capability on theme change.

Fixed spacing on HoverButtons taking image views for consistency with
other constructors.

Bug: None
Change-Id: I717a45fb068a3bd35b992797e38e729da1cd3521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2072998
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745058}

chrome/browser/ui/views/hover_button.cc

@@ -84,7 +84,8 @@ HoverButton::HoverButton(views::ButtonListener* button_listener,
   SetFocusBehavior(FocusBehavior::ALWAYS);
 
   const int vert_spacing = ChromeLayoutProvider::Get()->GetDistanceMetric(
-      DISTANCE_CONTROL_LIST_VERTICAL);
+                               DISTANCE_CONTROL_LIST_VERTICAL) /
+                           2;
   SetBorder(CreateBorderWithVerticalSpacing(vert_spacing));
 
   SetInkDropMode(InkDropMode::ON);

chrome/browser/ui/views/hover_button.h

@@ -55,7 +55,7 @@ class HoverButton : public views::LabelButton {
   HoverButton(views::ButtonListener* button_listener,
               std::unique_ptr<views::View> icon_view,
               const base::string16& title,
-              const base::string16& subtitle,
+              const base::string16& subtitle = base::string16(),
               std::unique_ptr<views::View> secondary_view = nullptr,
               bool resize_row_for_secondary_view = true,
               bool secondary_view_can_process_events = false);

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

@@ -55,10 +55,10 @@ void IncognitoMenuView::BuildMenu() {
                                              incognito_window_count)
           : base::string16());
   AddFeatureButton(
-      ImageForMenu(kCloseAllIcon),
       l10n_util::GetStringUTF16(IDS_INCOGNITO_PROFILE_MENU_CLOSE_BUTTON),
       base::BindRepeating(&IncognitoMenuView::OnExitButtonClicked,
-                          base::Unretained(this)));
+                          base::Unretained(this)),
+      kCloseAllIcon);
 }
 
 base::string16 IncognitoMenuView::GetAccessibleWindowTitle() const {

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

@@ -65,8 +65,6 @@ namespace {
 
 // Helpers --------------------------------------------------------------------
 
-constexpr float kShortcutIconToImageRatio = 9.0 / 16.0;
-
 ProfileAttributesEntry* GetProfileAttributesEntry(Profile* profile) {
   ProfileAttributesEntry* entry;
   CHECK(g_browser_process->profile_manager()
@@ -410,19 +408,18 @@ void ProfileMenuView::BuildGuestIdentity() {
 
 void ProfileMenuView::BuildAutofillButtons() {
   AddShortcutFeatureButton(
-      ImageForMenu(kKeyIcon, kShortcutIconToImageRatio),
-      l10n_util::GetStringUTF16(IDS_PROFILES_PASSWORDS_LINK),
+      kKeyIcon, l10n_util::GetStringUTF16(IDS_PROFILES_PASSWORDS_LINK),
       base::BindRepeating(&ProfileMenuView::OnPasswordsButtonClicked,
                           base::Unretained(this)));
 
   AddShortcutFeatureButton(
-      ImageForMenu(kCreditCardIcon, kShortcutIconToImageRatio),
+      kCreditCardIcon,
       l10n_util::GetStringUTF16(IDS_PROFILES_CREDIT_CARDS_LINK),
       base::BindRepeating(&ProfileMenuView::OnCreditCardsButtonClicked,
                           base::Unretained(this)));
 
   AddShortcutFeatureButton(
-      ImageForMenu(vector_icons::kLocationOnIcon, kShortcutIconToImageRatio),
+      vector_icons::kLocationOnIcon,
       l10n_util::GetStringUTF16(IDS_PROFILES_ADDRESSES_LINK),
       base::BindRepeating(&ProfileMenuView::OnAddressesButtonClicked,
                           base::Unretained(this)));
@@ -513,37 +510,42 @@ void ProfileMenuView::BuildFeatureButtons() {
       !is_guest && identity_manager->HasPrimaryAccount();
 
   if (has_unconsented_account && !IsSyncPaused(profile)) {
-    AddFeatureButton(
 #if BUILDFLAG(GOOGLE_CHROME_BRANDING)
-        // The Google G icon needs to be shrunk, so it won't look too big
-        // compared to the other icons.
-        ImageForMenu(kGoogleGLogoIcon, /*icon_to_image_ratio=*/0.75),
+    // The Google G icon needs to be shrunk, so it won't look too big compared
+    // to the other icons.
+    AddFeatureButton(
+        l10n_util::GetStringUTF16(IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT),
+        base::BindRepeating(
+            &ProfileMenuView::OnManageGoogleAccountButtonClicked,
+            base::Unretained(this)),
+        kGoogleGLogoIcon,
+        /*icon_to_image_ratio=*/0.75f);
 #else
-        gfx::ImageSkia(),
-#endif
+    AddFeatureButton(
         l10n_util::GetStringUTF16(IDS_SETTINGS_MANAGE_GOOGLE_ACCOUNT),
         base::BindRepeating(
             &ProfileMenuView::OnManageGoogleAccountButtonClicked,
             base::Unretained(this)));
+#endif
   }
 
   int window_count = CountBrowsersFor(profile);
   if (window_count > 1) {
     AddFeatureButton(
-        ImageForMenu(vector_icons::kCloseIcon),
         l10n_util::GetPluralStringFUTF16(IDS_PROFILES_CLOSE_X_WINDOWS_BUTTON,
                                          window_count),
         base::BindRepeating(&ProfileMenuView::OnExitProfileButtonClicked,
-                            base::Unretained(this)));
+                            base::Unretained(this)),
+        vector_icons::kCloseIcon);
   }
 
   // The sign-out button is always at the bottom.
   if (has_unconsented_account && !has_primary_account) {
     AddFeatureButton(
-        ImageForMenu(kSignOutIcon),
         l10n_util::GetStringUTF16(IDS_SCREEN_LOCK_SIGN_OUT),
         base::BindRepeating(&ProfileMenuView::OnSignoutButtonClicked,
-                            base::Unretained(this)));
+                            base::Unretained(this)),
+        kSignOutIcon);
   }
 }
 
@@ -585,7 +587,7 @@ void ProfileMenuView::BuildSelectableProfiles() {
 
 void ProfileMenuView::BuildProfileManagementFeatureButtons() {
   AddProfileManagementShortcutFeatureButton(
-      ImageForMenu(vector_icons::kSettingsIcon, kShortcutIconToImageRatio),
+      vector_icons::kSettingsIcon,
       l10n_util::GetStringUTF16(IDS_PROFILES_MANAGE_USERS_BUTTON),
       base::BindRepeating(&ProfileMenuView::OnManageProfilesButtonClicked,
                           base::Unretained(this)));
@@ -594,8 +596,7 @@ void ProfileMenuView::BuildProfileManagementFeatureButtons() {
   DCHECK(service);
   if (service->GetBoolean(prefs::kBrowserAddPersonEnabled)) {
     AddProfileManagementFeatureButton(
-        ImageForMenu(kAddIcon, /*icon_to_image_ratio=*/0.75),
-        l10n_util::GetStringUTF16(IDS_ADD),
+        kAddIcon, l10n_util::GetStringUTF16(IDS_ADD),
         base::BindRepeating(&ProfileMenuView::OnAddNewProfileButtonClicked,
                             base::Unretained(this)));
   }

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

@@ -28,6 +28,7 @@
 #include "ui/display/screen.h"
 #include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image_skia_operations.h"
+#include "ui/gfx/vector_icon_types.h"
 #include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/button/md_text_button.h"
@@ -64,16 +65,6 @@ constexpr int kMinimumScrollableContentHeight = 40;
 // the menu items.
 constexpr int kMenuEdgeMargin = 16;
 
-SkColor GetDefaultIconColor() {
-  return ui::NativeTheme::GetInstanceForNativeUi()->GetSystemColor(
-      ui::NativeTheme::kColorId_DefaultIconColor);
-}
-
-SkColor GetDefaultSeparatorColor() {
-  return ui::NativeTheme::GetInstanceForNativeUi()->GetSystemColor(
-      ui::NativeTheme::kColorId_MenuSeparatorColor);
-}
-
 gfx::ImageSkia SizeImage(const gfx::ImageSkia& image, int size) {
   return gfx::ImageSkiaOperations::CreateResizedImage(
       image, skia::ImageOperations::RESIZE_BEST, gfx::Size(size, size));
@@ -135,30 +126,98 @@ std::unique_ptr<views::BoxLayout> CreateBoxLayout(
   return layout;
 }
 
-std::unique_ptr<views::Button> CreateCircularImageButton(
-    views::ButtonListener* listener,
-    const gfx::ImageSkia& image,
-    const base::string16& text,
-    bool show_border = false) {
-  constexpr int kImageSize = 28;
-  const int kBorderThickness = show_border ? 1 : 0;
-  const SkScalar kButtonRadius = (kImageSize + 2 * kBorderThickness) / 2.0;
-
-  auto button = std::make_unique<views::ImageButton>(listener);
-  button->SetImage(views::Button::STATE_NORMAL, SizeImage(image, kImageSize));
-  button->SetTooltipText(text);
-  button->SetInkDropMode(views::Button::InkDropMode::ON);
-  button->SetFocusForPlatform();
-  button->set_ink_drop_base_color(GetDefaultIconColor());
-  if (show_border) {
-    button->SetBorder(views::CreateRoundedRectBorder(
-        kBorderThickness, kButtonRadius, GetDefaultSeparatorColor()));
+const gfx::ImageSkia ImageForMenu(const gfx::VectorIcon& icon,
+                                  float icon_to_image_ratio,
+                                  SkColor color) {
+  const int padding =
+      static_cast<int>(kMaxImageSize * (1.0f - icon_to_image_ratio) / 2.0f);
+
+  gfx::ImageSkia sized_icon =
+      gfx::CreateVectorIcon(icon, kMaxImageSize - 2 * padding, color);
+  return gfx::CanvasImageSource::CreatePadded(sized_icon, gfx::Insets(padding));
+}
+
+class CircularImageButton : public views::ImageButton {
+ public:
+  CircularImageButton(views::ButtonListener* listener,
+                      const gfx::VectorIcon& icon,
+                      const base::string16& text,
+                      bool show_border = false)
+      : ImageButton(listener), icon_(icon), show_border_(show_border) {
+    SetTooltipText(text);
+    SetInkDropMode(views::Button::InkDropMode::ON);
+    SetFocusForPlatform();
+
+    InstallCircleHighlightPathGenerator(this);
+  }
+
+  // views::ImageButton:
+  void OnThemeChanged() override {
+    constexpr int kImageSize = 28;
+    constexpr float kShortcutIconToImageRatio = 9.0f / 16.0f;
+    const int kBorderThickness = show_border_ ? 1 : 0;
+    const SkScalar kButtonRadius = (kImageSize + 2 * kBorderThickness) / 2.0f;
+    const SkColor icon_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_DefaultIconColor);
+
+    gfx::ImageSkia image =
+        ImageForMenu(icon_, kShortcutIconToImageRatio, icon_color);
+    SetImage(views::Button::STATE_NORMAL, SizeImage(image, kImageSize));
+    set_ink_drop_base_color(icon_color);
+
+    if (show_border_) {
+      const SkColor separator_color = GetNativeTheme()->GetSystemColor(
+          ui::NativeTheme::kColorId_MenuSeparatorColor);
+      SetBorder(views::CreateRoundedRectBorder(kBorderThickness, kButtonRadius,
+                                               separator_color));
+    }
   }
 
-  InstallCircleHighlightPathGenerator(button.get());
+ private:
+  const gfx::VectorIcon& icon_;
+  bool show_border_;
+};
 
-  return button;
-}
+class FeatureButtonIconView : public views::ImageView {
+ public:
+  FeatureButtonIconView(const gfx::VectorIcon& icon, float icon_to_image_ratio)
+      : icon_(icon), icon_to_image_ratio_(icon_to_image_ratio) {}
+  ~FeatureButtonIconView() override = default;
+
+  // views::ImageView:
+  void OnThemeChanged() override {
+    constexpr int kIconSize = 16;
+    const SkColor icon_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_DefaultIconColor);
+    gfx::ImageSkia image =
+        ImageForMenu(icon_, icon_to_image_ratio_, icon_color);
+    SetImage(SizeImage(ColorImage(image, icon_color), kIconSize));
+  }
+
+ private:
+  const gfx::VectorIcon& icon_;
+  const float icon_to_image_ratio_;
+};
+
+class ProfileManagementIconView : public views::ImageView {
+ public:
+  explicit ProfileManagementIconView(const gfx::VectorIcon& icon)
+      : icon_(icon) {}
+  ~ProfileManagementIconView() override = default;
+
+  // views::ImageView:
+  void OnThemeChanged() override {
+    constexpr float kIconToImageRatio = 0.75f;
+    constexpr int kIconSize = 20;
+    const SkColor icon_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_DefaultIconColor);
+    gfx::ImageSkia image = ImageForMenu(icon_, kIconToImageRatio, icon_color);
+    SetImage(SizeImage(image, kIconSize));
+  }
+
+ private:
+  const gfx::VectorIcon& icon_;
+};
 
 // AvatarImageView is used to ensure avatar adornments are kept in sync with
 // current theme colors.
@@ -433,7 +492,7 @@ void ProfileMenuViewBase::SetSyncInfo(
 }
 
 void ProfileMenuViewBase::AddShortcutFeatureButton(
-    const gfx::ImageSkia& icon,
+    const gfx::VectorIcon& icon,
     const base::string16& text,
     base::RepeatingClosure action) {
   const int kButtonSpacing = ChromeLayoutProvider::Get()->GetDistanceMetric(
@@ -452,26 +511,32 @@ void ProfileMenuViewBase::AddShortcutFeatureButton(
   }
 
   views::Button* button = shortcut_features_container_->AddChildView(
-      CreateCircularImageButton(this, icon, text, /*show_border=*/true));
+      std::make_unique<CircularImageButton>(this, icon, text,
+                                            /*show_border=*/true));
 
   RegisterClickAction(button, std::move(action));
 }
 
-void ProfileMenuViewBase::AddFeatureButton(const gfx::ImageSkia& icon,
-                                           const base::string16& text,
-                                           base::RepeatingClosure action) {
-  constexpr int kIconSize = 16;
-
+void ProfileMenuViewBase::AddFeatureButton(const base::string16& text,
+                                           base::RepeatingClosure action,
+                                           const gfx::VectorIcon& icon,
+                                           float icon_to_image_ratio) {
   // Initialize layout if this is the first time a button is added.
   if (!features_container_->GetLayoutManager()) {
     features_container_->SetLayoutManager(std::make_unique<views::BoxLayout>(
         views::BoxLayout::Orientation::kVertical));
   }
 
-  views::Button* button =
-      features_container_->AddChildView(std::make_unique<HoverButton>(
-          this, SizeImage(ColorImage(icon, GetDefaultIconColor()), kIconSize),
-          text));
+  views::View* button;
+  if (&icon == &gfx::kNoneIcon) {
+    button = features_container_->AddChildView(
+        std::make_unique<HoverButton>(this, text));
+  } else {
+    auto icon_view =
+        std::make_unique<FeatureButtonIconView>(icon, icon_to_image_ratio);
+    button = features_container_->AddChildView(
+        std::make_unique<HoverButton>(this, std::move(icon_view), text));
+  }
 
   RegisterClickAction(button, std::move(action));
 }
@@ -508,7 +573,7 @@ void ProfileMenuViewBase::AddSelectableProfile(const gfx::ImageSkia& image,
                                                const base::string16& name,
                                                bool is_guest,
                                                base::RepeatingClosure action) {
-  constexpr int kImageSize = 22;
+  constexpr int kImageSize = 20;
 
   // Initialize layout if this is the first time a button is added.
   if (!selectable_profiles_container_->GetLayoutManager()) {
@@ -528,7 +593,7 @@ void ProfileMenuViewBase::AddSelectableProfile(const gfx::ImageSkia& image,
 }
 
 void ProfileMenuViewBase::AddProfileManagementShortcutFeatureButton(
-    const gfx::ImageSkia& icon,
+    const gfx::VectorIcon& icon,
     const base::string16& text,
     base::RepeatingClosure action) {
   // Initialize layout if this is the first time a button is added.
@@ -541,17 +606,15 @@ void ProfileMenuViewBase::AddProfileManagementShortcutFeatureButton(
 
   views::Button* button =
       profile_mgmt_shortcut_features_container_->AddChildView(
-          CreateCircularImageButton(this, icon, text));
+          std::make_unique<CircularImageButton>(this, icon, text));
 
   RegisterClickAction(button, std::move(action));
 }
 
 void ProfileMenuViewBase::AddProfileManagementFeatureButton(
-    const gfx::ImageSkia& icon,
+    const gfx::VectorIcon& icon,
     const base::string16& text,
     base::RepeatingClosure action) {
-  constexpr int kIconSize = 22;
-
   // Initialize layout if this is the first time a button is added.
   if (!profile_mgmt_features_container_->GetLayoutManager()) {
     profile_mgmt_features_container_->SetLayoutManager(
@@ -559,23 +622,13 @@ void ProfileMenuViewBase::AddProfileManagementFeatureButton(
             views::BoxLayout::Orientation::kVertical));
   }
 
+  auto icon_button = std::make_unique<ProfileManagementIconView>(icon);
   views::Button* button = profile_mgmt_features_container_->AddChildView(
-      std::make_unique<HoverButton>(this, SizeImage(icon, kIconSize), text));
+      std::make_unique<HoverButton>(this, std::move(icon_button), text));
 
   RegisterClickAction(button, std::move(action));
 }
 
-gfx::ImageSkia ProfileMenuViewBase::ImageForMenu(
-    const gfx::VectorIcon& icon,
-    float icon_to_image_ratio) const {
-  const int padding =
-      static_cast<int>(kMaxImageSize * (1.0 - icon_to_image_ratio) / 2.0);
-
-  auto sized_icon = gfx::CreateVectorIcon(icon, kMaxImageSize - 2 * padding,
-                                          GetDefaultIconColor());
-  return gfx::CanvasImageSource::CreatePadded(sized_icon, gfx::Insets(padding));
-}
-
 gfx::ImageSkia ProfileMenuViewBase::ColoredImageForMenu(
     const gfx::VectorIcon& icon,
     SkColor color) const {
@@ -714,7 +767,7 @@ void ProfileMenuViewBase::Reset() {
   columns->AddColumn(views::GridLayout::FILL, views::GridLayout::FILL,
                      views::GridLayout::kFixedSize, views::GridLayout::FIXED,
                      kMenuWidth, kMenuWidth);
-  layout->StartRow(1.0, 0);
+  layout->StartRow(1.0f, 0);
   layout->AddView(std::move(scroll_view));
 }
 

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

@@ -107,29 +107,25 @@ class ProfileMenuViewBase : public content::WebContentsDelegate,
                    SyncInfoContainerBackgroundState background_state,
                    base::RepeatingClosure action,
                    bool show_badge = true);
-  void AddShortcutFeatureButton(const gfx::ImageSkia& icon,
+  void AddShortcutFeatureButton(const gfx::VectorIcon& icon,
                                 const base::string16& text,
                                 base::RepeatingClosure action);
-  void AddFeatureButton(const gfx::ImageSkia& icon,
-                        const base::string16& text,
-                        base::RepeatingClosure action);
+  void AddFeatureButton(const base::string16& text,
+                        base::RepeatingClosure action,
+                        const gfx::VectorIcon& icon = gfx::kNoneIcon,
+                        float icon_to_image_ratio = 1.0f);
   void SetProfileManagementHeading(const base::string16& heading);
   void AddSelectableProfile(const gfx::ImageSkia& image,
                             const base::string16& name,
                             bool is_guest,
                             base::RepeatingClosure action);
-  void AddProfileManagementShortcutFeatureButton(const gfx::ImageSkia& icon,
+  void AddProfileManagementShortcutFeatureButton(const gfx::VectorIcon& icon,
                                                  const base::string16& text,
                                                  base::RepeatingClosure action);
-  void AddProfileManagementFeatureButton(const gfx::ImageSkia& icon,
+  void AddProfileManagementFeatureButton(const gfx::VectorIcon& icon,
                                          const base::string16& text,
                                          base::RepeatingClosure action);
 
-  // 0 < |icon_to_image_ratio| <= 1 is the size ratio of |icon| in the returned
-  // image. E.g. a value of 0.8 means that |icon| only takes up 80% of the
-  // returned image, with the rest being padding around it.
-  gfx::ImageSkia ImageForMenu(const gfx::VectorIcon& icon,
-                              float icon_to_image_ratio = 1.0f) const;
   gfx::ImageSkia ColoredImageForMenu(const gfx::VectorIcon& icon,
                                      SkColor color) const;
   // Should be called inside each button/link action.