Fix issue where cmd-w was hard-coded to closing a browser tab regardless of the frontmost window type. Have cmd-key equiv correctly set depending on the window type and the number of tabs in the window. Broadcast notification when the number of tabs changes in the model. Disable "close tab" item when there's only 1 tab in the browser window.

BUG=10047
TEST=cmd-w correctly closes the expected thing (frontmost window, or tab in the frontmost window). close tab should be disabled when the frontmost tab is not a browser or if there is only 1 tab in the window.
Review URL: http://codereview.chromium.org/115789

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16981 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 182 insertions, 47 deletions

diff --git a/chrome/app/nibs/en.lproj/MainMenu.xib b/chrome/app/nibs/en.lproj/MainMenu.xib
index 1e452d8..e0a917c 100644
--- a/chrome/app/nibs/en.lproj/MainMenu.xib
+++ b/chrome/app/nibs/en.lproj/MainMenu.xib
@@ -8,7 +8,7 @@
 		<string key="IBDocument.HIToolboxVersion">352.00</string>
 		<object class="NSMutableArray" key="IBDocument.EditedObjectIDs">
 			<bool key="EncodedWithXMLCoder">YES</bool>
-			<integer value="111"/>
+			<integer value="81"/>
 		</object>
 		<object class="NSArray" key="IBDocument.PluginDependencies">
 			<bool key="EncodedWithXMLCoder">YES</bool>
@@ -305,7 +305,8 @@
 								<object class="NSMenuItem" id="776162233">
 									<reference key="NSMenu" ref="720053764"/>
 									<string key="NSTitle">Close Window</string>
-									<string key="NSKeyEquiv"/>
+									<string key="NSKeyEquiv">w</string>
+									<int key="NSKeyEquivModMask">1048576</int>
 									<int key="NSMnemonicLoc">2147483647</int>
 									<reference key="NSOnImage" ref="353210768"/>
 									<reference key="NSMixedImage" ref="549394948"/>
@@ -313,8 +314,7 @@
 								<object class="NSMenuItem" id="1059729334">
 									<reference key="NSMenu" ref="720053764"/>
 									<string key="NSTitle">Close Tab</string>
-									<string key="NSKeyEquiv">w</string>
-									<int key="NSKeyEquivModMask">1048576</int>
+									<string key="NSKeyEquiv"/>
 									<int key="NSMnemonicLoc">2147483647</int>
 									<reference key="NSOnImage" ref="353210768"/>
 									<reference key="NSMixedImage" ref="549394948"/>
@@ -1573,6 +1573,22 @@
 					<int key="connectionID">615</int>
 				</object>
 				<object class="IBConnectionRecord">
+					<object class="IBOutletConnection" key="connection">
+						<string key="label">closeWindowMenuItem_</string>
+						<reference key="source" ref="168151378"/>
+						<reference key="destination" ref="776162233"/>
+					</object>
+					<int key="connectionID">622</int>
+				</object>
+				<object class="IBConnectionRecord">
+					<object class="IBOutletConnection" key="connection">
+						<string key="label">closeTabMenuItem_</string>
+						<reference key="source" ref="168151378"/>
+						<reference key="destination" ref="1059729334"/>
+					</object>
+					<int key="connectionID">623</int>
+				</object>
+				<object class="IBConnectionRecord">
 					<object class="IBActionConnection" key="connection">
 						<string key="label">orderFrontStandardAboutPanel:</string>
 						<reference key="source" ref="168151378"/>
@@ -2669,7 +2685,7 @@
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<integer value="1" id="9"/>
-					<string>{{929, 698}, {190, 23}}</string>
+					<string>{{715, 938}, {190, 23}}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<reference ref="9"/>
 					<string>{{596, 852}, {216, 23}}</string>
@@ -2779,7 +2795,7 @@
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<reference ref="9"/>
 					<string>{{525, 802}, {197, 73}}</string>
-					<string>{{398, 662}, {535, 20}}</string>
+					<string>{{539, 1064}, {535, 20}}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<reference ref="9"/>
 					<string>{74, 862}</string>
@@ -2879,7 +2895,7 @@
 					<reference ref="9"/>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<reference ref="9"/>
-					<string>{{561, 418}, {249, 303}}</string>
+					<string>{{645, 761}, {249, 303}}</string>
 					<string>com.apple.InterfaceBuilder.CocoaPlugin</string>
 					<reference ref="9"/>
 					<string>{{323, 672}, {199, 203}}</string>
