MacViews: Handle Tab properly

On Mac, interpreting a tab keypress into an action message calls
[NSResponder insertTab]. [RenderWidgetHostViewCocoa
doCommandBySelector:] has for a long time had a check that skips
converting any action message whose selector starts with "insert" into
an editing command, and just treats it as a regular keypress instead.

This approach also works well for UI in MacViews, so do the same.

Theoretically a user could remap a different key or key combination to
the 'insertTab' action message, but that's unlikely.

Updates TextfieldTest.FocusTraversalTest with cross-platform checks to
test for this case. Also tested manually with views_examples.

For the test to work with the approach in this CL, the event simulator
needs to provide an answer for -[NSApplication currentEvent], so that's
done too.

BUG=454353

Review URL: https://codereview.chromium.org/951433002

Cr-Commit-Position: refs/heads/master@{#318172}

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)
+  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 {
   [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

ui/views/controls/textfield/textfield_unittest.cc

@@ -856,6 +856,25 @@ TEST_F(TextfieldTest, FocusTraversalTest) {
                        ui::EF_LEFT_MOUSE_BUTTON);
   textfield_->OnMousePressed(click);
   EXPECT_EQ(1, GetFocusedView()->id());
+
+  // Tab/Shift+Tab should also cycle focus, not insert a tab character.
+  SendKeyEvent(ui::VKEY_TAB, false, false);
+  EXPECT_EQ(2, GetFocusedView()->id());
+  SendKeyEvent(ui::VKEY_TAB, false, false);
+  EXPECT_EQ(3, GetFocusedView()->id());
+  // Cycle back to the first textfield.
+  SendKeyEvent(ui::VKEY_TAB, false, false);
+  EXPECT_EQ(1, GetFocusedView()->id());
+
+  SendKeyEvent(ui::VKEY_TAB, true, false);
+  EXPECT_EQ(3, GetFocusedView()->id());
+  SendKeyEvent(ui::VKEY_TAB, true, false);
+  EXPECT_EQ(2, GetFocusedView()->id());
+  SendKeyEvent(ui::VKEY_TAB, true, false);
+  EXPECT_EQ(1, GetFocusedView()->id());
+  // Cycle back to the last textfield.
+  SendKeyEvent(ui::VKEY_TAB, true, false);
+  EXPECT_EQ(3, GetFocusedView()->id());
 }
 
 TEST_F(TextfieldTest, ContextMenuDisplayTest) {

ui/views/test/event_generator_delegate_mac.mm

@@ -20,30 +20,16 @@ namespace {
 // Singleton to provide state for swizzled Objective C methods.
 ui::test::EventGenerator* g_active_generator = NULL;
 
+// Set (and always cleared) in EmulateSendEvent() to provide an answer for
+// [NSApp currentEvent].
+NSEvent* g_current_event = nil;
+
 }  // namespace
 
 @interface NSEventDonor : NSObject
 @end
 
-@implementation NSEventDonor
-
-// Donate +[NSEvent pressedMouseButtons] by retrieving the flags from the
-// active generator.
-+ (NSUInteger)pressedMouseButtons {
-  if (!g_active_generator)
-    return [NSEventDonor pressedMouseButtons];  // Call original implementation.
-
-  int flags = g_active_generator->flags();
-  NSUInteger bitmask = 0;
-  if (flags & ui::EF_LEFT_MOUSE_BUTTON)
-    bitmask |= 1;
-  if (flags & ui::EF_RIGHT_MOUSE_BUTTON)
-    bitmask |= 1 << 1;
-  if (flags & ui::EF_MIDDLE_MOUSE_BUTTON)
-    bitmask |= 1 << 2;
-  return bitmask;
-}
-
+@interface NSApplicationDonor : NSObject
 @end
 
 namespace {
@@ -133,6 +119,7 @@ NSEventType EventTypeToNative(ui::EventType ui_event_type,
 // sendEvent is a black box which (among other things) will try to peek at the
 // event queue and can block indefinitely.
 void EmulateSendEvent(NSWindow* window, NSEvent* event) {
+  base::AutoReset<NSEvent*> reset(&g_current_event, event);
   NSResponder* responder = [window firstResponder];
   switch ([event type]) {
     case NSKeyDown:
@@ -243,6 +230,10 @@ class EventGeneratorDelegateMac : public ui::EventTarget,
     return Singleton<EventGeneratorDelegateMac>::get();
   }
 
+  IMP CurrentEventMethod() {
+    return swizzle_current_event_->GetOriginalImplementation();
+  }
+
   // Overridden from ui::EventTarget:
   bool CanAcceptEvent(const ui::Event& event) override { return true; }
   ui::EventTarget* GetParentTarget() override { return NULL; }
@@ -291,6 +282,7 @@ class EventGeneratorDelegateMac : public ui::EventTarget,
   ui::test::EventGenerator* owner_;
   NSWindow* window_;
   scoped_ptr<base::mac::ScopedObjCClassSwizzler> swizzle_pressed_;
+  scoped_ptr<base::mac::ScopedObjCClassSwizzler> swizzle_current_event_;
   base::scoped_nsobject<NSMenu> fake_menu_;
 
   DISALLOW_COPY_AND_ASSIGN(EventGeneratorDelegateMac);
@@ -368,6 +360,7 @@ void EventGeneratorDelegateMac::SetContext(ui::test::EventGenerator* owner,
                                            gfx::NativeWindow root_window,
                                            gfx::NativeWindow window) {
   swizzle_pressed_.reset();
+  swizzle_current_event_.reset();
   owner_ = owner;
   window_ = window;
 
@@ -384,6 +377,10 @@ void EventGeneratorDelegateMac::SetContext(ui::test::EventGenerator* owner,
         [NSEvent class],
         [NSEventDonor class],
         @selector(pressedMouseButtons)));
+    swizzle_current_event_.reset(new base::mac::ScopedObjCClassSwizzler(
+        [NSApplication class],
+        [NSApplicationDonor class],
+        @selector(currentEvent)));
   }
 }
 
@@ -410,3 +407,37 @@ void InitializeMacEventGeneratorDelegate() {
 
 }  // namespace test
 }  // namespace views
+
+@implementation NSEventDonor
+
+// Donate +[NSEvent pressedMouseButtons] by retrieving the flags from the
+// active generator.
++ (NSUInteger)pressedMouseButtons {
+  if (!g_active_generator)
+    return [NSEventDonor pressedMouseButtons];  // Call original implementation.
+
+  int flags = g_active_generator->flags();
+  NSUInteger bitmask = 0;
+  if (flags & ui::EF_LEFT_MOUSE_BUTTON)
+    bitmask |= 1;
+  if (flags & ui::EF_RIGHT_MOUSE_BUTTON)
+    bitmask |= 1 << 1;
+  if (flags & ui::EF_MIDDLE_MOUSE_BUTTON)
+    bitmask |= 1 << 2;
+  return bitmask;
+}
+
+@end
+
+@implementation NSApplicationDonor
+
+- (NSEvent*)currentEvent {
+  if (g_current_event)
+    return g_current_event;
+
+  // Find the original implementation and invoke it.
+  IMP original = EventGeneratorDelegateMac::GetInstance()->CurrentEventMethod();
+  return original(self, _cmd);
+}
+
+@end