[Mac] Cmd+W should close the User Manager

chrome/browser/ui/cocoa/profiles/user_manager_mac.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.
 
 #include "chrome/browser/ui/cocoa/profiles/user_manager_mac.h"
 
+#include "base/mac/foundation_util.h"
+#include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
+#import "chrome/browser/ui/cocoa/browser_window_utils.h"
+#include "chrome/browser/ui/cocoa/chrome_event_processing_window.h"
+#include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
 // 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 chrome {
 
 // Declared in browser_dialogs.h so others don't have to depend on this header.
 void ShowUserManager(const base::FilePath& profile_path_to_focus) {
   UserManagerMac::Show(
       profile_path_to_focus, profiles::USER_MANAGER_NO_TUTORIAL);
 }
 
 void ShowUserManagerWithTutorial(profiles::UserManagerTutorialMode tutorial) {
   UserManagerMac::Show(base::FilePath(), tutorial);
 }
 
 void HideUserManager() {
   UserManagerMac::Hide();
 }
 
 }  // namespace chrome
 
+// Custom WebContentsDelegate that allows handling of hotkeys.
+class UserManagerWebContentsDelegate : public content::WebContentsDelegate {
+ public:
+  UserManagerWebContentsDelegate(NSWindow* window) : window_(window) {}

[groby-ooo-7-16, 2014/07/08 23:31:28]

Make the ctor take a ChromeEventProcessingWindow...

[noms (inactive), 2014/07/09 18:06:55]

Done.

+
+  // WebContentsDelegate implementation. Forwards all unhandled keyboard events
+  // to the current window.
+  virtual void HandleKeyboardEvent(
+      content::WebContents* source,
+      const content::NativeWebKeyboardEvent& event) OVERRIDE {
+    // Since the User Manager is a "top level" window, only handle close events.

[groby-ooo-7-16, 2014/07/08 23:31:28]

Really? No other shortcuts? Quit?

[noms (inactive), 2014/07/09 18:06:55]

I guess that would make sense. No other shortcuts though! The user manager can
be the only top level window, and it doesn't look like a normal browser window,
so people shouldn't think of it as being able to do anything in it other than
launch a profile.

I will add Quit, though.

On 2014/07/08 23:31:28, groby wrote:

+    if (![BrowserWindowUtils shouldHandleKeyboardEvent:event] ||
+        [BrowserWindowUtils getCommandId:event] != IDC_CLOSE_WINDOW)

[groby-ooo-7-16, 2014/07/08 23:31:28]

getCommandId != -1, maybe? Or skip this entirely, and just redispatch all key
events?

(I'm not sure if that'll work. jdduke touched the RenderWidgetHostImpl keyboard
handling last, so I'll loop him in)

[noms (inactive), 2014/07/09 18:06:55]

Hmm, I may be misreading this, but wouldn't redispatching all the events allow
all sort of Cmd+t and their brethren through? Currently the User Manager handles
(by design) no accelerators -- I'd just like people to be able to close it like
a tab, but not do anything else...

I refactored this a bit to be more readable (same functionality)

On 2014/07/08 23:31:28, groby wrote:

[groby-ooo-7-16, 2014/07/09 19:48:13]

That's really odd UI behavior. You're essentially making this into an app-modal
dialog. I don't believe there are _any_ app-modal dialogs in Chrome right now.
Why does the user manager qualify for that status?
I don't think so. |window_| is a top-level window. Accelerator events it doesn't
handle are dispatched to the menu. That means Cmd-T will do nothing

[noms (inactive), 2014/07/09 21:08:14]

Trying to soothe some worries: It's not app modal. It basically looks like a
browser window that's also opened (doesn't steal focus, doesn't close any
existing browser windows), but it has a separate, special
underlying profile, which can't really be used. The user manager is only
used to select different profiles (think of the cros login screen), so
it makes no sense to allow tab opening/bookmarks/etc, since it's not
meant to be a browser window...

Also, in particular, this CL doesn't take away functionality from anything
existent, it adds a little bit :)

On 2014/07/09 19:48:13, groby wrote:

+      return;
+
+    // Not invoking +[BrowserWindowUtils handleKeyboardEvent here], since the
+    // window in question is a ConstrainedWindowCustomWindow, not a
+    // BrowserWindow.
+    ChromeEventProcessingWindow* event_window =

[groby-ooo-7-16, 2014/07/08 23:31:28]

Which kills the DCHECK here.

[noms (inactive), 2014/07/09 18:06:55]

Done.

+        base::mac::ObjCCastStrict<ChromeEventProcessingWindow>(window_);
+    [event_window redispatchKeyEvent:event.os_event];
+  }
+
+ private:
+  NSWindow* window_;  // Used to redispatch key events.
+};
+
 // Window controller for the User Manager view.
 @interface UserManagerWindowController : NSWindowController <NSWindowDelegate> {
  @private
   scoped_ptr<content::WebContents> webContents_;
+  scoped_ptr<UserManagerWebContentsDelegate> webContentsDelegate_;
   UserManagerMac* userManagerObserver_;  // Weak.
 }
 - (void)windowWillClose:(NSNotification*)notification;
 - (void)dealloc;
 - (id)initWithProfile:(Profile*)profile
          withObserver:(UserManagerMac*)userManagerObserver;
 - (void)showURL:(const GURL&)url;
 - (void)show;
 - (void)close;
 - (BOOL)isVisible;
 @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);
-  NSWindow* window = [[NSWindow alloc]
+  ChromeEventProcessingWindow* window = [[ChromeEventProcessingWindow alloc]
       initWithContentRect:contentRect
                 styleMask:NSTitledWindowMask |
                           NSClosableWindowMask |
                           NSResizableWindowMask
                   backing:NSBackingStoreBuffered
                     defer:NO];
   [window setTitle:l10n_util::GetNSString(IDS_USER_MANAGER_SCREEN_TITLE)];
   [window setMinSize:NSMakeSize(kWindowWidth, kWindowHeight)];
 
   if ((self = [super initWithWindow:window])) {
     userManagerObserver_ = userManagerObserver;
 
     // Initialize the web view.
     webContents_.reset(content::WebContents::Create(
         content::WebContents::CreateParams(profile)));
     window.contentView = webContents_->GetNativeView();
+    webContentsDelegate_.reset(
+        new UserManagerWebContentsDelegate([self window]));
+    webContents_->SetDelegate(webContentsDelegate_.get());
     DCHECK(window.contentView);
 
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(windowWillClose:)
                name:NSWindowWillCloseNotification
              object:self.window];
   }
   return self;
 }
@@ skipping 73 matching lines @@
 void UserManagerMac::OnGuestProfileCreated(Profile* guest_profile,
                                            const std::string& url) {
   instance_ = new UserManagerMac(guest_profile);
   [instance_->window_controller_ showURL:GURL(url)];
 }
 
 void UserManagerMac::WindowWasClosed() {
   instance_ = NULL;
   delete this;
 }