Reland: Use dispatchKeyEventPreIme, and handle keys sent to InputConn…

…ection.sendKeyEvent on Android (flutter#22304)

This re-lands flutter#21163, which was reverted in flutter#22004

Now that flutter/flutter#67359 has landed, this change will no longer cause spaces (and other shortcuts) to be ignored in text fields if there is no action associated with the intent, even if there is a shortcut key mapping to an intent.

It also no longer causes web test failures (as far as I can tell without submitting it: the same tests don't fail locally).

Here's the original PR description:

This switches from using dispatchKeyEvent to using dispatchKeyEventPreIme so that keys can be intercepted before they reach the IME and be handled by the framework.

It also now intercepts key events sent to InputConnection.sendKeyEvent, as some IMEs do (e.g. the Hacker's Keyboard), and sends the to Flutter before sending them to the IME (which it now only does if they are not handled by the framework).

This fixes the problem where pressing TAB on a hardware keyboard sends the tab to both the text field and to the focus traversal system.

Note that we still can't intercept all keystrokes given to a soft keyboard, only those which the soft keyboard decides to send to InputConnection.sendKeyEvent.

shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java

@@ -66,7 +66,8 @@ public AndroidKeyProcessor(
       @NonNull TextInputPlugin textInputPlugin) {
     this.keyEventChannel = keyEventChannel;
     this.textInputPlugin = textInputPlugin;
-    this.eventResponder = new EventResponder(view);
+    textInputPlugin.setKeyEventProcessor(this);
+    this.eventResponder = new EventResponder(view, textInputPlugin);
     this.keyEventChannel.setEventResponseHandler(eventResponder);
   }
 
@@ -80,53 +81,33 @@ public void destroy() {
   }
 
   /**
-   * Called when a key up event is received by the {@link FlutterView}.
+   * Called when a key event is received by the {@link FlutterView} or the {@link
+   * InputConnectionAdaptor}.
    *
    * @param keyEvent the Android key event to respond to.
    * @return true if the key event should not be propagated to other Android components. Delayed
    *     synthesis events will return false, so that other components may handle them.
    */
-  public boolean onKeyUp(@NonNull KeyEvent keyEvent) {
-    if (eventResponder.dispatchingKeyEvent) {
-      // Don't handle it if it is from our own delayed event synthesis.
+  public boolean onKeyEvent(@NonNull KeyEvent keyEvent) {
+    int action = keyEvent.getAction();
+    if (action != KeyEvent.ACTION_DOWN && action != KeyEvent.ACTION_UP) {
+      // There is theoretically a KeyEvent.ACTION_MULTIPLE, but that shouldn't
+      // be sent anymore anyhow.
       return false;
     }
-
-    Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar());
-    KeyEventChannel.FlutterKeyEvent flutterEvent =
-        new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter, eventIdSerial++);
-    keyEventChannel.keyUp(flutterEvent);
-    eventResponder.addEvent(flutterEvent.eventId, keyEvent);
-    return true;
-  }
-
-  /**
-   * Called when a key down event is received by the {@link FlutterView}.
-   *
-   * @param keyEvent the Android key event to respond to.
-   * @return true if the key event should not be propagated to other Android components. Delayed
-   *     synthesis events will return false, so that other components may handle them.
-   */
-  public boolean onKeyDown(@NonNull KeyEvent keyEvent) {
     if (eventResponder.dispatchingKeyEvent) {
       // Don't handle it if it is from our own delayed event synthesis.
       return false;
     }
 
-    // If the textInputPlugin is still valid and accepting text, then we'll try
-    // and send the key event to it, assuming that if the event can be sent,
-    // that it has been handled.
-    if (textInputPlugin.getLastInputConnection() != null
-        && textInputPlugin.getInputMethodManager().isAcceptingText()) {
-      if (textInputPlugin.getLastInputConnection().sendKeyEvent(keyEvent)) {
-        return true;
-      }
-    }
-
     Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar());
     KeyEventChannel.FlutterKeyEvent flutterEvent =
         new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter, eventIdSerial++);
-    keyEventChannel.keyDown(flutterEvent);
+    if (action == KeyEvent.ACTION_DOWN) {
+      keyEventChannel.keyDown(flutterEvent);
+    } else {
+      keyEventChannel.keyUp(flutterEvent);
+    }
     eventResponder.addEvent(flutterEvent.eventId, keyEvent);
     return true;
   }
@@ -196,10 +177,12 @@ private static class EventResponder implements KeyEventChannel.EventResponseHand
     private static final long MAX_PENDING_EVENTS = 1000;
     final Deque<Entry<Long, KeyEvent>> pendingEvents = new ArrayDeque<Entry<Long, KeyEvent>>();
     @NonNull private final View view;
+    @NonNull private final TextInputPlugin textInputPlugin;
     boolean dispatchingKeyEvent = false;
 
-    public EventResponder(@NonNull View view) {
+    public EventResponder(@NonNull View view, @NonNull TextInputPlugin textInputPlugin) {
       this.view = view;
+      this.textInputPlugin = textInputPlugin;
     }
 
     /**
@@ -267,12 +250,26 @@ public void addEvent(long id, @NonNull KeyEvent event) {
      * @param event the event to be dispatched to the activity.
      */
     public void dispatchKeyEvent(KeyEvent event) {
+      // If the textInputPlugin is still valid and accepting text, then we'll try
+      // and send the key event to it, assuming that if the event can be sent,
+      // that it has been handled.
+      if (textInputPlugin.getLastInputConnection() != null
+          && textInputPlugin.getInputMethodManager().isAcceptingText()) {
+        dispatchingKeyEvent = true;
+        boolean handled = textInputPlugin.getLastInputConnection().sendKeyEvent(event);
+        dispatchingKeyEvent = false;
+        if (handled) {
+          return;
+        }
+      }
+
       // Since the framework didn't handle it, dispatch the key again.
       if (view != null) {
         // Turn on dispatchingKeyEvent so that we don't dispatch to ourselves and
         // send it to the framework again.
         dispatchingKeyEvent = true;
-        view.getRootView().dispatchKeyEvent(event);
+
+        view.getRootView().dispatchKeyEventPreIme(event);
         dispatchingKeyEvent = false;
       }
     }

shell/platform/android/io/flutter/embedding/android/FlutterView.java

@@ -721,27 +721,7 @@ public boolean checkInputConnectionProxy(View view) {
   }
 
   /**
-   * Invoked when key is released.
-   *
-   * <p>This method is typically invoked in response to the release of a physical keyboard key or a
-   * D-pad button. It is generally not invoked when a virtual software keyboard is used, though a
-   * software keyboard may choose to invoke this method in some situations.
-   *
-   * <p>{@link KeyEvent}s are sent from Android to Flutter. {@link AndroidKeyProcessor} may do some
-   * additional work with the given {@link KeyEvent}, e.g., combine this {@code keyCode} with the
-   * previous {@code keyCode} to generate a unicode combined character.
-   */
-  @Override
-  public boolean onKeyUp(int keyCode, @NonNull KeyEvent event) {
-    if (!isAttachedToFlutterEngine()) {
-      return super.onKeyUp(keyCode, event);
-    }
-
-    return androidKeyProcessor.onKeyUp(event) || super.onKeyUp(keyCode, event);
-  }
-
-  /**
-   * Invoked when key is pressed.
+   * Invoked when a hardware key is pressed or released, before the IME receives the key.
    *
    * <p>This method is typically invoked in response to the press of a physical keyboard key or a
    * D-pad button. It is generally not invoked when a virtual software keyboard is used, though a
@@ -752,12 +732,13 @@ public boolean onKeyUp(int keyCode, @NonNull KeyEvent event) {
    * previous {@code keyCode} to generate a unicode combined character.
    */
   @Override
-  public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
-    if (!isAttachedToFlutterEngine()) {
-      return super.onKeyDown(keyCode, event);
-    }
-
-    return androidKeyProcessor.onKeyDown(event) || super.onKeyDown(keyCode, event);
+  public boolean dispatchKeyEventPreIme(KeyEvent event) {
+    // If the key processor doesn't handle it, then send it on to the
+    // superclass. The key processor will typically handle all events except
+    // those where it has re-dispatched the event after receiving a reply from
+    // the framework that the framework did not handle it.
+    return (isAttachedToFlutterEngine() && androidKeyProcessor.onKeyEvent(event))
+        || super.dispatchKeyEventPreIme(event);
   }
 
   /**

shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java

@@ -27,13 +27,15 @@
 import android.view.inputmethod.InputMethodManager;
 import android.view.inputmethod.InputMethodSubtype;
 import io.flutter.Log;
+import io.flutter.embedding.android.AndroidKeyProcessor;
 import io.flutter.embedding.engine.FlutterJNI;
 import io.flutter.embedding.engine.systemchannels.TextInputChannel;
 
 class InputConnectionAdaptor extends BaseInputConnection {
   private final View mFlutterView;
   private final int mClient;
   private final TextInputChannel textInputChannel;
+  private final AndroidKeyProcessor keyProcessor;
   private final Editable mEditable;
   private final EditorInfo mEditorInfo;
   private int mBatchCount;
@@ -97,6 +99,7 @@ public InputConnectionAdaptor(
       View view,
       int client,
       TextInputChannel textInputChannel,
+      AndroidKeyProcessor keyProcessor,
       Editable editable,
       EditorInfo editorInfo,
       FlutterJNI flutterJNI) {
@@ -107,6 +110,7 @@ public InputConnectionAdaptor(
     mEditable = editable;
     mEditorInfo = editorInfo;
     mBatchCount = 0;
+    this.keyProcessor = keyProcessor;
     this.flutterTextUtils = new FlutterTextUtils(flutterJNI);
     // We create a dummy Layout with max width so that the selection
     // shifting acts as if all text were in one line.
@@ -128,9 +132,10 @@ public InputConnectionAdaptor(
       View view,
       int client,
       TextInputChannel textInputChannel,
+      AndroidKeyProcessor keyProcessor,
       Editable editable,
       EditorInfo editorInfo) {
-    this(view, client, textInputChannel, editable, editorInfo, new FlutterJNI());
+    this(view, client, textInputChannel, keyProcessor, editable, editorInfo, new FlutterJNI());
   }
 
   // Send the current state of the editable to Flutter.
@@ -323,6 +328,14 @@ private static int clampIndexToEditable(int index, Editable editable) {
 
   @Override
   public boolean sendKeyEvent(KeyEvent event) {
+    // Give the key processor a chance to process this event.  It will send it
+    // to the framework to be handled and return true. If the framework ends up
+    // not handling it, the processor will re-send the event, this time
+    // returning false so that it can be processed here.
+    if (keyProcessor != null && keyProcessor.onKeyEvent(event)) {
+      return true;
+    }
+
     markDirty();
     if (event.getAction() == KeyEvent.ACTION_DOWN) {
       if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

@@ -28,6 +28,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
+import io.flutter.embedding.android.AndroidKeyProcessor;
 import io.flutter.embedding.engine.systemchannels.TextInputChannel;
 import io.flutter.plugin.platform.PlatformViewsController;
 import java.util.HashMap;
@@ -48,6 +49,7 @@ public class TextInputPlugin {
   @Nullable private Rect lastClientRect;
   private final boolean restartAlwaysRequired;
   private ImeSyncDeferringInsetsCallback imeSyncCallback;
+  private AndroidKeyProcessor keyProcessor;
 
   // When true following calls to createInputConnection will return the cached lastInputConnection
   // if the input
@@ -172,6 +174,15 @@ ImeSyncDeferringInsetsCallback getImeSyncCallback() {
     return imeSyncCallback;
   }
 
+  @NonNull
+  public AndroidKeyProcessor getKeyEventProcessor() {
+    return keyProcessor;
+  }
+
+  public void setKeyEventProcessor(AndroidKeyProcessor processor) {
+    keyProcessor = processor;
+  }
+
   /**
    * Use the current platform view input connection until unlockPlatformViewInputConnection is
    * called.
@@ -313,7 +324,8 @@ public InputConnection createInputConnection(View view, EditorInfo outAttrs) {
     outAttrs.imeOptions |= enterAction;
 
     InputConnectionAdaptor connection =
-        new InputConnectionAdaptor(view, inputTarget.id, textInputChannel, mEditable, outAttrs);
+        new InputConnectionAdaptor(
+            view, inputTarget.id, textInputChannel, keyProcessor, mEditable, outAttrs);
     outAttrs.initialSelStart = Selection.getSelectionStart(mEditable);
     outAttrs.initialSelEnd = Selection.getSelectionEnd(mEditable);
 

shell/platform/android/io/flutter/view/FlutterView.java

@@ -269,19 +269,9 @@ public DartExecutor getDartExecutor() {
   }
 
   @Override
-  public boolean onKeyUp(int keyCode, KeyEvent event) {
-    if (!isAttached()) {
-      return super.onKeyUp(keyCode, event);
-    }
-    return androidKeyProcessor.onKeyUp(event) || super.onKeyUp(keyCode, event);
-  }
-
-  @Override
-  public boolean onKeyDown(int keyCode, KeyEvent event) {
-    if (!isAttached()) {
-      return super.onKeyDown(keyCode, event);
-    }
-    return androidKeyProcessor.onKeyDown(event) || super.onKeyDown(keyCode, event);
+  public boolean dispatchKeyEventPreIme(KeyEvent event) {
+    return (isAttached() && androidKeyProcessor.onKeyEvent(event))
+        || super.dispatchKeyEventPreIme(event);
   }
 
   public FlutterNativeView getFlutterNativeView() {

shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java

@@ -51,11 +51,11 @@ public void respondsTrueWhenHandlingNewEvents() {
     AndroidKeyProcessor processor =
         new AndroidKeyProcessor(fakeView, fakeKeyEventChannel, mock(TextInputPlugin.class));
 
-    boolean result = processor.onKeyDown(new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65));
+    boolean result = processor.onKeyEvent(new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65));
     assertEquals(true, result);
     verify(fakeKeyEventChannel, times(1)).keyDown(any(KeyEventChannel.FlutterKeyEvent.class));
     verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
-    verify(fakeView, times(0)).dispatchKeyEvent(any(KeyEvent.class));
+    verify(fakeView, times(0)).dispatchKeyEventPreIme(any(KeyEvent.class));
   }
 
   @Test
@@ -97,31 +97,31 @@ public View answer(InvocationOnMock invocation) throws Throwable {
         ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
     FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);
 
-    boolean result = processor.onKeyDown(fakeKeyEvent);
+    boolean result = processor.onKeyEvent(fakeKeyEvent);
     assertEquals(true, result);
 
     // Capture the FlutterKeyEvent so we can find out its event ID to use when
     // faking our response.
     verify(fakeKeyEventChannel, times(1)).keyDown(eventCaptor.capture());
     boolean[] dispatchResult = {true};
-    when(fakeView.dispatchKeyEvent(any(KeyEvent.class)))
+    when(fakeView.dispatchKeyEventPreIme(any(KeyEvent.class)))
         .then(
             new Answer<Boolean>() {
               @Override
               public Boolean answer(InvocationOnMock invocation) throws Throwable {
                 KeyEvent event = (KeyEvent) invocation.getArguments()[0];
                 assertEquals(fakeKeyEvent, event);
-                dispatchResult[0] = processor.onKeyDown(event);
+                dispatchResult[0] = processor.onKeyEvent(event);
                 return dispatchResult[0];
               }
             });
 
     // Fake a response from the framework.
     handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().eventId);
-    verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
+    verify(fakeView, times(1)).dispatchKeyEventPreIme(fakeKeyEvent);
     assertEquals(false, dispatchResult[0]);
     verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
-    verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent);
+    verify(fakeRootView, times(1)).dispatchKeyEventPreIme(fakeKeyEvent);
   }
 
   public void synthesizesEventsWhenKeyUpNotHandled() {
@@ -147,31 +147,31 @@ public View answer(InvocationOnMock invocation) throws Throwable {
         ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
     FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_UP, 65);
 
-    boolean result = processor.onKeyUp(fakeKeyEvent);
+    boolean result = processor.onKeyEvent(fakeKeyEvent);
     assertEquals(true, result);
 
     // Capture the FlutterKeyEvent so we can find out its event ID to use when
     // faking our response.
     verify(fakeKeyEventChannel, times(1)).keyUp(eventCaptor.capture());
     boolean[] dispatchResult = {true};
-    when(fakeView.dispatchKeyEvent(any(KeyEvent.class)))
+    when(fakeView.dispatchKeyEventPreIme(any(KeyEvent.class)))
         .then(
             new Answer<Boolean>() {
               @Override
               public Boolean answer(InvocationOnMock invocation) throws Throwable {
                 KeyEvent event = (KeyEvent) invocation.getArguments()[0];
                 assertEquals(fakeKeyEvent, event);
-                dispatchResult[0] = processor.onKeyUp(event);
+                dispatchResult[0] = processor.onKeyEvent(event);
                 return dispatchResult[0];
               }
             });
 
     // Fake a response from the framework.
     handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().eventId);
-    verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
+    verify(fakeView, times(1)).dispatchKeyEventPreIme(fakeKeyEvent);
     assertEquals(false, dispatchResult[0]);
     verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
-    verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent);
+    verify(fakeRootView, times(1)).dispatchKeyEventPreIme(fakeKeyEvent);
   }
 
   @NonNull