Bringing back fast user switching on Mac material design user menu

chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm

Index: chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
diff --git a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
index d263e6c1f57d2cf38b7cdf052582849c445279a5..4dd12bc3b7ecd474c99a25fb540afb1f36988790 100644
--- a/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
+++ b/chrome/browser/ui/cocoa/profiles/profile_chooser_controller.mm
@@ -34,6 +34,7 @@
 #include "chrome/browser/signin/signin_ui_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/chrome_style.h"
@@ -74,8 +75,12 @@
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
+#include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/text_elider.h"
+#include "ui/gfx/vector_icons_public.h"
 #include "ui/native_theme/common_theme.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/native_theme/native_theme_mac.h"
@@ -427,10 +432,14 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   int leftMarginSpacing_;
   // Spacing between the cell image and title.
   int imageTitleSpacing_;
+  // Padding added to the right margin of the button.
+  int rightMarginSpacing_;
 }
 
 - (id)initWithLeftMarginSpacing:(int)leftMarginSpacing
               imageTitleSpacing:(int)imageTitleSpacing;
+
+- (void)addRightMarginSpacing:(int)rightMarginSpacing;
 @end
 
 @implementation CustomPaddingImageButtonCell
@@ -443,6 +452,10 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   return self;
 }
 
+- (void)addRightMarginSpacing:(int)rightMarginSpacing {

[groby-ooo-7-16, 2016/07/20 20:54:22]

nit: setRightMargingSpacing seems more appropriate than add...

[Jane, 2016/07/20 22:03:46]

Done.

+  rightMarginSpacing_ = rightMarginSpacing;
+}
+
 - (NSRect)drawTitle:(NSAttributedString*)title
           withFrame:(NSRect)frame
              inView:(NSView*)controlView {
@@ -454,6 +467,9 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   if ([self imagePosition] == NSImageLeft)
     NSDivideRect(frame, &marginRect, &frame, imageTitleSpacing_, NSMinXEdge);
 
+  if (rightMarginSpacing_ > 0)
+    NSDivideRect(frame, &marginRect, &frame, rightMarginSpacing_, NSMaxXEdge);

[groby-ooo-7-16, 2016/07/20 20:54:22]

You can probably skip the ">0" test.

[Jane, 2016/07/20 22:03:46]

Done.

+
   return [super drawTitle:title withFrame:frame inView:controlView];
 }
 
@@ -797,6 +813,8 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   base::scoped_nsobject<NSColor> backgroundColor_;
   base::scoped_nsobject<NSColor> hoverColor_;
 }
+
+- (void)addRightMarginSpacing:(int)rightMarginSpacing;
 @end
 
 @implementation BackgroundColorHoverButton
@@ -826,6 +844,10 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   return self;
 }
 
