Make window cycling work even if you change it to something else than cmd-` in sysprefs.

Instead of just dispatching to the menu after a key comes back from the renderer, do a complete re-dispatch to NSApp (so that the event gets to the menu and cmd-` handlers) but then ignore it when it comes back to the web (because we already sent this event to the renderer once).

BUG=24817
TEST=Open sysprefs, change keyboard shortcut for "Move focus to next window in active application" to e.g. cmd-\. Open two chrome windows, focus the web, hit cmd-\. It should switch windows. All other keyboard shortcuts should still work (test that ctrl-tab works when web has focus, test backspace when text field is focussed in web, when background is focussed in web, when IME is active, test hitting cmd-1/2 when omnibox or web have focus, hit cmd-left when omnibox, textbox in web, background in web has focus, test that cmd-f in docs still opens doc's find interface)

Review URL: http://codereview.chromium.org/303002

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

6 files changed, 83 insertions, 49 deletions

diff --git a/chrome/browser/chrome_application_mac.mm b/chrome/browser/chrome_application_mac.mm
index d9b5eb4..135a77e 100644
--- a/chrome/browser/chrome_application_mac.mm
+++ b/chrome/browser/chrome_application_mac.mm
@@ -8,7 +8,7 @@
 #import "base/logging.h"
 #import "base/scoped_nsobject.h"
 #import "chrome/app/breakpad_mac.h"
-#import "chrome/browser/renderer_host/render_widget_host_view_mac.h"
+#import "chrome/browser/cocoa/chrome_event_processing_window.h"
 
 namespace CrApplicationNSException {
 
@@ -190,19 +190,11 @@ class ScopedCrashKey {
   // view loop before dispatching them to |keyDown:|. Since we want to send keys
   // to the renderer before sending them to the menu, and we never want them to
   // the kev view loop when the web is focussed, we change this behavior.
-  if ([event type] == NSKeyDown || [event type] == NSKeyUp) {
-    if ([[[self keyWindow] firstResponder]
-        isKindOfClass:[RenderWidgetHostViewCocoa class]]) {
-      // No other mac browser sends keyup() for keyboard equivalents, so let's
-      // suppress this.
-      if (([event modifierFlags] & NSCommandKeyMask) && [event type] == NSKeyUp)
-        return;
-
-      RenderWidgetHostViewCocoa* rwhv = static_cast<RenderWidgetHostViewCocoa*>(
-          [[self keyWindow] firstResponder]);
-      [rwhv keyEvent:event];
+  if ([[self keyWindow]
+      isKindOfClass:[ChromeEventProcessingWindow class]]) {
+    if ([static_cast<ChromeEventProcessingWindow*>([self keyWindow])
+        shortcircuitEvent:event])
       return;
-    }
   }
 
   [super sendEvent:event];
diff --git a/chrome/browser/cocoa/chrome_event_processing_window.h b/chrome/browser/cocoa/chrome_event_processing_window.h
index b3cc116..59241b7 100644
--- a/chrome/browser/cocoa/chrome_event_processing_window.h
+++ b/chrome/browser/cocoa/chrome_event_processing_window.h
@@ -12,7 +12,20 @@
 // Override NSWindow to access unhandled keyboard events (for command
 // processing); subclassing NSWindow is the only method to do
 // this.
-@interface ChromeEventProcessingWindow : NSWindow
+@interface ChromeEventProcessingWindow : NSWindow {
+ @private
+  BOOL redispatchingEvent_;
+}
+
+// Returns |YES| if |event| has been shortcircuited and should not be processed
+// further.
+- (BOOL)shortcircuitEvent:(NSEvent*)event;
+
+// Sends an event to |NSApp sendEvent:|, but also makes sure that it's not
+// short-circuited to the RWHV. This is used to send keyboard events to the menu
+// and the cmd-` handler if a keyboard event comes back unhandled from the
+// renderer.
+- (void)redispatchEvent:(NSEvent*)event;
 
 // See global_keyboard_shortcuts_mac.h for details on the next two functions.
 
diff --git a/chrome/browser/cocoa/chrome_event_processing_window.mm b/chrome/browser/cocoa/chrome_event_processing_window.mm
index 8633c1c..c1b270f 100644
--- a/chrome/browser/cocoa/chrome_event_processing_window.mm
+++ b/chrome/browser/cocoa/chrome_event_processing_window.mm
@@ -50,10 +50,30 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, int);
                                  fromTable:CommandForBrowserKeyboardShortcut];
 }
 
+- (BOOL)shortcircuitEvent:(NSEvent*)event {
+  if (!redispatchingEvent_ &&
+      ([event type] == NSKeyDown || [event type] == NSKeyUp)) {
+    if ([[self firstResponder]
+        isKindOfClass:[RenderWidgetHostViewCocoa class]]) {
+      // No other mac browser sends keyup() for keyboard equivalents, so let's
+      // suppress this.
+      if (([event modifierFlags] & NSCommandKeyMask) && [event type] == NSKeyUp)
+        return YES;
+
+      RenderWidgetHostViewCocoa* rwhv = static_cast<RenderWidgetHostViewCocoa*>(
+          [self firstResponder]);
+      [rwhv keyEvent:event];
+      return YES;
+    }
+  }
+  return NO;
+}
+
 - (BOOL)performKeyEquivalent:(NSEvent*)event {
-  // We have some magic in |CrApplication sendEvent:| that always sends key 
-  // events to |RWHVCocoa keyEvent:| so that cocoa doesn't have a chance to
-  // intercept it.
+  if (redispatchingEvent_)
+    return NO;
+
+  // |shortcircuitEvent:| should handle all events directed to the RWHV.
   DCHECK(![[self firstResponder]
       isKindOfClass:[RenderWidgetHostViewCocoa class]]);
 
@@ -66,5 +86,17 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, int);
   return [super performKeyEquivalent:event];
 }
 
