Add quit item to app menu.

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

Index: chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm
diff --git a/chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm b/chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm
index 69622b078eb9f623d25e406297505324e321ed93..440a047b19b0d2c301d9eab131bdcec033379077 100644
--- a/chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm
+++ b/chrome/browser/ui/cocoa/apps/app_menu_controller_mac.mm
@@ -8,8 +8,10 @@
 #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"
 
@@ -25,6 +27,7 @@
 // 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
@@ -43,10 +46,19 @@
 }
 
 - (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];
 }
 
@@ -103,10 +115,23 @@
   [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_];
@@ -124,6 +149,17 @@
   // 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