@@ -2911,49 +2927,12 @@
 				</object>
 			</object>
 			<nil key="sourceID"/>
-			<int key="maxID">619</int>
+			<int key="maxID">623</int>
 		</object>
 		<object class="IBClassDescriber" key="IBDocument.Classes">
 			<object class="NSMutableArray" key="referencedPartialClassDescriptions">
 				<bool key="EncodedWithXMLCoder">YES</bool>
 				<object class="IBPartialClassDescription">
-					<string key="className">AboutController</string>
-					<string key="superclassName">NSWindowController</string>
-					<object class="NSMutableDictionary" key="actions">
-						<string key="NS.key.0">updateNow:</string>
-						<string key="NS.object.0">id</string>
-					</object>
-					<object class="NSMutableDictionary" key="outlets">
-						<bool key="EncodedWithXMLCoder">YES</bool>
-						<object class="NSMutableArray" key="dict.sortedKeys">
-							<bool key="EncodedWithXMLCoder">YES</bool>
-							<string>spinner_</string>
-							<string>upToDate_</string>
-							<string>updateCompleted_</string>
-							<string>version_</string>
-						</object>
-						<object class="NSMutableArray" key="dict.values">
-							<bool key="EncodedWithXMLCoder">YES</bool>
-							<string>NSProgressIndicator</string>
-							<string>NSTextField</string>
-							<string>NSTextField</string>
-							<string>NSTextField</string>
-						</object>
-					</object>
-					<object class="IBClassDescriptionSource" key="sourceIdentifier">
-						<string key="majorKey">IBProjectSource</string>
-						<string key="minorKey">browser/cocoa/about_controller.h</string>
-					</object>
-				</object>
-				<object class="IBPartialClassDescription">
-					<string key="className">AboutController</string>
-					<string key="superclassName">NSWindowController</string>
-					<object class="IBClassDescriptionSource" key="sourceIdentifier">
-						<string key="majorKey">IBUserSource</string>
-						<string key="minorKey"/>
-					</object>
-				</object>
-				<object class="IBPartialClassDescription">
 					<string key="className">AppController</string>
 					<string key="superclassName">NSObject</string>
 					<object class="NSMutableDictionary" key="actions">
@@ -2972,8 +2951,17 @@
 						</object>
 					</object>
 					<object class="NSMutableDictionary" key="outlets">
-						<string key="NS.key.0">aboutController_</string>
-						<string key="NS.object.0">AboutController</string>
+						<bool key="EncodedWithXMLCoder">YES</bool>
+						<object class="NSMutableArray" key="dict.sortedKeys">
+							<bool key="EncodedWithXMLCoder">YES</bool>
+							<string>closeTabMenuItem_</string>
+							<string>closeWindowMenuItem_</string>
+						</object>
+						<object class="NSMutableArray" key="dict.values">
+							<bool key="EncodedWithXMLCoder">YES</bool>
+							<string>NSMenuItem</string>
+							<string>NSMenuItem</string>
+						</object>
 					</object>
 					<object class="IBClassDescriptionSource" key="sourceIdentifier">
 						<string key="majorKey">IBProjectSource</string>
diff --git a/chrome/browser/app_controller_mac.h b/chrome/browser/app_controller_mac.h
index bf2aebc..e47bc00 100644
--- a/chrome/browser/app_controller_mac.h
+++ b/chrome/browser/app_controller_mac.h
@@ -34,6 +34,13 @@ class Profile;
   // only needed during early startup, it points to a valid vector during early
   // startup and is NULL during the rest of app execution.
   scoped_ptr<std::vector<GURL> > pendingURLs_;
+  
+  // Outlets for the close tab/window menu items so that we can adjust the
+  // commmand-key equivalent depending on the kind of window and how many
+  // tabs it has.
+  IBOutlet NSMenuItem* closeTabMenuItem_;
+  IBOutlet NSMenuItem* closeWindowMenuItem_;
+  BOOL fileMenuUpdatePending_;  // ensure we only do this once per notificaion.
 }
 
 - (IBAction)quit:(id)sender;
diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 436fa6c..fbf2ef0 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -17,6 +17,8 @@
 #import "chrome/browser/cocoa/encoding_menu_controller_delegate_mac.h"
 #import "chrome/browser/cocoa/menu_localizer.h"
 #import "chrome/browser/cocoa/preferences_window_controller.h"
