[Mac] Redesign of the avatar menu button

chrome/browser/ui/cocoa/browser/new_avatar_button_controller.mm

Index: chrome/browser/ui/cocoa/browser/new_avatar_button_controller.mm
diff --git a/chrome/browser/ui/cocoa/browser/new_avatar_button_controller.mm b/chrome/browser/ui/cocoa/browser/new_avatar_button_controller.mm
new file mode 100644
index 0000000000000000000000000000000000000000..be1667db75752533780a43938de20734108395ac
--- /dev/null
+++ b/chrome/browser/ui/cocoa/browser/new_avatar_button_controller.mm
@@ -0,0 +1,194 @@
+// 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/browser/new_avatar_button_controller.h"
+
+#include "base/strings/sys_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile_metrics.h"
+#include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_window.h"
+#import "chrome/browser/ui/cocoa/browser/profile_chooser_controller.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#include "content/public/browser/notification_service.h"
+#include "grit/generated_resources.h"
+#include "grit/theme_resources.h"
+#include "ui/base/l10n/l10n_util_mac.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/text_elider.h"
+
+namespace {
+
+NSString* GetElidedProfileName(const base::string16& name) {
+  // Maximum characters the button can be before the text will get elided.
+  const int kMaxCharactersToDisplay = 15;
+
+  gfx::FontList font_list = ui::ResourceBundle::GetSharedInstance().GetFontList(
+      ui::ResourceBundle::BaseFont);
+  return base::SysUTF16ToNSString(gfx::ElideText(
+      name,
+      font_list,
+      font_list.GetExpectedTextWidth(kMaxCharactersToDisplay),
+      gfx::ELIDE_AT_END));
+}
+
+}  // namespace
+
+@interface NewAvatarButtonController (Private)
+// Shows the ProfileMenuController.
+- (IBAction)buttonClicked:(id)sender;
+
+- (void)bubbleWillClose:(NSNotification*)notif;
+
+// Updates the profile name displayed by the avatar button. If |layoutParent| is
+// yes, then the BrowserWindowController is notified to relayout the subviews,
+// as the button needs to be repositioned.
+- (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent;
+@end
+
+class ProfileInfoUpdateObserverBridge : public content::NotificationObserver {

[Nico, 2014/01/07 23:20:33]

In general, we try to not use notifications in new code. Since
NOTIFICATION_PROFILE_CACHED_INFO_CHANGED isn't new, it's ok though I think.

(It'd be nicer if the NotificationObserver was hidden somewhere in profile code
and was identical for all platforms, and then it'd call methods on a narrow
delegate interface which the per-platform uis then implemen)

[noms (inactive), 2014/01/08 15:05:31]

I can also make this an AvatarMenuObserver (like the views code does), and get
notified that way. Would that be better?

 On 2014/01/07 23:20:33, Nico wrote:

+ public:
+  ProfileInfoUpdateObserverBridge(NewAvatarButtonController* avatarButton)
+      : avatarButton_(avatarButton) {
+    registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
+                   content::NotificationService::AllSources());
+  }
+
+  virtual ~ProfileInfoUpdateObserverBridge() {
+    registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED,
+                   content::NotificationService::AllSources());

[Nico, 2014/01/07 23:20:33]

Do you need to do this? I thought the point of NotificationRegistrar is that it
removes all notifications that it added on destruction automatically.

+  }
+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE {
+    switch (type) {
+      case chrome::NOTIFICATION_PROFILE_CACHED_INFO_CHANGED:
+        [avatarButton_ updateAvatarButtonAndLayoutParent:YES];
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
+
+ private:
+  content::NotificationRegistrar registrar_;
+
+  NewAvatarButtonController* avatarButton_;  // Weak; owns this.
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileInfoUpdateObserverBridge);
+};
+
+@implementation NewAvatarButtonController
+
+- (id)initWithBrowser:(Browser*)browser {
+  if ((self = [super init])) {
+    browser_ = browser;
+    profileInfoObserver_.reset(new ProfileInfoUpdateObserverBridge(self));
+
+    base::scoped_nsobject<NSView> container([[NSView alloc]
+        initWithFrame:NSZeroRect]);
+    [self setView:container];
+
+    button_.reset([[NSButton alloc] initWithFrame:NSZeroRect]);
+    [button_ setBezelStyle:NSTexturedRoundedBezelStyle];
+    [button_ setImage:ui::ResourceBundle::GetSharedInstance().
+        GetNativeImageNamed(IDR_APP_DROPARROW).ToNSImage()];
+    [button_ setImagePosition:NSImageRight];
+    [button_ setAutoresizingMask:NSViewMinXMargin | NSViewMinYMargin];
+    [button_ setTarget:self];
+    [button_ setAction:@selector(buttonClicked:)];
+
+    [self updateAvatarButtonAndLayoutParent:NO];
+
+    [[self view] addSubview:button_];

[Nico, 2014/01/07 23:20:33]

Since this is the only view, can button_ just be the [self view] itself? Do you
need the container?

[noms (inactive), 2014/01/08 15:05:31]

Not really. The only reason I added the container is to match the
AvatarButtonController implementation, so that the outside code used them in
identical ways. Do you think that's worth it?

On 2014/01/07 23:20:33, Nico wrote:

+  }
+  return self;
+}
+
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter]
+      removeObserver:self
+                name:NSWindowWillCloseNotification
+              object:[menuController_ window]];
+  [super dealloc];
+}
+
+- (NSButton*)buttonView {
+  return button_.get();
+}
+
+- (void)showAvatarBubble:(NSView*)anchor {
+  if (menuController_)
+    return;
+
+  DCHECK(chrome::IsCommandEnabled(browser_, IDC_SHOW_AVATAR_MENU));
+
+  NSWindowController* wc =
+      [browser_->window()->GetNativeWindow() windowController];
+  if ([wc isKindOfClass:[BrowserWindowController class]]) {
+    [static_cast<BrowserWindowController*>(wc)
+        lockBarVisibilityForOwner:self withAnimation:NO delay:NO];
+  }
+
+  NSPoint point = NSMakePoint(NSMidX([anchor bounds]),
+                              NSMaxY([anchor bounds]));
+  point = [anchor convertPoint:point toView:nil];
+  point = [[anchor window] convertBaseToScreen:point];
+
+  menuController_ = [[ProfileChooserController alloc] initWithBrowser:browser_
+                                                           anchoredAt:point];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:self
+         selector:@selector(bubbleWillClose:)
+             name:NSWindowWillCloseNotification
+           object:[menuController_ window]];
+  [menuController_ showWindow:self];
+
+  ProfileMetrics::LogProfileOpenMethod(ProfileMetrics::ICON_AVATAR_BUBBLE);
+}
+
+- (IBAction)buttonClicked:(id)sender {
+  DCHECK(sender == button_.get());
+  [self showAvatarBubble:button_];
+}
+
+- (void)bubbleWillClose:(NSNotification*)notif {
+  NSWindowController* wc =
+      [browser_->window()->GetNativeWindow() windowController];
+  if ([wc isKindOfClass:[BrowserWindowController class]]) {
+    [static_cast<BrowserWindowController*>(wc)
+        releaseBarVisibilityForOwner:self withAnimation:YES delay:NO];
+  }
+  menuController_ = nil;
+}
+
+- (void)updateAvatarButtonAndLayoutParent:(BOOL)layoutParent {
+  [button_ setTitle:GetElidedProfileName(
+      profiles::GetActiveProfileDisplayName(browser_))];
+  [button_ sizeToFit];
+
+  // Resize the container.
+  [[self view] setFrameSize:[button_ frame].size];
+  [button_ setFrameOrigin:NSMakePoint(0, 0)];
+
+  if (layoutParent) {
+    // Because the width of the button might have changed, the parent browser
+    // frame needs to recalculate the button bounds and redraw it.
+    [[BrowserWindowController
+        browserWindowControllerForWindow:browser_->window()->GetNativeWindow()]
+        layoutSubviews];
+  }
+}
+
+- (ProfileChooserController*)menuController {
+  return menuController_;
+}
+
+@end