[Mac] User manager should show up as a standalone window.

chrome/browser/ui/cocoa/user_manager_mac.mm

Index: chrome/browser/ui/cocoa/user_manager_mac.mm
diff --git a/chrome/browser/ui/cocoa/user_manager_mac.mm b/chrome/browser/ui/cocoa/user_manager_mac.mm
index 07818f33ff8d69615ef679d3fb33e91002e72208..0e8d696c4447875adf57003ce9242e5600ce4a2a 100644
--- a/chrome/browser/ui/cocoa/user_manager_mac.mm
+++ b/chrome/browser/ui/cocoa/user_manager_mac.mm
@@ -2,20 +2,182 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import <Cocoa/Cocoa.h>
+#include "chrome/browser/ui/cocoa/user_manager_mac.h"
 
+#include "base/strings/string_number_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#include "chrome/common/url_constants.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_view.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util_mac.h"
+
+namespace {

[Nico, 2013/12/12 18:05:40]

nit: const has implicit internal linkage, no need for the unnamed namespace

[noms (inactive), 2013/12/12 22:59:23]

Done.

+
+// Default window size. Taken from the views implementation in
+// chrome/browser/ui/views/user_manager_view.cc.
+// TODO(noms): Figure out if this size can be computed dynamically or adjusted
+// for smaller screens.
+const int kWindowWidth = 900;
+const int kWindowHeight = 700;
+
+}  // namespace
 
 namespace chrome {
 
 // Declared in browser_dialogs.h so others don't have to depend on this header.
-// TODO(noms): Add implementation when the User Manager dialog is implemented.
 void ShowUserManager(const base::FilePath& profile_path_to_focus) {
-  NOTIMPLEMENTED();
+  UserManagerMac::Show(profile_path_to_focus);
 }
 
 void HideUserManager() {
-  NOTIMPLEMENTED();
+  UserManagerMac::Hide();
 }
 
 }  // namespace chrome
