Add quit item to app menu.

chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm

 // Copyright 2013 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/apps/app_menu_controller_mac.h"
 
 #include "apps/app_shim/extension_app_shim_handler_mac.h"
 #include "apps/shell_window.h"
 #include "apps/shell_window_registry.h"
 #include "base/strings/sys_string_conversions.h"
+#include "base/strings/utf_string_conversions.h"
 #import "chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h"
 #include "chrome/common/extensions/extension.h"
+#include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
 @interface AppMenuController ()
 // Construct the NSMenuItems for apps.
 - (void)buildAppMenuItems;
 // Register for NSWindow notifications.
 - (void)registerEventHandlers;
 // If the window is an app window, add or remove menu items.
 - (void)windowMainStatusChanged:(NSNotification*)notification;
 // Add menu items for an app and hide Chrome menu items.
 - (void)addMenuItems:(const extensions::Extension*)app;
 // If the window belongs to the currently focused app, remove the menu items and
 // unhide Chrome menu items.
 - (void)removeMenuItems:(NSString*)appId;
+- (void)quitCurrentPlatformApp;
 @end
 
 @implementation AppMenuController
 
 - (id)init {
   if ((self = [super init])) {
     [self buildAppMenuItems];
     [self registerEventHandlers];
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
 
 - (void)buildAppMenuItems {
+  quitChromeItem_ = [[[[[NSApp mainMenu] itemAtIndex:0] submenu] itemArray]

[tapted, 2013/08/22 04:40:06]

maybe chromeMenuQuitItem_ - keeps it consistent with appQuitItem below.

[jackhou1, 2013/08/22 06:41:01]

Done.

+      lastObject];

[tapted, 2013/08/22 04:40:06]

This might be a bit fragile. I think we need to look up similar to how it's done
in [ConfirmQuitPanelController quitAccelerator]. i.e. iterate through the first
submenu and look for ([item action] == @selector(terminate:))

[jackhou1, 2013/08/22 06:41:01]

Done.

+
   appMenuItem_.reset([[NSMenuItem alloc] initWithTitle:@""
                                                 action:nil
                                          keyEquivalent:@""]);
   base::scoped_nsobject<NSMenu> appMenu([[NSMenu alloc] initWithTitle:@""]);
+  [appMenu setAutoenablesItems:NO];
+  NSMenuItem* appQuitItem = [appMenu addItemWithTitle:@""

[tapted, 2013/08/22 04:40:06]

nit: maybe appMenuQuitItem?

[jackhou1, 2013/08/22 06:41:01]

Done.

+                                               action:nil
+                                        keyEquivalent:@""];
+  [appQuitItem setTarget:self];
+  [appQuitItem setEnabled:YES];

[tapted, 2013/08/22 04:40:06]

is this line needed?

[jackhou1, 2013/08/22 06:41:01]

Apparently not.

   [appMenuItem_ setSubmenu:appMenu];
 }
 
 - (void)registerEventHandlers {
   [[NSNotificationCenter defaultCenter]
       addObserver:self
          selector:@selector(windowMainStatusChanged:)
              name:NSWindowDidBecomeMainNotification
            object:nil];
 
@@ skipping 36 matching lines @@
 - (void)addMenuItems:(const extensions::Extension*)app {
   NSString* appId = base::SysUTF8ToNSString(app->id());
   NSString* title = base::SysUTF8ToNSString(app->name());
 
   if ([appId_ isEqualToString:appId])
     return;
 
   [self removeMenuItems:appId_];
   appId_.reset([appId copy]);
 
+  // Hide Chrome menu items.
   NSMenu* mainMenu = [NSApp mainMenu];
   for (NSMenuItem* item in [mainMenu itemArray])
     [item setHidden:YES];
 
+  // The action on the existing "Quit Chrome" item needs to be changed.
+  // This is so that the "Quit <app>" item to the app's menu can have the same
+  // keyboard shortcut.
+  [quitChromeItem_ setAction:@selector(quitCurrentPlatformApp)];

[tapted, 2013/08/22 04:40:06]

With the current code, can they both stay as terminate:? Since, we handle that
correctly now (right?).

This feels a little weird, because it doesn't also update the target of
Chrome->Quit, so if the action is ever applied to the item's target, Chrome will
crash.

[jackhou1, 2013/08/22 06:41:01]

Yeah, I don't really like it either. I was hoping to move the logic in
app_controller_mac into here. If they're both set to terminate:, clicking the
item will terminate Chrome.

I've changed this to unset the key equivalent instead of setting the action.
This simply disables the keyboard shortcut, which should be fine since it
shouldn't be activated while an app window is focused anyway.

[tapted, 2013/08/22 07:20:15]

Ahhhh - clicking. What if you just remove the condition

  if ([[app currentEvent] type] == NSKeyDown) {

from -
(NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication*)app in
app_controller_mac.mm? Would that let the App menu item work with terminate:
when clicked?

But if that doesn't work (and quitCurrentPlatformApp is used instead), then that
block in applicationShouldTerminate: that checks for a shellwindow should be
removed in this CL, since it should never be hit.

But, I'm actually not sure which alternative is better - this might be the time
to consult one of the Chrome mac gurus.

+
+  NSString* quit_localized_string =

[tapted, 2013/08/22 04:40:06]

maybe localizedQuitApp ? (should be camel case, since we're in an ObjC member)

[jackhou1, 2013/08/22 06:41:01]

Done.

+      l10n_util::GetNSStringF(IDS_EXIT_MAC, base::UTF8ToUTF16(app->name()));
+  NSMenuItem* appQuitItem = [[[appMenuItem_ submenu] itemArray] lastObject];
+  [appQuitItem setTitle:quit_localized_string];
+  [appQuitItem setAction:[quitChromeItem_ action]];
+  [appQuitItem setKeyEquivalent:[quitChromeItem_ keyEquivalent]];

[tapted, 2013/08/22 04:40:06]

should you also setKeyEquivalentModifierMask: ?

[jackhou1, 2013/08/22 06:41:01]

Done.

+
   [appMenuItem_ setTitle:appId];
   [[appMenuItem_ submenu] setTitle:title];
   [mainMenu addItem:appMenuItem_];
 }
 
 - (void)removeMenuItems:(NSString*)appId {
   if (![appId_ isEqualToString:appId])
     return;
 
   appId_.reset();
 
   NSMenu* mainMenu = [NSApp mainMenu];
   [mainMenu removeItem:appMenuItem_];
 
   // Restore the Chrome main menu bar.
   for (NSMenuItem* item in [mainMenu itemArray])
     [item setHidden:NO];
+
+  [quitChromeItem_ setAction:@selector(terminate:)];
+}
+
+// If the currently focused window belongs to a platform app, quit the app.

[tapted, 2013/08/22 04:40:06]

move this comment to the declaration? (if we keep this function at all ;)

[jackhou1, 2013/08/22 06:41:01]

Done.

+- (void)quitCurrentPlatformApp {
+  apps::ShellWindow* shellWindow =
+      apps::ShellWindowRegistry::GetShellWindowForNativeWindowAnyProfile(
+          [NSApp keyWindow]);
+  if (shellWindow)
+    apps::ExtensionAppShimHandler::QuitAppForWindow(shellWindow);
 }
 
 @end