Bringing back fast user switching on Mac material design user menu

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/profiles/profile_chooser_controller.h"
 
 #import <Carbon/Carbon.h>  // kVK_Return.
 #import <Cocoa/Cocoa.h>
 #include <stddef.h>
 
@@ skipping 16 matching lines @@
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/signin/chrome_signin_helper.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_error_controller_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/browser/signin/signin_promo.h"
 #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"
 #import "chrome/browser/ui/cocoa/browser_window_utils.h"
 #import "chrome/browser/ui/cocoa/info_bubble_view.h"
 #import "chrome/browser/ui/cocoa/info_bubble_window.h"
 #include "chrome/browser/ui/cocoa/profiles/signin_view_controller_delegate_mac.h"
 #import "chrome/browser/ui/cocoa/profiles/user_manager_mac.h"
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/user_manager.h"
@@ skipping 20 matching lines @@
 #import "ui/base/cocoa/cocoa_base_utils.h"
 #import "ui/base/cocoa/controls/blue_label_button.h"
 #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
 #import "ui/base/cocoa/controls/hyperlink_text_view.h"
 #import "ui/base/cocoa/hover_image_button.h"
 #include "ui/base/cocoa/window_size_constants.h"
 #include "ui/base/l10n/l10n_util.h"
 #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"
 
 namespace {
 
 // Constants taken from the Windows/Views implementation at:
 // chrome/browser/ui/views/profile_chooser_view.cc
 const int kLargeImageSide = 88;
 const int kSmallImageSide = 32;
@@ skipping 331 matching lines @@
 };
 
 // Custom button cell that adds a left padding before the button image, and
 // a custom spacing between the button image and title.
 @interface CustomPaddingImageButtonCell : NSButtonCell {
  @private
   // Padding added to the left margin of the button.
   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
 - (id)initWithLeftMarginSpacing:(int)leftMarginSpacing
               imageTitleSpacing:(int)imageTitleSpacing {
   if ((self = [super init])) {
     leftMarginSpacing_ = leftMarginSpacing;
     imageTitleSpacing_ = imageTitleSpacing;
   }
   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 {
   NSRect marginRect;
   NSDivideRect(frame, &marginRect, &frame, leftMarginSpacing_, NSMinXEdge);
 
   // The title frame origin isn't aware of the left margin spacing added
   // in -drawImage, so it must be added when drawing the title as well.
   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];
 }
 
 - (void)drawImage:(NSImage*)image
         withFrame:(NSRect)frame
            inView:(NSView*)controlView {
   if ([self imagePosition] == NSImageLeft)
     frame.origin.x = leftMarginSpacing_;
   [super drawImage:image withFrame:frame inView:controlView];
 }
@@ skipping 323 matching lines @@
 }
 
 @end
 
 // A custom button that allows for setting a background color when hovered over.
 @interface BackgroundColorHoverButton : HoverImageButton {
  @private
   base::scoped_nsobject<NSColor> backgroundColor_;
   base::scoped_nsobject<NSColor> hoverColor_;
 }
+
+- (void)addRightMarginSpacing:(int)rightMarginSpacing;
 @end
 
 @implementation BackgroundColorHoverButton
 
 - (id)initWithFrame:(NSRect)frameRect
   imageTitleSpacing:(int)imageTitleSpacing
     backgroundColor:(NSColor*)backgroundColor {
   if ((self = [super initWithFrame:frameRect])) {
     backgroundColor_.reset([backgroundColor retain]);
     // Use a color from Aura, since this button is not trying to look like a
     // native control.
     SkColor hoverColor = ui::GetAuraColor(
         ui::NativeTheme::kColorId_ButtonHoverBackgroundColor, nullptr);
     hoverColor_.reset([skia::SkColorToSRGBNSColor(hoverColor) retain]);
 
     [self setBordered:NO];
     [self setFont:[NSFont labelFontOfSize:kTextFontSize]];
     [self setButtonType:NSMomentaryChangeButton];
 
     base::scoped_nsobject<CustomPaddingImageButtonCell> cell(
         [[CustomPaddingImageButtonCell alloc]
             initWithLeftMarginSpacing:kHorizontalSpacing
                     imageTitleSpacing:imageTitleSpacing]);
     [cell setLineBreakMode:NSLineBreakByTruncatingTail];
     [self setCell:cell.get()];
   }
   return self;
 }
 
