MacViews: Fix accelerator handling while Omnibox is in focus.

ui/views/cocoa/bridged_content_view.mm

Index: ui/views/cocoa/bridged_content_view.mm
diff --git a/ui/views/cocoa/bridged_content_view.mm b/ui/views/cocoa/bridged_content_view.mm
index 7c49cd0444346d7291793796970f16a6d7a7c505..625140b35c8dd2996aa669e4b98273f0f2f89595 100644
--- a/ui/views/cocoa/bridged_content_view.mm
+++ b/ui/views/cocoa/bridged_content_view.mm
@@ -760,10 +760,28 @@ NSAttributedString* GetAttributedString(
 
 // NSResponder implementation.
 
+- (BOOL)_wantsKeyDownForEvent:(NSEvent*)event {
+  // This is a SPI that AppKit apparently calls after |performKeyEquivalent:|
+  // returned NO. If this function returns |YES|, Cocoa sends the event to
+  // |keyDown:| instead of doing other things with it. Ctrl-tab will be sent
+  // to us instead of doing key view loop control, ctrl-left/right get handled
+  // correctly, etc.
+  // (However, there are still some keys that Cocoa swallows, e.g. the key
+  // equivalent that Cocoa uses for toggling the input language. In this case,
+  // that's actually a good thing, though -- see http://crbug.com/26115 .)
+  return YES;
+}
+
 - (void)keyDown:(NSEvent*)theEvent {
   // Convert the event into an action message, according to OSX key mappings.
   keyDownEvent_ = theEvent;
   [self interpretKeyEvents:@[ theEvent ]];
+
+  if (keyDownEvent_) {

[tapted, 2016/11/18 07:28:04]

comment above: 

  // If |keyDownEvent_| wasn't cleared during -interpretKeyEvents:, it wasn't
  // handled. Give Widget accelerators a chance to handle it.

[themblsha, 2016/11/18 13:31:50]

Done.

+    ui::KeyEvent event(keyDownEvent_);
+    [self handleKeyEvent:&event];
+  }
+
   keyDownEvent_ = nil;
 }
 
@@ -1286,13 +1304,25 @@ NSAttributedString* GetAttributedString(
   if (keyDownEvent_ && [NSStringFromSelector(selector) hasPrefix:@"insert"]) {
     ui::KeyEvent event(keyDownEvent_);

[tapted, 2016/11/18 07:28:04]

Can this just return here? I.e.

  return;  // Handle in -keyDown:.

[themblsha, 2016/11/18 13:31:50]

Yep, should work.

     [self handleKeyEvent:&event];
+    keyDownEvent_ = nil;
     return;
   }
 
-  if ([self respondsToSelector:selector])
+  if ([self respondsToSelector:selector]) {
     [self performSelector:selector withObject:nil];
-  else
-    [[self nextResponder] doCommandBySelector:selector];
+    keyDownEvent_ = nil;
+    return;
+  }
+
+  if (keyDownEvent_) {

[tapted, 2016/11/18 07:28:04]

This needs a comment to say why it can't be done in keyDown

Something like

// For events that AppKit sends via doCommandBySelector:, first attempt to
// handle as a Widget accelerator. Forward along the responder chain only if the
// Widget doesn't handle it.


See how you feel about a helper method like

- (BOOL)handleUnhandledKeyDownAsKeyEvent {
  if (!keyDownEvent_)
    return NO;

  ui::KeyEvent event(keyDownEvent_);
  [self handleKeyEvent:&event];
  keyDownEvent_ = nil;
  return event.handled();
}

Then this would look like

if (![self handleUnhandledKeyDownAsKeyEvent])
  [[self nextResponder] doCommandBySelector:selector];

and we'd have

- (void)keyDown:(NSEvent*)theEvent {
  keyDownEvent_ = theEvent;
  // Convert the event into an action message, according to OSX key mappings.
  [self interpretKeyEvents:@[ theEvent ]];
  [self handleUnhandledKeyDownAsKeyEvent];
  DCHECK(!keyDownEvent_);
}

[themblsha, 2016/11/18 13:31:50]

Nice, removes quite a bit of duplicate code.

+    ui::KeyEvent event(keyDownEvent_);
+    keyDownEvent_ = nil;
+    [self handleKeyEvent:&event];
+    if (event.handled())
+      return;
+  }
+
+  [[self nextResponder] doCommandBySelector:selector];
 }
 
 - (NSRect)firstRectForCharacterRange:(NSRange)range