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 147 matching lines @@
   // Sync after a delay avoid I/O contention on startup; 1500 ms is plenty.
   BrowserThread::PostDelayedTask(BrowserThread::FILE, FROM_HERE,
                                  new PrefsSyncTask(), 1500);
 }
 
 }  // anonymous namespace
 
 const AEEventClass kAECloudPrintInstallClass = 'GCPi';
 const AEEventClass kAECloudPrintUninstallClass = 'GCPu';
 
+@interface NSWindow (AppControllerHelpers)
+- (BOOL)isDescendantOf:(NSWindow*)other;
+@end
+
+@implementation NSWindow (AppControllerHelpers)
+// Returns true if |self| is a descendant of the |other| window.
+- (BOOL)isDescendantOf:(NSWindow*)other {
+  NSWindow* parent;
+  do {
+    parent = [self parentWindow];
+    if (parent == other)
+      return YES;
+  } while (parent);
+
+  return NO;
+}

[Scott Hess - ex-Googler, 2011/10/25 23:40:59]

I think this should probably be -isDescendentOfWindow:.

That said, is it justified?  I'd be uncomfortable knowing that we have child
windows who have child windows.  Lot's of scope for weak references.

[Ilya Sherman, 2011/10/26 23:22:40]

I just added this because I wasn't sure if we could have child windows that have
their own child windows.  I'm happy to just check parentWindow: if that 
shouldn't be possible.

+@end
+
 @interface AppController (Private)
 - (void)initMenuState;
 - (void)initProfileMenu;
 - (void)updateConfirmToQuitPrefMenuItem:(NSMenuItem*)item;
 - (void)registerServicesMenuTypesTo:(NSApplication*)app;
 - (void)openUrls:(const std::vector<GURL>&)urls;
 - (void)getUrl:(NSAppleEventDescriptor*)event
      withReply:(NSAppleEventDescriptor*)reply;
 - (void)submitCloudPrintJob:(NSAppleEventDescriptor*)event;
 - (void)installCloudPrint:(NSAppleEventDescriptor*)event;
@@ skipping 167 matching lines @@
   if (!BrowserList::HasBrowserWithProfile([self lastProfile])) {
     // 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".
-- (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".
-- (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,
 // otherwise leave it as the normal cmd-w. Capitalization of the key equivalent
 // affects whether the shift modifer is used.
 - (void)adjustCloseWindowMenuItemKeyEquivalent:(BOOL)hasTabs {
   [closeWindowMenuItem_ setKeyEquivalent:(hasTabs ? @"W" : @"w")];
   [closeWindowMenuItem_ setKeyEquivalentModifierMask:NSCommandKeyMask];
 }
 
 // If the window has a tab controller, make "close tab" take over cmd-w,
 // otherwise it shouldn't have any key-equivalent because it should be disabled.
@@ skipping 10 matching lines @@
 // Explicitly remove any command-key equivalents from the close tab/window
 // menus so that nothing can go haywire if we get a user action during pending
 // updates.
 - (void)clearCloseMenuItemKeyEquivalents {
   [closeTabMenuItem_ setKeyEquivalent:@""];
   [closeTabMenuItem_ setKeyEquivalentModifierMask:0];
   [closeWindowMenuItem_ setKeyEquivalent:@""];
   [closeWindowMenuItem_ setKeyEquivalentModifierMask:0];
 }
 
-// See if we have a window with tabs open, and adjust the key equivalents for
+// See if the focused window window has tabs, and adjust the key equivalents for
 // Close Tab/Close Window accordingly.
-- (void)fixCloseMenuItemKeyEquivalents:(NSWindow*)window {
+- (void)fixCloseMenuItemKeyEquivalents {
   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.
-    tabController = [self mainWindowTabController];
+
+  NSWindow* window = [NSApp keyWindow];
+  NSWindow* mainWindow = [NSApp mainWindow];
+  if (!window || [window isDescendantOf:mainWindow]) {

[Scott Hess - ex-Googler, 2011/10/25 23:40:59]

Wow, can window be nil and mainWindow not nil?  Crazy, if true.

[Ilya Sherman, 2011/10/26 23:22:40]

Apparently, assuming the comment that used to be here (and is still here, in
expanded form) is not a lie.

+    // If the key window is a child of the main window (e.g. a bubble), the main
+    // window should be the one that handles the close menu item action.
+    // 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.
+    // You might think that we should just always use the main window, but the
+    // "About Chrome" window serves as a counterexample.
+    window = mainWindow;
   }
-  BOOL hasTabs = !!tabController;
 
+  BOOL hasTabs =
+      [[window windowController] isKindOfClass:[TabWindowController class]];
   [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
 // calls this (even as a side-effect) from other threads needs to be fixed.
-- (void)delayedFixCloseMenuItemKeyEquivalents:(NSNotification*)notify {
+- (void)delayedFixCloseMenuItemKeyEquivalents {
   DCHECK([NSThread isMainThread]);
   if (!fileMenuUpdatePending_) {
     // The OS prefers keypresses to timers, so it's possible that a cmd-w
     // can sneak in before this timer fires. In order to prevent that from
     // having any bad consequences, just clear the keys combos altogether. They
     // will be reset when the timer eventually fires.
     if ([NSThread isMainThread]) {
       fileMenuUpdatePending_ = YES;
       [self clearCloseMenuItemKeyEquivalents];
-      [self performSelector:@selector(fixCloseMenuItemKeyEquivalents:)
-                 withObject:[notify object]
+      [self performSelector:@selector(fixCloseMenuItemKeyEquivalents)
+                 withObject:nil
                  afterDelay:0];
     } else {
       // This shouldn't be happening, but if it does, force it to the main
       // thread to avoid dropping the update. Don't mess with
       // |fileMenuUpdatePending_| as it's not expected to be threadsafe and
       // there could be a race between the selector finishing and setting the
       // flag.
       [self
-          performSelectorOnMainThread:@selector(fixCloseMenuItemKeyEquivalents:)
-                           withObject:[notify object]
+          performSelectorOnMainThread:@selector(fixCloseMenuItemKeyEquivalents)
+                           withObject:nil
                         waitUntilDone:NO];
     }
   }
 }
 
 // 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:notify];
+  [self delayedFixCloseMenuItemKeyEquivalents];
 
   if ([notify name] == NSWindowDidResignKeyNotification) {
     // If a window is closed, this notification is fired but |[NSApp keyWindow]|
     // returns nil regardless of whether any suitable candidates for the key
     // window remain. It seems that the new key window for the app is not set
     // until after this notification is fired, so a check is performed after the
     // run loop is allowed to spin.
     [self performSelector:@selector(checkForAnyKeyWindows)
                withObject:nil
                afterDelay:0.0];
@@ skipping 813 matching lines @@
 
 }  // namespace browser
 
 namespace app_controller_mac {
 
 bool IsOpeningNewWindow() {
   return g_is_opening_new_window;
 }
 
 }  // namespace app_controller_mac