+- (void)addRightMarginSpacing:(int)rightMarginSpacing {
+  [[self cell] addRightMarginSpacing:rightMarginSpacing];
+}
+
 - (void)drawRect:(NSRect)dirtyRect {
   if ([self isEnabled]) {
     bool isHighlighted = ([self hoverState] != kHoverStateNone);
@@ -899,14 +921,18 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 - (CGFloat)addSeparatorToContainer:(NSView*)container
                          atYOffset:(CGFloat)yOffset;
 
-// Builds the right-click profile switcher.
-- (void)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles
-                                    atYOffset:(CGFloat)yOffset
-                                  inContainer:(NSView*)container;
+// Builds the fast user switcher view. In the current user menu, this is
+// triggered by right-clicking the avatar button; in the material design user
+// menu, this appears as part of the user menu. Returns the yOffset
+// corresponding to after the profile switcher buttons.
+- (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles
+                                       atYOffset:(CGFloat)yOffset
+                                     inContainer:(NSView*)container;
 
 // Builds the regular profile chooser view.
 - (void)buildProfileChooserViewWithProfileView:(NSView*)currentProfileView
                                   tutorialView:(NSView*)tutorialView
+                                 otherProfiles:(NSMutableArray*)otherProfiles
                                      atYOffset:(CGFloat)yOffset
                                    inContainer:(NSView*)container
                                    displayLock:(bool)displayLock;
@@ -967,12 +993,17 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 - (NSView*)createGuestProfileView;
 
 // Creates an item for the profile |itemIndex| that is used in the fast profile
-// switcher in the middle of the bubble.
+// switcher view.
 - (NSButton*)createOtherProfileView:(int)itemIndex;
 
-// Creates the "Not you" and Lock option buttons.
+// Creates the following option buttons: lock profile, go incognito, and switch
+// user/exit guest.
 - (NSView*)createOptionsViewWithRect:(NSRect)rect
                          displayLock:(BOOL)displayLock;
+// For material design user menu, creates the following option buttons: lock
+// profile/close all windows, switch user/exit guest, and open guest profile.
+- (NSView*)createMaterialDesignOptionsViewWithRect:(NSRect)rect
+                                       displayLock:(BOOL)displayLock;
 
 // Creates the account management view for the active profile.
 - (NSView*)createCurrentProfileAccountsView:(NSRect)rect;
@@ -991,11 +1022,15 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 - (NSView*)buildSwitchUserView;
 
 // Creates a button with |text| and |action|, optionally with an icon given by
-// |imageResourceId|.
+// |imageResourceId| or |image|.
 - (NSButton*)hoverButtonWithRect:(NSRect)rect
                             text:(NSString*)text
                  imageResourceId:(int)imageResourceId
                           action:(SEL)action;
+- (NSButton*)hoverButtonWithRect:(NSRect)rect
+                            text:(NSString*)text
+                           image:(NSImage*)image
+                          action:(SEL)action;
 - (BackgroundColorHoverButton*)hoverButtonWithRect:(NSRect)rect
                                               text:(NSString*)text
                                             action:(SEL)action;
@@ -1040,6 +1075,13 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
                                ProfileMetrics::SWITCH_PROFILE_ICON);
 }
 
+- (IBAction)switchToGuest:(id)sender {
+  PrefService* service = g_browser_process->local_state();

[groby-ooo-7-16, 2016/07/20 20:54:22]

Curious - why go through the global process, instead of
browser_->profile()->GetPrefs()?

[Jane, 2016/07/20 22:03:46]

It doesn't seem like profile()->GetPrefs() have this value (?) - I get a "trying
to read an unregistered pref" error when I use that. The global process way is
how it's done in profile_chooser_view.cc so I just took it.

[groby-ooo-7-16, 2016/07/21 00:01:46]

Ah, nevermind, my brain misfired - kBrowserGuestModeEnabled is indeed a
browser-wide preference instead of per-profile (it has to be, obviously :)

+  DCHECK(service);
+  DCHECK(service->GetBoolean(prefs::kBrowserGuestModeEnabled));
+  profiles::SwitchToGuestProfile(ProfileManager::CreateCallback());
+}
+
 - (IBAction)showUserManager:(id)sender {
   UserManager::Show(base::FilePath(),
                     profiles::USER_MANAGER_NO_TUTORIAL,
@@ -1056,6 +1098,10 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   profiles::CloseGuestProfileWindows();
 }
 
+- (IBAction)closeAllWindows:(id)sender {
+  profiles::CloseProfileWindows(browser_->profile());
+}
+
 - (IBAction)goIncognito:(id)sender {
   DCHECK([self shouldShowGoIncognito]);
   chrome::NewIncognitoWindow(browser_);
@@ -1339,33 +1385,52 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 // Builds the fast user switcher view in |container| at |yOffset| and populates
 // it with the entries for every profile in |otherProfiles|. Returns the new
 // yOffset after adding the elements.
-- (void)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles
-                                    atYOffset:(CGFloat)yOffset
-                                  inContainer:(NSView*)container {
+- (CGFloat)buildFastUserSwitcherViewWithProfiles:(NSMutableArray*)otherProfiles

[groby-ooo-7-16, 2016/07/20 20:54:22]

Is there any reason this needs to be a mutable array?

[Jane, 2016/07/20 22:03:46]

I think it's because this array needs the dynamic insertion operations for its
construction. See L1521 in this patch. Should this be something else?

[groby-ooo-7-16, 2016/07/21 00:01:46]

You need that only at construction time, though. The method here doesn't need to
modify, so NSArray is fine. (In cocoa, NSArray vs NSMutableArray - same naming
for other ObjC containers - is the equivalent of specifying const for C++
containers)

[Jane, 2016/07/21 13:49:52]

Done. Ah, I see what you mean! I changed it to be NSArray everywhere else.

+                                       atYOffset:(CGFloat)yOffset
+                                     inContainer:(NSView*)container {
   // Other profiles switcher. The profiles have already been sorted
   // by their y-coordinate, so they can be added in the existing order.
   for (NSView* otherProfileView in otherProfiles) {
-   [otherProfileView setFrameOrigin:NSMakePoint(0, yOffset)];
-   [container addSubview:otherProfileView];
-   yOffset = NSMaxY([otherProfileView frame]);
+    [otherProfileView setFrameOrigin:NSMakePoint(0, yOffset)];
+    [container addSubview:otherProfileView];
+    yOffset = NSMaxY([otherProfileView frame]);
 
-   yOffset = [self addSeparatorToContainer:container atYOffset: yOffset];
+    if (!switches::IsMaterialDesignUserMenu())
+      yOffset = [self addSeparatorToContainer:container atYOffset:yOffset];
   }
 
   [container setFrameSize:NSMakeSize(GetFixedMenuWidth(), yOffset)];
+  return yOffset;
 }
 
 - (void)buildProfileChooserViewWithProfileView:(NSView*)currentProfileView
                                   tutorialView:(NSView*)tutorialView
+                                 otherProfiles:(NSMutableArray*)otherProfiles
                                      atYOffset:(CGFloat)yOffset
                                    inContainer:(NSView*)container
                                    displayLock:(bool)displayLock {
+  if (switches::IsMaterialDesignUserMenu())
+    yOffset += kRelatedControllVerticalSpacing;
+
   // Option buttons.
   NSRect rect = NSMakeRect(0, yOffset, GetFixedMenuWidth(), 0);
-  NSView* optionsView = [self createOptionsViewWithRect:rect
-                                            displayLock:displayLock];
+  NSView* optionsView =
+      switches::IsMaterialDesignUserMenu()
+          ? [self createMaterialDesignOptionsViewWithRect:rect
+                                              displayLock:displayLock]
+          : [self createOptionsViewWithRect:rect displayLock:displayLock];
   [container addSubview:optionsView];
   rect.origin.y = NSMaxY([optionsView frame]);
+  yOffset = rect.origin.y;
+
+  // For material design user menu, add the fast user switching buttons.
+  if (switches::IsMaterialDesignUserMenu()) {
+    yOffset = [self buildFastUserSwitcherViewWithProfiles:otherProfiles
+                                                atYOffset:yOffset
+                                              inContainer:container];
+    yOffset += kRelatedControllVerticalSpacing;
+    rect.origin.y = yOffset;
+  }
 
   NSBox* separator = [self horizontalSeparatorWithFrame:rect];
   [container addSubview:separator];
@@ -1465,12 +1530,15 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   CGFloat yOffset = 1;
 
   if (isFastProfileChooser) {
-    [self buildFastUserSwitcherViewWithProfiles:otherProfiles.get()
-                                      atYOffset:yOffset
-                                    inContainer:container.get()];
+    if (!switches::IsMaterialDesignUserMenu()) {
+      [self buildFastUserSwitcherViewWithProfiles:otherProfiles.get()
+                                        atYOffset:yOffset
+                                      inContainer:container.get()];

[groby-ooo-7-16, 2016/07/20 20:54:22]

No reason for .get(), I think.

[Jane, 2016/07/20 22:03:46]

Done. I'm assuming all the .get()'s here? (What is .get() usually used for?)

[groby-ooo-7-16, 2016/07/21 00:01:46]

get() is usually used if the types don't match exactly - i.e. if you have a
scoped_nsobject<NSMutableArray>, but the method takes an NSArray*, you'll need
get(). scoped_nsobject<T> safely degrades to T*, though.  

[Jane, 2016/07/21 13:49:52]

Ah, I see! Now that I modified these two methods to be NSArray, I added back the
.get()'s for otherProfiles.

+    }
   } else {
     [self buildProfileChooserViewWithProfileView:currentProfileView
                                     tutorialView:tutorialView
+                                   otherProfiles:otherProfiles.get()

[groby-ooo-7-16, 2016/07/20 20:54:22]

see above

[Jane, 2016/07/20 22:03:46]

Done.

                                        atYOffset:yOffset
                                      inContainer:container.get()
                                      displayLock:displayLock];
@@ -2082,23 +2150,39 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
 
   NSRect rect = NSMakeRect(0, 0, GetFixedMenuWidth(),
                            kBlueButtonHeight + kSmallVerticalSpacing);
+  const int imageTitleSpacing = switches::IsMaterialDesignUserMenu()
+                                    ? kHorizontalSpacing
+                                    : kImageTitleSpacing;
   base::scoped_nsobject<BackgroundColorHoverButton> profileButton(
       [[BackgroundColorHoverButton alloc]
-          initWithFrame:rect
-      imageTitleSpacing:kImageTitleSpacing
-        backgroundColor:GetDialogBackgroundColor()]);
+              initWithFrame:rect
+          imageTitleSpacing:imageTitleSpacing
+            backgroundColor:GetDialogBackgroundColor()]);
+  if (switches::IsMaterialDesignUserMenu())
+    [profileButton addRightMarginSpacing:kHorizontalSpacing];
 
   NSString* title = base::SysUTF16ToNSString(
       profiles::GetProfileSwitcherTextForItem(item));
   [profileButton setTitle:title];
 
-  // Use the low-res, small default avatars in the fast user switcher, like
-  // we do in the menu bar.
-  gfx::Image itemIcon;
-  AvatarMenu::GetImageForMenuButton(item.profile_path, &itemIcon);
+  CGFloat availableWidth;
+  if (switches::IsMaterialDesignUserMenu()) {
+    int iconImageSide = 18;
+    [profileButton setDefaultImage:CreateProfileImage(item.icon, iconImageSide,
+                                                      profiles::SHAPE_CIRCLE)];
+    availableWidth = rect.size.width - iconImageSide - imageTitleSpacing -
+                     2 * kHorizontalSpacing;
+  } else {
+    // Use the low-res, small default avatars in the fast user switcher, like
+    // we do in the menu bar.
+    gfx::Image itemIcon;
+    AvatarMenu::GetImageForMenuButton(item.profile_path, &itemIcon);
+    [profileButton setDefaultImage:CreateProfileImage(itemIcon, kSmallImageSide,
+                                                      profiles::SHAPE_SQUARE)];
+    availableWidth = rect.size.width - kSmallImageSide - imageTitleSpacing -
+                     kHorizontalSpacing;
+  }
 
-  [profileButton setDefaultImage:CreateProfileImage(itemIcon, kSmallImageSide,
-                                                    profiles::SHAPE_SQUARE)];
   [profileButton setImagePosition:NSImageLeft];
   [profileButton setAlignment:NSLeftTextAlignment];
   [profileButton setBordered:NO];
@@ -2110,9 +2194,6 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
     NSFontAttributeName : [profileButton font]
   }];
 
-  CGFloat availableWidth = rect.size.width - kSmallImageSide -
-                           kImageTitleSpacing - kHorizontalSpacing;
-
   if (std::ceil(textSize.width) > availableWidth)
     [profileButton setToolTip:[profileButton title]];
 
@@ -2141,7 +2222,7 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
     viewRect.origin.y = NSMaxY([separator frame]);
   }
 
-  if (!switches::IsMaterialDesignUserMenu() && [self shouldShowGoIncognito]) {
+  if ([self shouldShowGoIncognito]) {
     NSButton* goIncognitoButton =
         [self hoverButtonWithRect:viewRect
                              text:l10n_util::GetNSString(
@@ -2159,9 +2240,6 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   NSString* text = isGuestSession_ ?
       l10n_util::GetNSString(IDS_PROFILES_EXIT_GUEST) :
       l10n_util::GetNSString(IDS_PROFILES_SWITCH_USERS_BUTTON);
-  if (!isGuestSession_ && switches::IsMaterialDesignUserMenu()) {
-    text = l10n_util::GetNSString(IDS_PROFILES_MANAGE_USERS_BUTTON);
-  }
   NSButton* switchUsersButton =
       [self hoverButtonWithRect:viewRect
                            text:text
@@ -2216,6 +2294,86 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   return container.autorelease();
 }
 
+- (NSView*)createMaterialDesignOptionsViewWithRect:(NSRect)rect

[groby-ooo-7-16, 2016/07/20 20:54:22]

Traditionally, this would be named create...WithFrame:

(We also wouldn't have a method called createXYZ, but that ship has sailed :)

[Jane, 2016/07/20 22:03:46]

Done. But there are soo many methods called createXYZ now...

[groby-ooo-7-16, 2016/07/21 00:01:46]

I know - that's why I said the ship has sailed for the create prefix :)

+                                       displayLock:(BOOL)displayLock {

[groby-ooo-7-16, 2016/07/20 20:54:22]

showLock: instead of displayLock: is usually the convention.

[Jane, 2016/07/20 22:03:46]

Done. I went ahead to change all the displayLock -> showLock.

+  NSRect viewRect = NSMakeRect(0, 0, rect.size.width,
+                               kBlueButtonHeight + kSmallVerticalSpacing);
+  base::scoped_nsobject<NSView> container([[NSView alloc] initWithFrame:rect]);
+  const int material_icon_size = 20;
+
+  // Create a lock profile button when supervised users exist; otherwise, create
+  // a button that closes all of the current profile's windows if more than one
+  // is open.
+  if (displayLock) {
+    NSButton* lockButton =
+        [self hoverButtonWithRect:viewRect
+                             text:l10n_util::GetNSString(
+                                      IDS_PROFILES_PROFILE_SIGNOUT_BUTTON)
+                            image:NSImageFromImageSkia(gfx::CreateVectorIcon(
+                                      gfx::VectorIconId::LOCK,
+                                      material_icon_size, gfx::kChromeIconGrey))
+                           action:@selector(lockProfile:)];
+    [container addSubview:lockButton];
+    viewRect.origin.y = NSMaxY([lockButton frame]);
+  } else if (!isGuestSession_) {
+    int num_browsers = 0;
+    for (auto* browser : *BrowserList::GetInstance()) {
+      if (browser->profile()->GetOriginalProfile() ==
+          browser_->profile()->GetOriginalProfile())

[groby-ooo-7-16, 2016/07/20 20:54:22]

nit: can you hoist the rhs into a separate variable, outside the loop? (I had to
read this a few times before seeing the underscore :)

[Jane, 2016/07/20 22:03:46]

Done.

+        num_browsers++;
+    }
+    if (num_browsers > 1) {
+      NSButton* closeAllWindowsButton = [self
+          hoverButtonWithRect:viewRect
+                         text:l10n_util::GetNSString(
+                                  IDS_PROFILES_CLOSE_ALL_WINDOWS_BUTTON)
+                        image:NSImageFromImageSkia(gfx::CreateVectorIcon(
+                                  gfx::VectorIconId::CLOSE_ALL,
+                                  material_icon_size, gfx::kChromeIconGrey))
+                       action:@selector(closeAllWindows:)];
+      [container addSubview:closeAllWindowsButton];
+      viewRect.origin.y = NSMaxY([closeAllWindowsButton frame]);
+    }
+  }
+
+  // Create a manage users/exit guest button.
+  NSString* text =
+      isGuestSession_
+          ? l10n_util::GetNSString(IDS_PROFILES_EXIT_GUEST)
+          : l10n_util::GetNSString(IDS_PROFILES_MANAGE_USERS_BUTTON);
+  NSImage* icon = NSImageFromImageSkia(
+      gfx::CreateVectorIcon(isGuestSession_ ? gfx::VectorIconId::CLOSE_ALL
+                                            : gfx::VectorIconId::SETTINGS,
+                            material_icon_size, gfx::kChromeIconGrey));
+  SEL action =
+      isGuestSession_ ? @selector(exitGuest:) : @selector(showUserManager:);
+  NSButton* manageUsersButton =
+      [self hoverButtonWithRect:viewRect text:text image:icon action:action];
+  viewRect.origin.y = NSMaxY([manageUsersButton frame]);
+  [container addSubview:manageUsersButton];
+
+  // Create a guest profile button.
+  if (!isGuestSession_) {
+    PrefService* service = g_browser_process->local_state();

[groby-ooo-7-16, 2016/07/20 20:54:22]

See above question re: getting the prefs from |browser_|

[Jane, 2016/07/20 22:03:46]

See above.

+    DCHECK(service);
+    if (service->GetBoolean(prefs::kBrowserGuestModeEnabled)) {
+      NSButton* guestProfileButton = [self
+          hoverButtonWithRect:viewRect
+                         text:l10n_util::GetNSString(
+                                  IDS_PROFILES_GUEST_PROFILE_NAME)
+                        image:NSImageFromImageSkia(gfx::CreateVectorIcon(
+                                  gfx::VectorIconId::ACCOUNT_CIRCLE,
+                                  material_icon_size, gfx::kChromeIconGrey))
+                       action:@selector(switchToGuest:)];
+      viewRect.origin.y = NSMaxY([guestProfileButton frame]);
+      [container addSubview:guestProfileButton];
+    }
+  }
+
+  [container setFrameSize:NSMakeSize(rect.size.width, viewRect.origin.y)];
+  return container.autorelease();
+}
+
 - (NSView*)createAccountsListWithRect:(NSRect)rect {
   base::scoped_nsobject<NSView> container([[NSView alloc] initWithFrame:rect]);
   currentProfileAccounts_.clear();
@@ -2465,10 +2623,17 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
                             text:(NSString*)text
                  imageResourceId:(int)imageResourceId
                           action:(SEL)action {
-  BackgroundColorHoverButton* button =
-      [self hoverButtonWithRect:rect text:text action:action];
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();

[groby-ooo-7-16, 2016/07/20 20:54:22]

nit: Reference please.

[Jane, 2016/07/20 22:03:46]

Done.

   NSImage* image = rb->GetNativeImageNamed(imageResourceId).ToNSImage();
+  return [self hoverButtonWithRect:rect text:text image:image action:action];
+}
+
+- (NSButton*)hoverButtonWithRect:(NSRect)rect
+                            text:(NSString*)text
+                           image:(NSImage*)image
+                          action:(SEL)action {
+  BackgroundColorHoverButton* button =
+      [self hoverButtonWithRect:rect text:text action:action];
   [button setDefaultImage:image];
   [button setHoverImage:image];
   [button setPressedImage:image];
@@ -2483,7 +2648,9 @@ class ActiveProfileObserverBridge : public AvatarMenuObserver,
   base::scoped_nsobject<BackgroundColorHoverButton> button(
       [[BackgroundColorHoverButton alloc]
               initWithFrame:rect
-          imageTitleSpacing:kImageTitleSpacing
+          imageTitleSpacing:switches::IsMaterialDesignUserMenu()
+                                ? (kHorizontalSpacing - 2.0)

[groby-ooo-7-16, 2016/07/20 20:54:22]

Why -2.0? (Please leave a comment explaining magic numbers. Even if it's just
"so it looks right, not sure why we have to move 2.0")

[Jane, 2016/07/20 22:03:46]

Done. Is this clear, and is this too long of a comment?

[groby-ooo-7-16, 2016/07/21 00:01:46]

It's a bit long - but not exceedingly so. If you can shorten it a bit, great, if
not, also good. (I'd rather have a long comment than a cryptic comment :)

+                                : kImageTitleSpacing
             backgroundColor:GetDialogBackgroundColor()]);
 
   [button setTitle:text];