Fix command-click on buttons in background windows to perform their action in the context of the background window's controller, not the one associated with the foreground window. Command-click on back/fwd button in a background window doesn't have any special open disposition like it does in fg window.

BUG=16191
TEST=menus, key commands, button command dispatching should all still work for foreground and background windows. Test cmd-clicking buttons in a browser when a non-browser is the foreground window.
Review URL: http://codereview.chromium.org/543044

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

4 files changed, 79 insertions, 12 deletions

diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 08d61b8..4ad4d9f 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -523,13 +523,25 @@ static bool g_is_opening_new_window = false;
   return enable;
 }
 
-// Called when the user picks a menu item when there are no key windows. Calls
-// through to the browser object to execute the command. This assumes that the
-// command is supported and doesn't check, otherwise it would have been disabled
-// in the UI in validateUserInterfaceItem:.
+// Called when the user picks a menu item when there are no key windows, or when
+// there is no foreground browser window. Calls through to the browser object to
+// execute the command. This assumes that the command is supported and doesn't
+// check, otherwise it would have been disabled in the UI in
+// validateUserInterfaceItem:.
 - (void)commandDispatch:(id)sender {
   Profile* defaultProfile = [self defaultProfile];
 
+  // Handle the case where we're dispatching a command from a sender that's in a
+  // browser window. This means that the command came from a background window
+  // and is getting here because the foreground window is not a browser window.
+  if ([sender respondsToSelector:@selector(window)]) {
+    id delegate = [[sender window] windowController];
+    if ([delegate isKindOfClass:[BrowserWindowController class]]) {
+      [delegate commandDispatch:sender];
+      return;
+    }
+  }
+
   NSInteger tag = [sender tag];
   switch (tag) {
     case IDC_NEW_TAB:
@@ -604,6 +616,21 @@ static bool g_is_opening_new_window = false;
   };
 }
 
+// Same as |-commandDispatch:|, but executes commands using a disposition
+// determined by the key flags. This will get called in the case where the
+// frontmost window is not a browser window, and the user has command-clicked
+// a button in a background browser window whose action is
+// |-commandDispatchUsingKeyModifiers:|
+- (void)commandDispatchUsingKeyModifiers:(id)sender {
+  DCHECK(sender);
+  if ([sender respondsToSelector:@selector(window)]) {
+    id delegate = [[sender window] windowController];
+    if ([delegate isKindOfClass:[BrowserWindowController class]]) {
+      [delegate commandDispatchUsingKeyModifiers:sender];
+    }
+  }
+}
+
 // NSApplication delegate method called when someone clicks on the
 // dock icon and there are no open windows.  To match standard mac
 // behavior, we should open a new window.
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 0d767eb..d8be276 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -855,6 +855,17 @@ willPositionSheet:(NSWindow*)sheet
 // the command is supported and doesn't check, otherwise it would have been
 // disabled in the UI in validateUserInterfaceItem:.
 - (void)commandDispatch:(id)sender {
+  DCHECK(sender);
+  // Identify the actual BWC to which the command should be dispatched. It might
+  // belong to a background window, yet this controller gets it because it is
+  // the foreground window's controller and thus in the responder chain. Some
+  // senders don't have this problem (for example, menus only operate on the
+  // foreground window), so this is only an issue for senders that are part of
+  // windows.
+  BrowserWindowController* targetController = self;
+  if ([sender respondsToSelector:@selector(window)])
+    targetController = [[sender window] windowController];
+  DCHECK([targetController isKindOfClass:[BrowserWindowController class]]);
   NSInteger tag = [sender tag];
   switch (tag) {
     case IDC_RELOAD:
@@ -864,19 +875,35 @@ willPositionSheet:(NSWindow*)sheet
         // for Windows (ToolbarView::ButtonPressed()), this function handles
         // both reload button press event and Command+r press event. Thus the
         // 'isKindofClass' check is necessary.
-        [self locationBarBridge]->Revert();
+        [targetController locationBarBridge]->Revert();
       }
       break;
   }