+#import "chrome/browser/cocoa/tab_strip_controller.h"
+#import "chrome/browser/cocoa/tab_window_controller.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
@@ -31,6 +33,7 @@
      withReply:(NSAppleEventDescriptor*)reply;
 - (void)openFiles:(NSAppleEventDescriptor*)event
         withReply:(NSAppleEventDescriptor*)reply;
+- (void)windowLayeringDidChange:(NSNotification*)inNotification;
 @end
 
 @implementation AppController
@@ -56,10 +59,119 @@
         forEventClass:kCoreEventClass
            andEventID:kAEOpenDocuments];
 
+  // Register for various window layering changes. We use these to update
+  // various UI elements (command-key equivalents, etc) when the frontmost
+  // window changes.
+  NSNotificationCenter* notificationCenter =
+      [NSNotificationCenter defaultCenter];
+  [notificationCenter 
+      addObserver:self
+         selector:@selector(windowLayeringDidChange:)
+             name:NSWindowDidBecomeKeyNotification
+           object:nil];
+  [notificationCenter
+      addObserver:self
+         selector:@selector(windowLayeringDidChange:)
+             name:NSWindowDidResignKeyNotification
+           object:nil];
+  [notificationCenter
+      addObserver:self
+         selector:@selector(windowLayeringDidChange:)
+             name:NSWindowDidBecomeMainNotification
+           object:nil];
+  [notificationCenter
+      addObserver:self
+         selector:@selector(windowLayeringDidChange:)
+             name:NSWindowDidResignMainNotification
+           object:nil];
+
+  // Register for a notification that the number of tabs changes in windows
+  // so we can adjust the close tab/window command keys.
+  [notificationCenter
+      addObserver:self
+         selector:@selector(tabsChanged:)
+             name:kTabStripNumberOfTabsChanged
+           object:nil];
+
   // Set up the command updater for when there are no windows open
   [self initMenuState];
 }
 
+// Called when the app is shutting down. Clean-up as appropriate.
+- (void)applicationWillTerminate:(NSNotification *)aNotification {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+}
+
+// Helper routine to get the window controller if the key window is a tabbed
+// window, or nil if not. Examples of non-tabbed windows are "about" or
+// "preferences".
+- (TabWindowController*)keyWindowTabController {
+  NSWindowController* keyWindowController =
+      [[[NSApplication sharedApplication] keyWindow] windowController];
+  if ([keyWindowController isKindOfClass:[TabWindowController class]])
+    return (TabWindowController*)keyWindowController;
+
+  return nil;
+}
+
+// If the window has tabs, make "close window" be cmd-shift-w, otherwise leave
+// it as the normal cmd-w. Capitalization of the key equivalent affects whether
+// the shift modifer is used.
+- (void)adjustCloseWindowMenuItemKeyEquivalent:(BOOL)inHaveTabs {
+  [closeWindowMenuItem_ setKeyEquivalent:(inHaveTabs ? @"W" : @"w")];
+}
+
+// If the window has tabs, make "close tab" take over cmd-w, otherwise it
+// shouldn't have any key-equivalent because it should be disabled.
+- (void)adjustCloseTabMenuItemKeyEquivalent:(BOOL)hasTabs {
+  if (hasTabs) {
+    [closeTabMenuItem_ setKeyEquivalent:@"w"];
+    [closeTabMenuItem_ setKeyEquivalentModifierMask:NSCommandKeyMask];
+  } else {
+    [closeTabMenuItem_ setKeyEquivalent:@""];
+    [closeTabMenuItem_ setKeyEquivalentModifierMask:0];
+  }
+}
+
+// See if we have a window with tabs open, and adjust the key equivalents for
+// Close Tab/Close Window accordingly
+- (void)fixCloseMenuItemKeyEquivalents {
+  TabWindowController* tabController = [self keyWindowTabController];
+  BOOL windowWithMultipleTabs =
+      (tabController && [tabController numberOfTabs] > 1);
+  [self adjustCloseWindowMenuItemKeyEquivalent:windowWithMultipleTabs];
+  [self adjustCloseTabMenuItemKeyEquivalent:windowWithMultipleTabs];
+  fileMenuUpdatePending_ = NO;
+}
+
+// Fix up the "close tab/close window" command-key equivalents. We do this
+// after a delay to ensure that window layer state has been set by the time
+// we do the enabling.
+- (void)delayedFixCloseMenuItemKeyEquivalents {
+  if (!fileMenuUpdatePending_) {
+    [self performSelector:@selector(fixCloseMenuItemKeyEquivalents)
+               withObject:nil
+               afterDelay:0];
+    fileMenuUpdatePending_ = YES;
+  }
+}
+
+// Called when we get a notification about the window layering changing to
+// update the UI based on the new main window.
+- (void)windowLayeringDidChange:(NSNotification*)notify {
+  [self delayedFixCloseMenuItemKeyEquivalents];
+  
+  // TODO(pinkerton): If we have other things here, such as inspector panels
+  // that follow the contents of the selected webpage, we would update those
+  // here.
+}
+
+// Called when the number of tabs changes in one of the browser windows. The
+// object is the tab strip controller, but we don't currently care.
+- (void)tabsChanged:(NSNotification*)notify {
+  [self delayedFixCloseMenuItemKeyEquivalents];
+}
+
 // If the auto-update interval is not set, make it 5 hours.
 // This code is specific to Mac Chrome Dev Channel.
 // Placed here for 2 reasons:
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 2070be8..1129a6c 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -309,6 +309,11 @@ willPositionSheet:(NSWindow *)sheet
       // Generate return value (enabled state)
       enable = browser_->command_updater()->IsCommandEnabled(tag) ? YES : NO;
 