+- (void)addRightMarginSpacing:(int)rightMarginSpacing {
+  [[self cell] addRightMarginSpacing:rightMarginSpacing];
+}
+
 - (void)drawRect:(NSRect)dirtyRect {
   if ([self isEnabled]) {
     bool isHighlighted = ([self hoverState] != kHoverStateNone);
     NSColor* backgroundColor = isHighlighted ? hoverColor_ : backgroundColor_;
     [[self cell] setBackgroundColor:backgroundColor];
   }
   [super drawRect:dirtyRect];
 }
 
 -(void)keyDown:(NSEvent*)event {
@@ skipping 53 matching lines @@
 }
 
 @end
 
 @interface ProfileChooserController ()
 // Adds an horizontal separator to |container| at |yOffset| and returns the
 // yOffset corresponding to after the separator.
 - (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;
 
 // Builds the profile chooser view.
 - (NSView*)buildProfileChooserView;
 
 - (NSView*)buildTutorialViewIfNeededForItem:(const AvatarMenu::Item&)item;
 
 // Builds a tutorial card with a title label using |titleMessage|, a content
@@ skipping 40 matching lines @@
                                        rect:(NSRect)rect;
 
 // Creates the disclaimer text for supervised users, telling them that the
 // manager can view their history etc.
 - (NSView*)createSupervisedUserDisclaimerView;
 
 // Creates a main profile card for the guest user.
 - (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;
 
 // Creates the list of accounts for the active profile.
 - (NSView*)createAccountsListWithRect:(NSRect)rect;
 
 // Creates the Gaia sign-in/add account view.
 - (NSView*)buildGaiaEmbeddedView;
 
 // Creates the account removal view.
 - (NSView*)buildAccountRemovalView;
 
 // Create a view that shows various options for an upgrade user who is not
 // the same person as the currently signed in user.
 - (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;
 
 // Creates a generic link button with |title| and an |action| positioned at
 // |frameOrigin|.
 - (NSButton*)linkButtonWithTitle:(NSString*)title
                      frameOrigin:(NSPoint)frameOrigin
                           action:(SEL)action;
 
@@ skipping 24 matching lines @@
 }
 
 - (IBAction)switchToProfile:(id)sender {
   // Check the event flags to see if a new window should be created.
   bool alwaysCreate = ui::WindowOpenDispositionFromNSEvent(
       [NSApp currentEvent]) == NEW_WINDOW;
   avatarMenu_->SwitchToProfile([sender tag], alwaysCreate,
                                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,
                     profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
   [self postActionPerformed:
       ProfileMetrics::PROFILE_DESKTOP_MENU_OPEN_USER_MANAGER];
 }
 
 - (IBAction)exitGuest:(id)sender {
   DCHECK(browser_->profile()->IsGuestSession());
   UserManager::Show(base::FilePath(),
                     profiles::USER_MANAGER_NO_TUTORIAL,
                     profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
   profiles::CloseGuestProfileWindows();
 }
 
+- (IBAction)closeAllWindows:(id)sender {
+  profiles::CloseProfileWindows(browser_->profile());
+}
+
 - (IBAction)goIncognito:(id)sender {
   DCHECK([self shouldShowGoIncognito]);
   chrome::NewIncognitoWindow(browser_);
   [self postActionPerformed:
       ProfileMetrics::PROFILE_DESKTOP_MENU_GO_INCOGNITO];
 }
 
 - (IBAction)showAccountManagement:(id)sender {
   [self initMenuContentsWithView:profiles::BUBBLE_VIEW_MODE_ACCOUNT_MANAGEMENT];
 }
@@ skipping 263 matching lines @@
   NSBox* separator =
       [self horizontalSeparatorWithFrame:NSMakeRect(0, yOffset,
                                                     GetFixedMenuWidth(), 0)];
   [container addSubview:separator];
   return NSMaxY([separator frame]);
 }
 
 // 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];
   yOffset = NSMaxY([separator frame]);
 
   // For supervised users, add the disclaimer text.
   if (browser_->profile()->IsSupervised()) {
     yOffset += switches::IsMaterialDesignUserMenu() ? kVerticalSpacing
                                                     : kSmallVerticalSpacing;
     NSView* disclaimerContainer = [self createSupervisedUserDisclaimerView];
@@ skipping 79 matching lines @@
   }
   if (!currentProfileView)  // Guest windows don't have an active profile.
     currentProfileView = [self createGuestProfileView];
 
   // |yOffset| is the next position at which to draw in |container|
   // coordinates. Add a pixel offset so that the bottom option buttons don't
   // overlap the bubble's rounded corners.
   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];
   }
 
   return container.autorelease();
 }
 
 - (NSView*)buildSigninConfirmationView {
   ProfileMetrics::LogProfileNewAvatarMenuSignin(
@@ skipping 591 matching lines @@
   return switches::IsMaterialDesignUserMenu()
              ? [self createMaterialDesignCurrentProfileView:guestItem]
              : [self createCurrentProfileView:guestItem];
 }
 
 - (NSButton*)createOtherProfileView:(int)itemIndex {
   const AvatarMenu::Item& item = avatarMenu_->GetItemAt(itemIndex);
 
   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];
   [profileButton setTag:itemIndex];
   [profileButton setTarget:self];
   [profileButton setAction:@selector(switchToProfile:)];
 
   NSSize textSize = [[profileButton title] sizeWithAttributes:@{
     NSFontAttributeName : [profileButton font]
   }];
 
-  CGFloat availableWidth = rect.size.width - kSmallImageSide -
-                           kImageTitleSpacing - kHorizontalSpacing;
-
   if (std::ceil(textSize.width) > availableWidth)
     [profileButton setToolTip:[profileButton title]];
 
   return profileButton.autorelease();
 }
 
 - (NSView*)createOptionsViewWithRect:(NSRect)rect
                          displayLock:(BOOL)displayLock {
   NSRect viewRect = NSMakeRect(0, 0,
                                rect.size.width,
                                kBlueButtonHeight + kSmallVerticalSpacing);
   base::scoped_nsobject<NSView> container([[NSView alloc] initWithFrame:rect]);
 
   if (displayLock) {
     NSButton* lockButton =
         [self hoverButtonWithRect:viewRect
                              text:l10n_util::GetNSString(
                                   IDS_PROFILES_PROFILE_SIGNOUT_BUTTON)
                   imageResourceId:IDR_ICON_PROFILES_MENU_LOCK
                            action:@selector(lockProfile:)];
     [container addSubview:lockButton];
     viewRect.origin.y = NSMaxY([lockButton frame]);
 
     NSBox* separator = [self horizontalSeparatorWithFrame:viewRect];
     [container addSubview:separator];
     viewRect.origin.y = NSMaxY([separator frame]);
   }
 
-  if (!switches::IsMaterialDesignUserMenu() && [self shouldShowGoIncognito]) {
+  if ([self shouldShowGoIncognito]) {
     NSButton* goIncognitoButton =
         [self hoverButtonWithRect:viewRect
                              text:l10n_util::GetNSString(
                                   IDS_PROFILES_GO_INCOGNITO_BUTTON)
                   imageResourceId:IDR_ICON_PROFILES_MENU_INCOGNITO
                            action:@selector(goIncognito:)];
     viewRect.origin.y = NSMaxY([goIncognitoButton frame]);
     [container addSubview:goIncognitoButton];
 
     NSBox* separator = [self horizontalSeparatorWithFrame:viewRect];
     [container addSubview:separator];
     viewRect.origin.y = NSMaxY([separator frame]);
   }
 
   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
                 imageResourceId:IDR_ICON_PROFILES_MENU_AVATAR
                          action:isGuestSession_? @selector(exitGuest:) :
                                                  @selector(showUserManager:)];
   viewRect.origin.y = NSMaxY([switchUsersButton frame]);
   [container addSubview:switchUsersButton];
 
   [container setFrameSize:NSMakeSize(rect.size.width, viewRect.origin.y)];
@@ skipping 34 matching lines @@
 
   NSView* accountEmails = [self createAccountsListWithRect:NSMakeRect(
       0, rect.origin.y, rect.size.width, kAccountButtonHeight)];
   [container addSubview:accountEmails];
 
   [container setFrameSize:NSMakeSize(rect.size.width,
                                      NSMaxY([accountEmails frame]))];
   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();
 
   Profile* profile = browser_->profile();
   std::string primaryAccount =
       SigninManagerFactory::GetForProfile(profile)->GetAuthenticatedAccountId();
   DCHECK(!primaryAccount.empty());
   std::vector<std::string>accounts =
       profiles::GetSecondaryAccountsForProfile(profile, primaryAccount);
@@ skipping 229 matching lines @@
    clickedOnLink:(id)link
          atIndex:(NSUInteger)charIndex {
   chrome::ShowSettings(browser_);
   return YES;
 }
 
 - (NSButton*)hoverButtonWithRect:(NSRect)rect
                             text:(NSString*)text
                  imageResourceId:(int)imageResourceId
                           action:(SEL)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];
-  ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
-  NSImage* image = rb->GetNativeImageNamed(imageResourceId).ToNSImage();
   [button setDefaultImage:image];
   [button setHoverImage:image];
   [button setPressedImage:image];
   [button setImagePosition:NSImageLeft];
 
   return button;
 }
 
 - (BackgroundColorHoverButton*)hoverButtonWithRect:(NSRect)rect
                                               text:(NSString*)text
                                             action:(SEL)action {
   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];
   [button setAlignment:NSLeftTextAlignment];
   [button setBordered:NO];
   [button setTarget:self];
   [button setAction:action];
 
   return button.autorelease();
 }
@@ skipping 85 matching lines @@
 }
 
 - (bool)shouldShowGoIncognito {
   bool incognitoAvailable =
       IncognitoModePrefs::GetAvailability(browser_->profile()->GetPrefs()) !=
           IncognitoModePrefs::DISABLED;
   return incognitoAvailable && !browser_->profile()->IsGuestSession();
 }
 
 @end