+- (void)redispatchEvent:(NSEvent*)event {
+  DCHECK([event window] == self);
+  redispatchingEvent_ = YES;
+  [NSApp sendEvent:event];
+  redispatchingEvent_ = NO;
+}
+
+- (void)sendEvent:(NSEvent*)event {
+  if (!redispatchingEvent_)
+    [super sendEvent:event];
+}
+
 @end  // ChromeEventProcessingWindow
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h
index 0e76e95..38edca2 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h
@@ -42,12 +42,18 @@ class RWHVMEditCommandHelper;
   void *trackingRectUserData_;
   NSTrackingRectTag lastToolTipTag_;
   NSString* toolTip_;
+
+  BOOL ignoreKeyEvents_;
 }
 
 - (void)setCanBeKeyView:(BOOL)can;
 - (void)setCloseOnDeactivate:(BOOL)b;
 - (void)setToolTipAtMousePoint:(NSString *)string;
 
+// When a keyboard event comes back from the renderer, we redispatch it. This
+// makes sure we ignore it if we should receive it during redispatch, instead
+// of sending it to the renderer again.
+- (void)setIgnoreKeyEvents:(BOOL)ignorekeyEvents;
 @end
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.mm b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
index 4df230e..4c12e9c 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
@@ -536,7 +536,14 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) {
     renderWidgetHostView_->render_widget_host_->ForwardMouseEvent(event);
 }
 
