MacViews: Handle Tab properly

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 493ebdf4f83e7e5fc09dae827ddfc4a22c689e60..58d2972393fb4a50bdef72df2e69a4326d8cf4ca 100644
--- a/ui/views/cocoa/bridged_content_view.mm
+++ b/ui/views/cocoa/bridged_content_view.mm
@@ -41,14 +41,30 @@ gfx::Point MovePointToWindow(const NSPoint& point,
                     NSHeight(content_rect) - point_in_window.y);
 }
 
+// Checks if there's an active MenuController during key event dispatch. If
+// there is one, it gets preference, and it will likely swallow the event.
+bool DispatchEventToMenu(views::Widget* widget, ui::KeyboardCode key_code) {
+  MenuController* menuController = MenuController::GetActiveInstance();
+  if (menuController && menuController->owner() == widget) {
+    if (menuController->OnWillDispatchKeyEvent(0, key_code) ==
+        ui::POST_DISPATCH_NONE)
+      return true;
+  }
+  return false;
 }
 
+}  // namespace
+
 @interface BridgedContentView ()
 
 // Translates the location of |theEvent| to toolkit-views coordinates and passes
 // the event to NativeWidgetMac for handling.
 - (void)handleMouseEvent:(NSEvent*)theEvent;
 
+// Translates keycodes and modifiers on |theEvent| to ui::KeyEvents and passes
+// the event to the InputMethod for dispatch.
+- (void)handleKeyEvent:(NSEvent*)theEvent;
+
 // Handles an NSResponder Action Message by mapping it to a corresponding text
 // editing command from ui_strings.grd and, when not being sent to a
 // TextInputClient, the keyCode that toolkit-views expects internally.
@@ -140,6 +156,18 @@ gfx::Point MovePointToWindow(const NSPoint& point,
   hostedView_->GetWidget()->OnMouseEvent(&event);
 }
 
+- (void)handleKeyEvent:(NSEvent*)theEvent {
+  if (!hostedView_)
+    return;
+
+  DCHECK(theEvent);
+  ui::KeyEvent event(theEvent);
+  if (DispatchEventToMenu(hostedView_->GetWidget(), event.key_code()))
+    return;
+
+  hostedView_->GetWidget()->GetInputMethod()->DispatchKeyEvent(event);
+}
+
 - (void)handleAction:(int)commandId
              keyCode:(ui::KeyboardCode)keyCode
              domCode:(ui::DomCode)domCode
@@ -147,14 +175,8 @@ gfx::Point MovePointToWindow(const NSPoint& point,
   if (!hostedView_)
     return;
 
-  // If there's an active MenuController it gets preference, and it will likely
-  // swallow the event.
-  MenuController* menuController = MenuController::GetActiveInstance();
-  if (menuController && menuController->owner() == hostedView_->GetWidget()) {
-    if (menuController->OnWillDispatchKeyEvent(0, keyCode) ==
-        ui::POST_DISPATCH_NONE)
-      return;
-  }
+  if (DispatchEventToMenu(hostedView_->GetWidget(), keyCode))
+    return;
 
   // If there's an active TextInputClient, schedule the editing command to be
   // performed.
@@ -326,10 +348,14 @@ gfx::Point MovePointToWindow(const NSPoint& point,
 // NSResponder Action Messages. Keep sorted according NSResponder.h (from the
 // 10.9 SDK). The list should eventually be complete. Anything not defined will
 // beep when interpretKeyEvents: would otherwise call it.
-// TODO(tapted): Make this list complete.
+// TODO(tapted): Make this list complete, except for insert* methods which are
+// dispatched as regular key events in doCommandBySelector:.
 
 // The insertText action message forwards to the TextInputClient unless a menu
-// is active.
+// is active. Note that NSResponder's interpretKeyEvents: implementation doesn't
+// direct insertText: through doCommandBySelector:, so this is still needed to
+// handle the case when inputContext: is nil. When inputContext: returns non-nil
+// text goes directly to insertText:replacementRange:.
 - (void)insertText:(id)text {

[tapted, 2015/02/26 00:46:34]

Ah, my macviews trybot picked up a regression here in interactive_ui_tests (menu
mnemonic stuff) . Turns out insertText: is still needed because NSResponder's
interpretKeyEvents:implementation doesn't direct insertText: through
doCommandBySelector:, so the insertText: method is still needed (as well as
insertText:replacementRange:)

I've restored the old method and added a comment for the unwary.

   [self insertText:text replacementRange:NSMakeRange(NSNotFound, 0)];
 }
@@ -434,15 +460,6 @@ gfx::Point MovePointToWindow(const NSPoint& point,
           eventFlags:ui::EF_SHIFT_DOWN];
 }
 
-// Insertions and Indentations.
-
-- (void)insertNewline:(id)sender {
-  [self handleAction:0
-             keyCode:ui::VKEY_RETURN
-             domCode:ui::DomCode::ENTER
-          eventFlags:0];
-}
-
 // Deletions.
 
 - (void)deleteForward:(id)sender {
@@ -545,6 +562,13 @@ gfx::Point MovePointToWindow(const NSPoint& point,
 }
 
 - (void)doCommandBySelector:(SEL)selector {
+  // Like the renderer, handle insert action messages as a regular key dispatch.
+  // This ensures, e.g., insertTab correctly changes focus between fields.
+  if (inKeyDown_ && [NSStringFromSelector(selector) hasPrefix:@"insert"]) {
+    [self handleKeyEvent:[NSApp currentEvent]];
+    return;
+  }
+
   if ([self respondsToSelector:selector])
     [self performSelector:selector withObject:nil];
   else