+
+@interface UserManagerWindowController (Private)
+- (void)windowWillClose:(NSNotification*)notification;
+- (void)dealloc;
+@end
+
+@implementation UserManagerWindowController
+
+- (id)initWithProfile:(Profile*)profile
+         withObserver:(UserManagerMac*)userManagerObserver {
+
+  // Center the window on the primary screen.
+  CGFloat screenHeight =
+      [[[NSScreen screens] objectAtIndex:0] frame].size.height;
+  CGFloat screenWidth =
+      [[[NSScreen screens] objectAtIndex:0] frame].size.width;
+
+  NSRect contentRect = NSMakeRect((screenWidth - kWindowWidth) / 2,
+                                  (screenHeight - kWindowHeight) / 2,
+                                  kWindowWidth, kWindowHeight);
+
+  userManagerObserver_ = userManagerObserver;
+  NSWindow* window = [[NSWindow alloc]
+      initWithContentRect:contentRect
+                styleMask:NSTitledWindowMask |
+                          NSClosableWindowMask |
+                          NSResizableWindowMask
+                  backing:NSBackingStoreBuffered
+                    defer:NO];
+  [window setTitle:l10n_util::GetNSString(IDS_USER_MANAGER_SCREEN_TITLE)];
+
+  // Initialize the web view.
+  webContents_.reset(content::WebContents::Create(

[Nico, 2013/12/12 18:05:40]

This is technically not correct (the best kind of not correct?). webContents_ is
an instance variable, so this line really is

  self->webContents_…

and self isn't really initialized yet – or at least the call to [super
initWithWindow:] could change it. The usual pattern for object initialization we
use in chrome is

- (id)init… {
  // Initialize stack-local variables that are required to call [super init…]
  // (in this case, |window|
  if ((self = [super init…])) {
    // initialize instance variables, do other initialization stuff
  }
  return self;
}


(It probably doesn't make a huge difference in practice in this case, but it's
consistent with other classes)

[noms (inactive), 2013/12/12 22:59:23]

Eeeek. Thank you. Fixed and won't do again.
On 2013/12/12 18:05:40, Nico wrote:

+      content::WebContents::CreateParams(profile)));
+  window.contentView = webContents_->GetView()->GetNativeView();
+  DCHECK(window.contentView);
+
+  // Initialize, but don't show the window until the web contents have loaded.
+  self = [super initWithWindow:window];
+
+  [[NSNotificationCenter defaultCenter] addObserver:self
+                                         selector:@selector(windowWillClose:)
+                                             name:NSWindowWillCloseNotification
+                                           object:self.window];
+  return self;
+}
+
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [super dealloc];
+}
+
+- (void)showURL:(const GURL&)url {
+  webContents_->GetController().LoadURL(url, content::Referrer(),
+                                        content::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                        std::string());
+  [self show];
+}
+
+- (void)show {
+  [[self window] makeKeyAndOrderFront:self];
+}
+
+- (void)close {
+  [[self window] close];
+}
+
+-(BOOL)isVisible {
+  return [[self window] isVisible];
+}
+
+- (void)windowWillClose:(NSNotification*)notification {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  DCHECK(userManagerObserver_);
+  userManagerObserver_->WindowWasClosed();
+}
+
+@end
+
+// static
+UserManagerMac* UserManagerMac::instance_ = NULL;
+
+UserManagerMac::UserManagerMac(Profile* profile) {
+  window_controller_.reset([[UserManagerWindowController alloc]
+      initWithProfile:profile withObserver:this]);
+}
+
+UserManagerMac::~UserManagerMac() {
+}
+
+// static
+void UserManagerMac::Show(const base::FilePath& profile_path_to_focus) {
+  if (instance_) {
+    [instance_->window_controller_ show];
+    return;
+  }
+
+  // Create the guest profile, if necessary, and open the User Manager
+  // from the guest profile.
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  profile_manager->CreateProfileAsync(
+      ProfileManager::GetGuestProfilePath(),
+      base::Bind(&UserManagerMac::OnGuestProfileCreated,
+                 profile_path_to_focus),
+      string16(),
+      string16(),
+      std::string());
+}
+
+// static
+void UserManagerMac::Hide() {
+  if (instance_)
+    [instance_->window_controller_ close];
+}
+
+void UserManagerMac::WindowWasClosed() {
+  delete this;
+  instance_ = NULL;

[Nico, 2013/12/12 18:05:40]

This too writes to a member variable, so this is really

  this->instance_ = NULL;

Doing this right after deleting this seems optimistic :-)

[noms (inactive), 2013/12/12 22:59:23]

OH I see. I herp derped this one. Also, the TaskManager where I copied this from
does the same thing, so I'll fix it there too.
On 2013/12/12 18:05:40, Nico wrote:

[Nico, 2013/12/12 23:04:09]

The Mac TaskManager was written by someone who had no idea what they were doing,
don't trust that code. (But chrome/browser/ui/cocoa/task_manager_mac_unittest.mm
has an example of a unit test for this type of code, maybe that could be used as
inspiration?)

[Nico, 2013/12/17 17:27:52]

The person who wrote the mac task manager reached out to me yesterday and
politely reminded me that "instance_" is a static variable, not a member, so
assigning to it after deleting |this| is perfectly safe and this was ok as it
was (but so is assigning to it before deleting |this|, so as it is now is ok
too).

+}
+
+void UserManagerMac::OnGuestProfileCreated(
+    const base::FilePath& profile_path_to_focus,
+    Profile* guest_profile,
+    Profile::CreateStatus status) {
+  if (status != Profile::CREATE_STATUS_INITIALIZED)
+    return;
+
+  instance_ = new UserManagerMac(guest_profile);
+
+  // Tell the webui which user pod should be focused.
+  std::string page = chrome::kChromeUIUserManagerURL;
+
+  if (!profile_path_to_focus.empty()) {
+    ProfileInfoCache& cache =
+        g_browser_process->profile_manager()->GetProfileInfoCache();
+    size_t index = cache.GetIndexOfProfileWithPath(profile_path_to_focus);
+    if (index != std::string::npos) {
+      page += "#";
+      page += base::IntToString(index);
+    }
+  }
+  [instance_->window_controller_ showURL:GURL(page)];
+}
+