-  browser_->ExecuteCommand(tag);
+  DCHECK(targetController->browser_.get());
+  targetController->browser_->ExecuteCommand(tag);
 }
 
 // Same as |-commandDispatch:|, but executes commands using a disposition
-// determined by the key flags.
+// determined by the key flags. If the window is in the background and the
+// command key is down, ignore the command key, but process any other modifiers.
 - (void)commandDispatchUsingKeyModifiers:(id)sender {
+  DCHECK(sender);
+  // See comment above for why we do this.
+  BrowserWindowController* targetController = self;
+  if ([sender respondsToSelector:@selector(window)])
+    targetController = [[sender window] windowController];
+  DCHECK([targetController isKindOfClass:[BrowserWindowController class]]);
   NSInteger tag = [sender tag];
-  browser_->ExecuteCommandWithDisposition(tag,
-      event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]));
+  DCHECK(targetController->browser_.get());
+  NSUInteger modifierFlags = [[NSApp currentEvent] modifierFlags];
+  if (![[sender window] isMainWindow]) {
+    // Remove the command key from the flags, it means "keep the window in
+    // the background" in this case.
+    modifierFlags &= ~NSCommandKeyMask;
+  }
+  targetController->browser_->ExecuteCommandWithDisposition(tag,
+      event_utils::WindowOpenDispositionFromNSEventWithFlags(
+          [NSApp currentEvent], modifierFlags));
 }
 
 // Called when another part of the internal codebase needs to execute a
diff --git a/chrome/browser/cocoa/event_utils.h b/chrome/browser/cocoa/event_utils.h
index 7d0f87b..7921187 100644
--- a/chrome/browser/cocoa/event_utils.h
+++ b/chrome/browser/cocoa/event_utils.h
@@ -16,6 +16,14 @@ namespace event_utils {
 // associated link in a background tab.
 WindowOpenDisposition WindowOpenDispositionFromNSEvent(NSEvent* event);
 
+// Retrieves the WindowOpenDisposition used to open a link from a user gesture
+// represented by |event|, but instead use the modifier flags given by |flags|,
+// which is the same format as |-NSEvent modifierFlags|. This allows
+// substitution of the modifiers without having to create a new event from
+// scratch.
+WindowOpenDisposition WindowOpenDispositionFromNSEventWithFlags(
+    NSEvent* event, NSUInteger flags);
+
 }  // namespace event_utils
 
 #endif  // CHROME_BROWSER_COCOA_EVENT_UTILS_H_
diff --git a/chrome/browser/cocoa/event_utils.mm b/chrome/browser/cocoa/event_utils.mm
index a528d6c..2f65d9a 100644
--- a/chrome/browser/cocoa/event_utils.mm
+++ b/chrome/browser/cocoa/event_utils.mm
@@ -8,9 +8,14 @@ namespace event_utils {
 
 WindowOpenDisposition WindowOpenDispositionFromNSEvent(NSEvent* event) {
   NSUInteger modifiers = [event modifierFlags];
-  if ([event buttonNumber] == 2 || modifiers & NSCommandKeyMask)
-    return modifiers & NSShiftKeyMask ? NEW_FOREGROUND_TAB : NEW_BACKGROUND_TAB;
-  return modifiers & NSShiftKeyMask ? NEW_WINDOW : CURRENT_TAB;
+  return WindowOpenDispositionFromNSEventWithFlags(event, modifiers);
+}
+
+WindowOpenDisposition WindowOpenDispositionFromNSEventWithFlags(
+    NSEvent* event, NSUInteger flags) {
+  if ([event buttonNumber] == 2 || flags & NSCommandKeyMask)
+    return flags & NSShiftKeyMask ? NEW_FOREGROUND_TAB : NEW_BACKGROUND_TAB;
+  return flags & NSShiftKeyMask ? NEW_WINDOW : CURRENT_TAB;
 }
 
 }  // namespace event_utils