+- (void)setIgnoreKeyEvents:(BOOL)ignorekeyEvents {
+  ignoreKeyEvents_ = ignorekeyEvents;
+}
+
 - (BOOL)performKeyEquivalent:(NSEvent*)theEvent {
+  if (ignoreKeyEvents_)
+    return NO;
+
   // We have some magic in |CrApplication sendEvent:| that always sends key 
   // events to |keyEvent:| so that cocoa doesn't have a chance to intercept it.
   DCHECK([[self window] firstResponder] != self);
@@ -544,6 +551,9 @@ void RenderWidgetHostViewMac::SetBackground(const SkBitmap& background) {
 }
 
 - (void)keyEvent:(NSEvent*)theEvent {
+  if (ignoreKeyEvents_)
+    return;
+
   // TODO(avi): Possibly kill self? See RenderWidgetHostViewWin::OnKeyEvent and
   // http://b/issue?id=1192881 .
 
diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.mm b/chrome/browser/tab_contents/tab_contents_view_mac.mm
index 0dd47738..84f2e0e 100644
--- a/chrome/browser/tab_contents/tab_contents_view_mac.mm
+++ b/chrome/browser/tab_contents/tab_contents_view_mac.mm
@@ -9,6 +9,7 @@
 #include <string>
 
 #include "chrome/browser/browser.h" // TODO(beng): this dependency is awful.
+#import "chrome/browser/chrome_application_mac.h"
 #import "chrome/browser/cocoa/focus_tracker.h"
 #import "chrome/browser/cocoa/chrome_browser_window.h"
 #import "chrome/browser/cocoa/browser_window_controller.h"
@@ -316,50 +317,30 @@ void TabContentsViewMac::Observe(NotificationType type,
 
   NSEvent* event = wkEvent->os_event;
 
-  if ([event type] == NSKeyDown && ([event modifierFlags] & NSCommandKeyMask)) {
-    // We need to dispatch this to the menu.
-    if ([[NSApp mainMenu] performKeyEquivalent:event])
-      return;
-  }
-
-  // Cmd-` is not in the menu and it's apparently handled by |NSApp sendEvent|
-  // if the application doesn't swallow it. We do, so we need to handle this
-  // key ourself. On foreign keyboards, the "switch windows" key is not the
-  // ` key, so do this by keycode instead of |event characters|.
-  if ([event type] == NSKeyDown &&
-      [event keyCode] == kVK_ANSI_Grave &&
-      [NSApp respondsToSelector:@selector(_cycleWindowsReversed:)]) {
-    const NSUInteger kModifierMask = NSShiftKeyMask |
-                                     NSControlKeyMask |
-                                     NSAlternateKeyMask |
-                                     NSCommandKeyMask;
-    if (([event modifierFlags] & kModifierMask) == NSCommandKeyMask)
-      [NSApp _cycleWindowsReversed:NO];
-    else if (([event modifierFlags] & kModifierMask) ==
-        (NSCommandKeyMask | NSShiftKeyMask) &&
-        [NSApp respondsToSelector:@selector(_cycleWindowsReversed:)])
-      [NSApp _cycleWindowsReversed:YES];
-  }
-
-  // If this tab is no longer active, it's window will be |nil|. In that case,
+  // If this tab is no longer active, its window will be |nil|. In that case,
   // best ignore the event.
   if (![self window])
     return;
+  ChromeEventProcessingWindow* window =
+      (ChromeEventProcessingWindow*)[self window];
+  DCHECK([window isKindOfClass:[ChromeEventProcessingWindow class]]);
 
   // Do not fire shortcuts on key up.
   if ([event type] == NSKeyDown) {
-    ChromeBrowserWindow* window = (ChromeBrowserWindow*)[self window];
-    DCHECK([window isKindOfClass:[ChromeBrowserWindow class]]);
     if ([window handleExtraBrowserKeyboardShortcut:event])
       return;
     if ([window handleExtraWindowKeyboardShortcut:event])
       return;
   }
 
-  if ([event type] == NSKeyDown)
-    [super keyDown:event];
-  else if ([event type] == NSKeyUp)
-    [super keyUp:event];
+  // We need to re-dispatch the event, so that it is sent to the menu or other
+  // cocoa mechanisms (such as the cmd-` handler).
+  RenderWidgetHostViewCocoa* rwhv = static_cast<RenderWidgetHostViewCocoa*>(
+      tabContentsView_->GetContentNativeView());
+  DCHECK([rwhv isKindOfClass:[RenderWidgetHostViewCocoa class]]);
+  [rwhv setIgnoreKeyEvents:YES];
+  [window redispatchEvent:event];
+  [rwhv setIgnoreKeyEvents:NO];
 }
 
 - (void)mouseEvent:(NSEvent *)theEvent {