Fixed "Close Window" shortcut in File Menu for Hosted/Packaged apps.

chrome/browser/ui/cocoa/apps/app_shim_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_shim_menu_controller_mac.h"
 
 #include "base/mac/scoped_nsautorelease_pool.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
@@ skipping 280 matching lines @@
   [appMenu addItem:[NSMenuItem separatorItem]];
   [appMenu addItem:[hideDoppelganger_ menuItem]];
   [appMenu addItem:[NSMenuItem separatorItem]];
   [appMenu addItem:[quitDoppelganger_ menuItem]];
 
   // File menu.
   fileMenuItem_.reset([NewTopLevelItemFrom(IDC_FILE_MENU) retain]);
   [[fileMenuItem_ submenu] addItem:[newDoppelganger_ menuItem]];
   [[fileMenuItem_ submenu] addItem:[openDoppelganger_ menuItem]];
   [[fileMenuItem_ submenu] addItem:[NSMenuItem separatorItem]];
-  AddDuplicateItem(fileMenuItem_, IDC_FILE_MENU, IDC_CLOSE_WINDOW);
-  // Set the expected key equivalent explicitly here because
-  // -[AppControllerMac adjustCloseWindowMenuItemKeyEquivalent:] sets it to
-  // "W" (Cmd+Shift+w) when a tabbed window has focus; it will change it back
-  // to Cmd+w when a non-tabbed window has focus.
-  NSMenuItem* closeWindowMenuItem =
-      [[fileMenuItem_ submenu] itemWithTag:IDC_CLOSE_WINDOW];
-  [closeWindowMenuItem setKeyEquivalent:@"w"];
-  [closeWindowMenuItem setKeyEquivalentModifierMask:NSCommandKeyMask];
+
+  // Since the "Close Window" menu item will have the same shortcut as "Close
+  // Tab" on the Chrome menu, we need to swap out these modifier keys
+  // depending on the current menu we're displaying.
+  closeWindowDuplicateMenuItem_.reset(
+      [GetItemByTag(IDC_FILE_MENU, IDC_CLOSE_WINDOW) copy]);
+  closeTabSourceMenuItem_.reset(
+      [GetItemByTag(IDC_FILE_MENU, IDC_CLOSE_TAB) retain]);
+  [[fileMenuItem_ submenu] addItem:closeWindowDuplicateMenuItem_];
 
   // Edit menu. We copy the menu because the last two items, "Start Dictation"
   // and "Special Characters" are added by OSX, so we can't copy them
   // explicitly.
   editMenuItem_.reset([[[NSApp mainMenu] itemWithTag:IDC_EDIT_MENU] copy]);
 
   // View menu. Remove "Always Show Bookmark Bar" and separator.
   viewMenuItem_.reset([[[NSApp mainMenu] itemWithTag:IDC_VIEW_MENU] copy]);
   RemoveMenuItemWithTag(viewMenuItem_, IDC_SHOW_BOOKMARK_BAR, YES);
 
@@ skipping 84 matching lines @@
 
   [aboutDoppelganger_ enableForApp:app];
   [hideDoppelganger_ enableForApp:app];
   [quitDoppelganger_ enableForApp:app];
   [newDoppelganger_ enableForApp:app];
   [openDoppelganger_ enableForApp:app];
 
   [appMenuItem_ setTitle:appId];
   [[appMenuItem_ submenu] setTitle:title];
 
+  // Remove the "Close Tab" key equivalent in the main menu first, then set
+  // the key equivalent for our copy.
+  [closeTabSourceMenuItem_ setKeyEquivalent:@""];

[tapted, 2015/02/04 05:02:49]

this logic is pretty close to what DoppelgangerMenuItem does -- is there a way
to re-use that? Maybe just pass a nil controller and, in initWithController,
copy the target from |sourceItem_| if the controller is nil.

[jackhou1, 2015/02/04 05:46:28]

The "Close Window" item kind of has two source items: we get the string and
target from "Close Window", and clear the key equivalent in "Close Tab".

Mitchell, you could try adding another initializer to AppShimMenuDoppelganger.
Something like:
- (id)initWithMenuItem:(NSMenuItem*) sourceItem:(NSMenuItem*);

Then you pass in:
GetItemByTag(IDC_FILE_MENU, IDC_CLOSE_WINDOW) copy]
and
GetItemByTag(IDC_FILE_MENU, IDC_CLOSE_TAB) retain]

[mitchellj, 2015/02/04 22:33:40]

Good idea, this seems cleaner. Do you think the function should take these
NSMenuItems directly though? Or the tags?

I was thinking something like

- (id)initWithMenuTag:(NSInteger)menuTag
              itemTag:(NSInteger)itemTag
        sourceMenuTag:(NSInteger)sourceMenuTag
        sourceItemTag:(NSInteger)sourceItemTag
        keyEquivalent:(NSString*)keyEquivalent;

And I could just mention something along the lines: "Copy the menu item given
|menuTag| and |itemTag|. Retain the source item given |sourceMenuTag| and
|sourceItemTag|."

+
+  // Set the expected key equivalent explicitly here because it may have been
+  // either Cmd+Shift+w or Cmd+w.
+  // See -[AppControllerMac adjustCloseWindowMenuItemKeyEquivalent:].
+  [closeWindowDuplicateMenuItem_ setKeyEquivalent:@"w"];
+  [closeWindowDuplicateMenuItem_
+      setKeyEquivalentModifierMask:NSCommandKeyMask];
+
   [mainMenu addItem:appMenuItem_];
   [mainMenu addItem:fileMenuItem_];
 
   SetItemWithTagVisible(editMenuItem_,
                         IDC_CONTENT_CONTEXT_PASTE_AND_MATCH_STYLE,
                         app->is_hosted_app(), true);
   SetItemWithTagVisible(editMenuItem_, IDC_FIND_MENU, app->is_hosted_app(),
                         false);
   [mainMenu addItem:editMenuItem_];
 
   if (app->is_hosted_app()) {
     [mainMenu addItem:viewMenuItem_];
     [mainMenu addItem:historyMenuItem_];
   }
   [mainMenu addItem:windowMenuItem_];
 }
 
 - (void)removeMenuItems {
   if (!appId_)
     return;
 
   appId_.reset();
 
+  // Restore the "Close Tab" key equivalent in the main menu.
+  [closeWindowDuplicateMenuItem_ setKeyEquivalent:@""];
+  [closeTabSourceMenuItem_ setKeyEquivalent:@"w"];
+  [closeTabSourceMenuItem_ setKeyEquivalentModifierMask:NSCommandKeyMask];
+
   NSMenu* mainMenu = [NSApp mainMenu];
   [mainMenu removeItem:appMenuItem_];
   [mainMenu removeItem:fileMenuItem_];
   if ([mainMenu indexOfItem:viewMenuItem_] >= 0)
     [mainMenu removeItem:viewMenuItem_];
   if ([mainMenu indexOfItem:historyMenuItem_] >= 0)
     [mainMenu removeItem:historyMenuItem_];
   [mainMenu removeItem:editMenuItem_];
   [mainMenu removeItem:windowMenuItem_];
 
@@ skipping 42 matching lines @@
 
 - (void)focusCurrentPlatformApp {
   extensions::AppWindow* appWindow =
       AppWindowRegistryUtil::GetAppWindowForNativeWindowAnyProfile(
           [NSApp keyWindow]);
   if (appWindow)
     apps::ExtensionAppShimHandler::FocusAppForWindow(appWindow);
 }
 
 @end