Prevent extension and bookmark popups from interfering with close window/tab shortcuts.

chrome/browser/app_controller_mac.mm

 // Copyright (c) 2011 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/app_controller_mac.h"
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/file_path.h"
 #include "base/mac/foundation_util.h"
@@ skipping 26 matching lines @@
 #include "chrome/browser/sync/sync_ui_util.h"
 #include "chrome/browser/sync/sync_ui_util_mac.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_init.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #import "chrome/browser/ui/cocoa/about_window_controller.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h"
 #import "chrome/browser/ui/cocoa/browser_window_cocoa.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/chrome_event_processing_window.h"
 #import "chrome/browser/ui/cocoa/confirm_quit_panel_controller.h"
 #import "chrome/browser/ui/cocoa/encoding_menu_controller_delegate_mac.h"
 #import "chrome/browser/ui/cocoa/history_menu_bridge.h"
 #import "chrome/browser/ui/cocoa/profile_menu_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_strip_controller.h"
 #import "chrome/browser/ui/cocoa/tabs/tab_window_controller.h"
 #include "chrome/browser/ui/cocoa/task_manager_mac.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 289 matching lines @@
     // As we're shutting down, we need to nuke the TabRestoreService, which
     // will start the shutdown of the NavigationControllers and allow for
     // proper shutdown. If we don't do this, Chrome won't shut down cleanly,
     // and may end up crashing when some thread tries to use the IO thread (or
     // another thread) that is no longer valid.
     TabRestoreServiceFactory::ResetForProfile([self lastProfile]);
   }
 }
 
 // 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".
+// window, or nil if not. For example, the "about" window is non-tabbed.
 - (TabWindowController*)keyWindowTabController {
   NSWindowController* keyWindowController =
       [[NSApp keyWindow] windowController];
   if ([keyWindowController isKindOfClass:[TabWindowController class]])
     return (TabWindowController*)keyWindowController;
 
   return nil;
 }
 
 // Helper routine to get the window controller if the main window is a tabbed
-// window, or nil if not. Examples of non-tabbed windows are "about" or
-// "preferences".
+// window, or nil if not. For example, the "about" window is non-tabbed.
 - (TabWindowController*)mainWindowTabController {
   NSWindowController* mainWindowController =
       [[NSApp mainWindow] windowController];
   if ([mainWindowController isKindOfClass:[TabWindowController class]])
     return (TabWindowController*)mainWindowController;
 
   return nil;
 }
 
 // If the window has a tab controller, make "close window" be cmd-shift-w,
@@ skipping 24 matching lines @@
   [closeTabMenuItem_ setKeyEquivalentModifierMask:0];
   [closeWindowMenuItem_ setKeyEquivalent:@""];
   [closeWindowMenuItem_ setKeyEquivalentModifierMask:0];
 }
 
 // See if we have a window with tabs open, and adjust the key equivalents for
 // Close Tab/Close Window accordingly.
 - (void)fixCloseMenuItemKeyEquivalents:(NSWindow*)window {
   fileMenuUpdatePending_ = NO;
   TabWindowController* tabController = [self keyWindowTabController];
-  if (!tabController && ![NSApp keyWindow]) {
-    // There might be a small amount of time where there is no key window,
-    // so just use our main browser window if there is one.
+  if (!tabController &&
+      (![NSApp keyWindow] ||
+       [[NSApp keyWindow] isKindOfClass:[ChromeEventProcessingWindow class]])) {

[Ilya Sherman, 2011/10/15 04:31:38]

This seems a little hacky, but I wasn't sure of a better way to check whether
the key window is one that we're uninterested in.  Do you know why we try to
care about the key window, rather than just always querying the main window? 
Off the top of my head, I can't think of any cases where querying the main
window would do the wrong thing.

[Scott Hess - ex-Googler, 2011/10/19 02:43:41]

I agree - I think that this is EXACTLY the kind of case where the distinction
between key and main window is useful.  The canonical example is a find panel. 
The main windows and key windows are the same until you pull up a find panel, at
which point the key window is the find panel while the document window is still
the main window.

+    // If the key window is a |ChromeEventProcessingWindow| (i.e. a bubble), the
+    // window that should handle the close menu item action is the main window.
+    // Also, there might be a small amount of time where there is no key window;
+    // in that case as well, just use our main browser window if there is one.
     tabController = [self mainWindowTabController];

[Scott Hess - ex-Googler, 2011/10/19 02:43:41]

Wow, this whole thing seems hokey.  AFAICT, the only caller of the two helpers
is this function, and it does this additional work on top of all that.  I think
it would make sense to just wrap the entire thing up as a -hasTabsThingyHelper.

Something that would be 23% more tricky would be to have the window which cares
about this define a -hasTabs: action method, and then instead of putzing around,
just use hasTabs = !![NSApp targetForAction:@selector(hasTabs:)];.  It'll fall
through the key window to the main window as desired.

Another option would be to unwire Close Tab from -commandDispatch: and rewire it
to -closeTab: (which can forward to -commandDispatch:), and key off that.

   }
   BOOL hasTabs = !!tabController;
 
   [self adjustCloseWindowMenuItemKeyEquivalent:hasTabs];
   [self adjustCloseTabMenuItemKeyEquivalent:hasTabs];
 }
 
 // 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. This should only be called on the main thread, code that
@@ skipping 854 matching lines @@
 
 }  // namespace browser
 
 namespace app_controller_mac {
 
 bool IsOpeningNewWindow() {
   return g_is_opening_new_window;
 }
 
 }  // namespace app_controller_mac