Reland: [macOS] performKeyEquivalent cleanup (#46377)

Fixes the issue in original PR where `FlutterViewWrapper` does not pass the key equivalent to subviews thus prevents `TextInputPlugin` from receiving it. Also adds a regression test for this scenario.

Note that key equivalent flow does not respect the regular responder chain. It is passed from root view to down to subviews and if unhandled will be forwarded to menus.

Original message: 

#40706 added a duplicate `NSEvent (KeyEquivalentMarker)` category. This PR removes it. It also removes the call to `markAsKeyEquivalent` from `FlutterViewController`. That call is internal to `FlutterTextInputPlugin` and classes outside should not be calling it.

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: knopp

shell/platform/darwin/macos/framework/Source/FlutterViewController.mm

@@ -164,8 +164,6 @@ @interface FlutterViewWrapper : NSView
 
 - (void)setBackgroundColor:(NSColor*)color;
 
-- (BOOL)performKeyEquivalent:(NSEvent*)event;
-
 @end
 
 /**
@@ -242,37 +240,6 @@ - (void)onKeyboardLayoutChanged;
 
 @end
 
-#pragma mark - NSEvent (KeyEquivalentMarker) protocol
-
-@interface NSEvent (KeyEquivalentMarker)
-
-// Internally marks that the event was received through performKeyEquivalent:.
-// When text editing is active, keyboard events that have modifier keys pressed
-// are received through performKeyEquivalent: instead of keyDown:. If such event
-// is passed to TextInputContext but doesn't result in a text editing action it
-// needs to be forwarded by FlutterKeyboardManager to the next responder.
-- (void)markAsKeyEquivalent;
-
-// Returns YES if the event is marked as a key equivalent.
-- (BOOL)isKeyEquivalent;
-
-@end
-
-@implementation NSEvent (KeyEquivalentMarker)
-
-// This field doesn't need a value because only its address is used as a unique identifier.
-static char markerKey;
-
-- (void)markAsKeyEquivalent {
-  objc_setAssociatedObject(self, &markerKey, @true, OBJC_ASSOCIATION_RETAIN);
-}
-
-- (BOOL)isKeyEquivalent {
-  return [objc_getAssociatedObject(self, &markerKey) boolValue] == YES;
-}
-
-@end
-
 #pragma mark - Private dependant functions
 
 namespace {
@@ -312,19 +279,15 @@ - (void)setBackgroundColor:(NSColor*)color {
 }
 
 - (BOOL)performKeyEquivalent:(NSEvent*)event {
-  if ([_controller isDispatchingKeyEvent:event]) {
-    // When NSWindow is nextResponder, keyboard manager will send to it
-    // unhandled events (through [NSWindow keyDown:]). If event has both
-    // control and cmd modifiers set (i.e. cmd+control+space - emoji picker)
-    // NSWindow will then send this event as performKeyEquivalent: to first
-    // responder, which might be FlutterTextInputPlugin. If that's the case, the
-    // plugin must not handle the event, otherwise the emoji picker would not
-    // work (due to first responder returning YES from performKeyEquivalent:)
-    // and there would be an infinite loop, because FlutterViewController will
-    // send the event back to [keyboardManager handleEvent:].
-    return NO;
+  // Do not intercept the event if flutterView is not first responder, otherwise this would
+  // interfere with TextInputPlugin, which also handles key equivalents.
+  //
+  // Also do not intercept the event if key equivalent is a product of an event being
+  // redispatched by the TextInputPlugin, in which case it needs to bubble up so that menus
+  // can handle key equivalents.
+  if (self.window.firstResponder != _flutterView || [_controller isDispatchingKeyEvent:event]) {
+    return [super performKeyEquivalent:event];
   }
-  [event markAsKeyEquivalent];
   [_flutterView keyDown:event];
   return YES;
 }

shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm

@@ -60,6 +60,7 @@ - (bool)testKeyEventsAreSentToFramework;
 - (bool)testKeyEventsArePropagatedIfNotHandled;
 - (bool)testKeyEventsAreNotPropagatedIfHandled;
 - (bool)testCtrlTabKeyEventIsPropagated;
+- (bool)testKeyEquivalentIsPassedToTextInputPlugin;
 - (bool)testFlagsChangedEventsArePropagatedIfNotHandled;
 - (bool)testKeyboardIsRestartedOnEngineRestart;
 - (bool)testTrackpadGesturesAreSentToFramework;
@@ -215,6 +216,10 @@ id MockGestureEvent(NSEventType type, NSEventPhase phase, double magnification,
   ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testCtrlTabKeyEventIsPropagated]);
 }
 
+TEST(FlutterViewControllerTest, TestKeyEquivalentIsPassedToTextInputPlugin) {
+  ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyEquivalentIsPassedToTextInputPlugin]);
+}
+
 TEST(FlutterViewControllerTest, TestFlagsChangedEventsArePropagatedIfNotHandled) {
   ASSERT_TRUE(
       [[FlutterViewControllerTestObjC alloc] testFlagsChangedEventsArePropagatedIfNotHandled]);
@@ -331,6 +336,64 @@ - (bool)testCtrlTabKeyEventIsPropagated {
   const uint64_t kPhysicalKeyTab = 0x7002b;
 
   [viewController viewWillAppear];  // Initializes the event channel.
+  // Creates a NSWindow so that FlutterView view can be first responder.
+  NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
+                                                 styleMask:NSBorderlessWindowMask
+                                                   backing:NSBackingStoreBuffered
+                                                     defer:NO];
+  window.contentView = viewController.view;
+  [window makeFirstResponder:viewController.flutterView];
+  [viewController.view performKeyEquivalent:event];
+
+  EXPECT_TRUE(called);
+  EXPECT_EQ(last_event.type, kFlutterKeyEventTypeDown);
+  EXPECT_EQ(last_event.physical, kPhysicalKeyTab);
+  return true;
+}
+
+- (bool)testKeyEquivalentIsPassedToTextInputPlugin {
+  id engineMock = flutter::testing::CreateMockFlutterEngine(@"");
+  __block bool called = false;
+  __block FlutterKeyEvent last_event;
+  OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {}
+                                                  callback:nil
+                                                  userData:nil])
+      .andDo((^(NSInvocation* invocation) {
+        FlutterKeyEvent* event;
+        [invocation getArgument:&event atIndex:2];
+        called = true;
+        last_event = *event;
+      }));
+  FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock
+                                                                                nibName:@""
+                                                                                 bundle:nil];
+  // Ctrl+tab
+  NSEvent* event = [NSEvent keyEventWithType:NSEventTypeKeyDown
+                                    location:NSZeroPoint
+                               modifierFlags:0x40101
+                                   timestamp:0
+                                windowNumber:0
+                                     context:nil
+                                  characters:@""
+                 charactersIgnoringModifiers:@""
+                                   isARepeat:NO
+                                     keyCode:48];
+  const uint64_t kPhysicalKeyTab = 0x7002b;
+
+  [viewController viewWillAppear];  // Initializes the event channel.
+
+  NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
+                                                 styleMask:NSBorderlessWindowMask
+                                                   backing:NSBackingStoreBuffered
+                                                     defer:NO];
+  window.contentView = viewController.view;
+
+  [viewController.view addSubview:viewController.textInputPlugin];
+
+  // Make the textInputPlugin first responder. This should still result in
+  // view controller reporting the key event.
+  [window makeFirstResponder:viewController.textInputPlugin];
+
   [viewController.view performKeyEquivalent:event];
 
   EXPECT_TRUE(called);