+      // Disable "close tab" if we're not the key window or if there's only
+      // one tab.
+      if (tag == IDC_CLOSE_TAB)
+        enable &= [self numberOfTabs] > 1 && [[self window] isKeyWindow];
+
       // If the item is toggleable, find it's toggle state and
       // try to update it.  This is a little awkward, but the alternative is
       // to check after a commandDispatch, which seems worse.
diff --git a/chrome/browser/cocoa/tab_strip_controller.h b/chrome/browser/cocoa/tab_strip_controller.h
index eeab6b0..6004768 100644
--- a/chrome/browser/cocoa/tab_strip_controller.h
+++ b/chrome/browser/cocoa/tab_strip_controller.h
@@ -106,4 +106,8 @@ class ToolbarModel;
 + (CGFloat)defaultTabHeight;
 @end
 
+// Notification sent when the number of tabs changes. The object will be this
+// controller.
+extern NSString* const kTabStripNumberOfTabsChanged;
+
 #endif  // CHROME_BROWSER_COCOA_TAB_STRIP_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/tab_strip_controller.mm b/chrome/browser/cocoa/tab_strip_controller.mm
index 6fed79f..b81b455 100644
--- a/chrome/browser/cocoa/tab_strip_controller.mm
+++ b/chrome/browser/cocoa/tab_strip_controller.mm
@@ -22,6 +22,8 @@
 #include "chrome/browser/tabs/tab_strip_model.h"
 #include "grit/generated_resources.h"
 
+NSString* const kTabStripNumberOfTabsChanged = @"kTabStripNumberOfTabsChanged";
+
 // A simple view class that prevents the windowserver from dragging the
 // area behind tabs. Sometimes core animation confuses it.
 @interface TabStripControllerDragBlockingView : NSView
@@ -337,6 +339,11 @@
   if (!inForeground) {
     [self layoutTabs];
   }
+  
+  // Send a broadcast that the number of tabs have changed.
+  [[NSNotificationCenter defaultCenter]
+      postNotificationName:kTabStripNumberOfTabsChanged
+                    object:self];
 }
 
 // Called when a notification is received from the model to select a particular
@@ -397,6 +404,11 @@
   // Once we're totally done with the tab, delete its controller
   [tabArray_ removeObjectAtIndex:index];
 
+  // Send a broadcast that the number of tabs have changed.
+  [[NSNotificationCenter defaultCenter]
+      postNotificationName:kTabStripNumberOfTabsChanged
+                    object:self];
+
   [self layoutTabs];
 }
 
diff --git a/chrome/browser/cocoa/tab_strip_controller_unittest.mm b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
index 386b8e1..7470bb9 100644
--- a/chrome/browser/cocoa/tab_strip_controller_unittest.mm
+++ b/chrome/browser/cocoa/tab_strip_controller_unittest.mm
@@ -127,4 +127,11 @@ TEST_F(TabStripControllerTest, RearrangeTabs) {
   // if you don't do anything with it. http://crbug.com/10899
 }
 
+// Test that changing the number of tabs broadcasts a 
+// kTabStripNumberOfTabsChanged notifiction.
+TEST_F(TabStripControllerTest, Notifications) {
+  // TODO(pinkerton): Creating a TabContents crashes an unrelated test, even
+  // if you don't do anything with it. http://crbug.com/10899
+}
+
 }  // namespace