From 40fa345c02e17a67293e7496e2613e13cf201dd8 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 1 Dec 2020 09:42:38 -0800 Subject: [PATCH] Fix race condition in key event handling on Android (#22658) This fixes a problem in Android key event handling where, because I was only using a single bool to indicate that we were re-dispatching, there was a race condition when multiple keys were pending (sent to the framework, awaiting responses). This fixes that by switching to a mechanism that uses the event itself to tell if it was redispatched. In doing this, I realized that because key events can come from either the dispatchEvent call, or through the InputConnectionAdaptor, I needed to handle both routes properly so that the events would all be handled, and all go through the same mechanism on the framework side. --- .../android/AndroidKeyProcessor.java | 109 ++++----- .../embedding/android/FlutterView.java | 5 - .../systemchannels/KeyEventChannel.java | 208 ++++-------------- .../editing/InputConnectionAdaptor.java | 19 +- .../android/io/flutter/view/FlutterView.java | 7 - .../android/AndroidKeyProcessorTest.java | 4 +- .../systemchannels/KeyEventChannelTest.java | 28 +-- .../editing/InputConnectionAdaptorTest.java | 43 +++- 8 files changed, 169 insertions(+), 254 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java index dba036e73113e..33988c83aaf0c 100644 --- a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java +++ b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java @@ -12,10 +12,8 @@ import io.flutter.Log; import io.flutter.embedding.engine.systemchannels.KeyEventChannel; import io.flutter.plugin.editing.TextInputPlugin; -import java.util.AbstractMap.SimpleImmutableEntry; import java.util.ArrayDeque; import java.util.Deque; -import java.util.Map.Entry; /** * A class to process key events from Android, passing them to the framework as messages using @@ -33,7 +31,6 @@ */ public class AndroidKeyProcessor { private static final String TAG = "AndroidKeyProcessor"; - private static long eventIdSerial = 0; @NonNull private final KeyEventChannel keyEventChannel; @NonNull private final TextInputPlugin textInputPlugin; @@ -50,8 +47,8 @@ public class AndroidKeyProcessor { *

It is possible that that in the middle of the async round trip, the focus chain could * change, and instead of the native widget that was "next" when the event was fired getting the * event, it may be the next widget when the event is synthesized that gets it. In practice, this - * shouldn't be a huge problem, as this is an unlikely occurance to happen without user input, and - * it may actually be desired behavior, but it is possible. + * shouldn't be a huge problem, as this is an unlikely occurrence to happen without user input, + * and it may actually be desired behavior, but it is possible. * * @param view takes the activity to use for re-dispatching of events that were not handled by the * framework. @@ -96,23 +93,39 @@ public boolean onKeyEvent(@NonNull KeyEvent keyEvent) { // case the theory is wrong. return false; } - if (eventResponder.dispatchingKeyEvent) { - // Don't handle it if it is from our own delayed event dispatch. + if (eventResponder.isHeadEvent(keyEvent)) { + // If the keyEvent is at the head of the queue of pending events we've seen, + // and has the same id, then we know that this is a re-dispatched keyEvent, and + // we shouldn't respond to it, but we should remove it from tracking now. + eventResponder.removeHeadEvent(); return false; } Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar()); KeyEventChannel.FlutterKeyEvent flutterEvent = - new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter, eventIdSerial++); + new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter); + + eventResponder.addEvent(keyEvent); if (action == KeyEvent.ACTION_DOWN) { keyEventChannel.keyDown(flutterEvent); } else { keyEventChannel.keyUp(flutterEvent); } - eventResponder.addEvent(flutterEvent.eventId, keyEvent); return true; } + /** + * Returns whether or not the given event is currently being processed by this key processor. This + * is used to determine if a new key event sent to the {@link InputConnectionAdaptor} originates + * from a hardware key event, or a soft keyboard editing event. + * + * @param event the event to check for being the current event. + * @return + */ + public boolean isCurrentEvent(@NonNull KeyEvent event) { + return eventResponder.isHeadEvent(event); + } + /** * Applies the given Unicode character in {@code newCharacterCodePoint} to a previously entered * Unicode combining character and returns the combination of these characters if a combination @@ -176,65 +189,63 @@ private static class EventResponder implements KeyEventChannel.EventResponseHand // The maximum number of pending events that are held before starting to // complain. private static final long MAX_PENDING_EVENTS = 1000; - final Deque> pendingEvents = new ArrayDeque>(); + final Deque pendingEvents = new ArrayDeque(); @NonNull private final View view; @NonNull private final TextInputPlugin textInputPlugin; - boolean dispatchingKeyEvent = false; public EventResponder(@NonNull View view, @NonNull TextInputPlugin textInputPlugin) { this.view = view; this.textInputPlugin = textInputPlugin; } - /** - * Removes the pending event with the given id from the cache of pending events. - * - * @param id the id of the event to be removed. - */ - private KeyEvent removePendingEvent(long id) { - if (pendingEvents.getFirst().getKey() != id) { + /** Removes the first pending event from the cache of pending events. */ + private KeyEvent removeHeadEvent() { + return pendingEvents.removeFirst(); + } + + private KeyEvent checkIsHeadEvent(KeyEvent event) { + if (pendingEvents.size() == 0) { + throw new AssertionError( + "Event response received when no events are in the queue. Received event " + event); + } + if (pendingEvents.getFirst() != event) { throw new AssertionError( "Event response received out of order. Should have seen event " - + pendingEvents.getFirst().getKey() + + pendingEvents.getFirst() + " first. Instead, received " - + id); + + event); } - return pendingEvents.removeFirst().getValue(); + return pendingEvents.getFirst(); + } + + private boolean isHeadEvent(KeyEvent event) { + return pendingEvents.size() > 0 && pendingEvents.getFirst() == event; } /** * Called whenever the framework responds that a given key event was handled by the framework. * - * @param id the event id of the event to be marked as being handled by the framework. Must not - * be null. + * @param event the event to be marked as being handled by the framework. Must not be null. */ @Override - public void onKeyEventHandled(long id) { - removePendingEvent(id); + public void onKeyEventHandled(KeyEvent event) { + removeHeadEvent(); } /** * Called whenever the framework responds that a given key event wasn't handled by the * framework. * - * @param id the event id of the event to be marked as not being handled by the framework. Must - * not be null. + * @param event the event to be marked as not being handled by the framework. Must not be null. */ @Override - public void onKeyEventNotHandled(long id) { - dispatchKeyEvent(removePendingEvent(id)); + public void onKeyEventNotHandled(KeyEvent event) { + redispatchKeyEvent(checkIsHeadEvent(event)); } - /** Adds an Android key event with an id to the event responder to wait for a response. */ - public void addEvent(long id, @NonNull KeyEvent event) { - if (pendingEvents.size() > 0 && pendingEvents.getFirst().getKey() >= id) { - throw new AssertionError( - "New events must have ids greater than the most recent pending event. New id " - + id - + " is less than or equal to the last event id of " - + pendingEvents.getFirst().getKey()); - } - pendingEvents.addLast(new SimpleImmutableEntry(id, event)); + /** Adds an Android key event to the event responder to wait for a response. */ + public void addEvent(@NonNull KeyEvent event) { + pendingEvents.addLast(event); if (pendingEvents.size() > MAX_PENDING_EVENTS) { Log.e( TAG, @@ -250,27 +261,21 @@ public void addEvent(long id, @NonNull KeyEvent event) { * * @param event the event to be dispatched to the activity. */ - public void dispatchKeyEvent(KeyEvent event) { + private void redispatchKeyEvent(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; - } + if (textInputPlugin.getInputMethodManager().isAcceptingText() + && textInputPlugin.getLastInputConnection() != null + && textInputPlugin.getLastInputConnection().sendKeyEvent(event)) { + // The event was handled, so we can remove it from the queue. + removeHeadEvent(); + return; } // Since the framework didn't handle it, dispatch the event 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); - dispatchingKeyEvent = false; } } } diff --git a/shell/platform/android/io/flutter/embedding/android/FlutterView.java b/shell/platform/android/io/flutter/embedding/android/FlutterView.java index 8f3e4b9eb8603..2b886bc9c2475 100644 --- a/shell/platform/android/io/flutter/embedding/android/FlutterView.java +++ b/shell/platform/android/io/flutter/embedding/android/FlutterView.java @@ -739,11 +739,6 @@ public boolean dispatchKeyEvent(KeyEvent event) { } else if (event.getAction() == KeyEvent.ACTION_UP) { // Stop tracking the event. getKeyDispatcherState().handleUpEvent(event); - if (!event.isTracking() || event.isCanceled()) { - // Don't send the event to the key processor if it was canceled, or no - // longer being tracked. - return super.dispatchKeyEvent(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 diff --git a/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java b/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java index 3638c2d364ae7..dbd6bf7f9c924 100644 --- a/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java +++ b/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java @@ -43,19 +43,17 @@ public interface EventResponseHandler { /** * Called whenever the framework responds that a given key event was handled by the framework. * - * @param id the event id of the event to be marked as being handled by the framework. Must not - * be null. + * @param event the event to be marked as being handled by the framework. Must not be null. */ - public void onKeyEventHandled(long id); + public void onKeyEventHandled(KeyEvent event); /** * Called whenever the framework responds that a given key event wasn't handled by the * framework. * - * @param id the event id of the event to be marked as not being handled by the framework. Must - * not be null. + * @param event the event to be marked as not being handled by the framework. Must not be null. */ - public void onKeyEventNotHandled(long id); + public void onKeyEventNotHandled(KeyEvent event); } /** @@ -69,11 +67,11 @@ public KeyEventChannel(@NonNull BinaryMessenger binaryMessenger) { } /** - * Creates a reply handler for this an event with the given eventId. + * Creates a reply handler for the given key event. * - * @param eventId the event ID to create a reply for. + * @param event the Android key event to create a reply for. */ - BasicMessageChannel.Reply createReplyHandler(long eventId) { + BasicMessageChannel.Reply createReplyHandler(KeyEvent event) { return message -> { if (eventResponseHandler == null) { return; @@ -81,19 +79,19 @@ BasicMessageChannel.Reply createReplyHandler(long eventId) { try { if (message == null) { - eventResponseHandler.onKeyEventNotHandled(eventId); + eventResponseHandler.onKeyEventNotHandled(event); return; } final JSONObject annotatedEvent = (JSONObject) message; final boolean handled = annotatedEvent.getBoolean("handled"); if (handled) { - eventResponseHandler.onKeyEventHandled(eventId); + eventResponseHandler.onKeyEventHandled(event); } else { - eventResponseHandler.onKeyEventNotHandled(eventId); + eventResponseHandler.onKeyEventNotHandled(event); } } catch (JSONException e) { Log.e(TAG, "Unable to unpack JSON message: " + e); - eventResponseHandler.onKeyEventNotHandled(eventId); + eventResponseHandler.onKeyEventNotHandled(event); } }; } @@ -106,7 +104,7 @@ public void keyUp(@NonNull FlutterKeyEvent keyEvent) { message.put("keymap", "android"); encodeKeyEvent(keyEvent, message); - channel.send(message, createReplyHandler(keyEvent.eventId)); + channel.send(message, createReplyHandler(keyEvent.event)); } public void keyDown(@NonNull FlutterKeyEvent keyEvent) { @@ -115,176 +113,58 @@ public void keyDown(@NonNull FlutterKeyEvent keyEvent) { message.put("keymap", "android"); encodeKeyEvent(keyEvent, message); - channel.send(message, createReplyHandler(keyEvent.eventId)); + channel.send(message, createReplyHandler(keyEvent.event)); } private void encodeKeyEvent( - @NonNull FlutterKeyEvent event, @NonNull Map message) { - message.put("flags", event.flags); - message.put("plainCodePoint", event.plainCodePoint); - message.put("codePoint", event.codePoint); - message.put("keyCode", event.keyCode); - message.put("scanCode", event.scanCode); - message.put("metaState", event.metaState); - if (event.complexCharacter != null) { - message.put("character", event.complexCharacter.toString()); + @NonNull FlutterKeyEvent keyEvent, @NonNull Map message) { + message.put("flags", keyEvent.event.getFlags()); + message.put("plainCodePoint", keyEvent.event.getUnicodeChar(0x0)); + message.put("codePoint", keyEvent.event.getUnicodeChar()); + message.put("keyCode", keyEvent.event.getKeyCode()); + message.put("scanCode", keyEvent.event.getScanCode()); + message.put("metaState", keyEvent.event.getMetaState()); + if (keyEvent.complexCharacter != null) { + message.put("character", keyEvent.complexCharacter.toString()); } - message.put("source", event.source); - message.put("vendorId", event.vendorId); - message.put("productId", event.productId); - message.put("deviceId", event.deviceId); - message.put("repeatCount", event.repeatCount); + message.put("source", keyEvent.event.getSource()); + InputDevice device = InputDevice.getDevice(keyEvent.event.getDeviceId()); + int vendorId = 0; + int productId = 0; + if (device != null) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { + vendorId = device.getVendorId(); + productId = device.getProductId(); + } + } + message.put("vendorId", vendorId); + message.put("productId", productId); + message.put("deviceId", keyEvent.event.getDeviceId()); + message.put("repeatCount", keyEvent.event.getRepeatCount()); } /** A key event as defined by Flutter. */ public static class FlutterKeyEvent { /** - * The id for the device this event came from. - * - * @see KeyEvent.getDeviceId() - */ - public final int deviceId; - /** - * The flags for this key event. - * - * @see KeyEvent.getFlags() - */ - public final int flags; - /** - * The code point for the Unicode character produced by this event if no meta keys were pressed - * (by passing 0 to {@code KeyEvent.getUnicodeChar(int)}). - * - * @see KeyEvent.getUnicodeChar(int) - */ - public final int plainCodePoint; - /** - * The code point for the Unicode character produced by this event, taking into account the meta - * keys currently pressed. + * The Android key event that this Flutter key event was created from. * - * @see KeyEvent.getUnicodeChar() - */ - public final int codePoint; - /** - * The Android key code for this event. - * - * @see KeyEvent.getKeyCode() + *

This event is used to identify pending events when results are received from the + * framework. */ - public final int keyCode; + public final KeyEvent event; /** * The character produced by this event, including any combining characters pressed before it. */ @Nullable public final Character complexCharacter; - /** - * The Android scan code for the key pressed. - * - * @see KeyEvent.getScanCode() - */ - public final int scanCode; - /** - * The meta key state for the Android key event. - * - * @see KeyEvent.getMetaState() - */ - public final int metaState; - /** - * The source of the key event. - * - * @see KeyEvent.getSource() - */ - public final int source; - /** - * The vendorId of the device that produced this key event. - * - * @see InputDevice.getVendorId() - */ - public final int vendorId; - /** - * The productId of the device that produced this key event. - * - * @see InputDevice.getProductId() - */ - public final int productId; - /** - * The repeat count for this event. - * - * @see KeyEvent.getRepeatCount() - */ - public final int repeatCount; - /** - * The unique id for this Flutter key event. - * - *

This id is used to identify pending events when results are received from the framework. - * This ID does not come from Android. - */ - public final long eventId; - - public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent, long eventId) { - this(androidKeyEvent, null, eventId); - } - public FlutterKeyEvent( - @NonNull KeyEvent androidKeyEvent, @Nullable Character complexCharacter, long eventId) { - this( - androidKeyEvent.getDeviceId(), - androidKeyEvent.getFlags(), - androidKeyEvent.getUnicodeChar(0x0), - androidKeyEvent.getUnicodeChar(), - androidKeyEvent.getKeyCode(), - complexCharacter, - androidKeyEvent.getScanCode(), - androidKeyEvent.getMetaState(), - androidKeyEvent.getSource(), - androidKeyEvent.getRepeatCount(), - eventId); + public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent) { + this(androidKeyEvent, null); } public FlutterKeyEvent( - int deviceId, - int flags, - int plainCodePoint, - int codePoint, - int keyCode, - @Nullable Character complexCharacter, - int scanCode, - int metaState, - int source, - int repeatCount, - long eventId) { - this.deviceId = deviceId; - this.flags = flags; - this.plainCodePoint = plainCodePoint; - this.codePoint = codePoint; - this.keyCode = keyCode; + @NonNull KeyEvent androidKeyEvent, @Nullable Character complexCharacter) { + this.event = androidKeyEvent; this.complexCharacter = complexCharacter; - this.scanCode = scanCode; - this.metaState = metaState; - this.source = source; - this.repeatCount = repeatCount; - this.eventId = eventId; - InputDevice device = InputDevice.getDevice(deviceId); - if (device != null) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { - this.vendorId = device.getVendorId(); - this.productId = device.getProductId(); - } else { - this.vendorId = 0; - this.productId = 0; - } - } else { - this.vendorId = 0; - this.productId = 0; - } } } } diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index eceadf91dc837..d65275210b880 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -285,13 +285,22 @@ private static int clampIndexToEditable(int index, Editable editable) { return clamped; } + // This function is called both when hardware key events occur and aren't + // handled by the framework, as well as when soft keyboard editing events + // occur, and need a chance to be handled by the framework. @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)) { + // This gives the key processor a chance to process this event if it came + // from a soft keyboard. 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 to this function. Only do this if the event is not the + // current event, since that indicates that the key processor sent it to us, + // and we only want to call the key processor for events that it doesn't + // already know about (i.e. when events arrive here from a soft keyboard and + // not a hardware keyboard), to avoid a loop. + if (keyProcessor != null + && !keyProcessor.isCurrentEvent(event) + && keyProcessor.onKeyEvent(event)) { return true; } diff --git a/shell/platform/android/io/flutter/view/FlutterView.java b/shell/platform/android/io/flutter/view/FlutterView.java index b83c7e90cbf5d..5e8c7cbc36819 100644 --- a/shell/platform/android/io/flutter/view/FlutterView.java +++ b/shell/platform/android/io/flutter/view/FlutterView.java @@ -21,7 +21,6 @@ import android.util.AttributeSet; import android.util.SparseArray; import android.view.DisplayCutout; -import android.view.KeyEvent; import android.view.MotionEvent; import android.view.PointerIcon; import android.view.Surface; @@ -268,12 +267,6 @@ public DartExecutor getDartExecutor() { return dartExecutor; } - @Override - public boolean dispatchKeyEventPreIme(KeyEvent event) { - return (isAttached() && androidKeyProcessor.onKeyEvent(event)) - || super.dispatchKeyEventPreIme(event); - } - public FlutterNativeView getFlutterNativeView() { return mNativeView; } diff --git a/shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java b/shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java index 6327f0ed6baa6..64b0f10ce63db 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/AndroidKeyProcessorTest.java @@ -117,7 +117,7 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable { }); // Fake a response from the framework. - handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().eventId); + handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event); verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent); assertEquals(false, dispatchResult[0]); verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class)); @@ -167,7 +167,7 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable { }); // Fake a response from the framework. - handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().eventId); + handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event); verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent); assertEquals(false, dispatchResult[0]); verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class)); diff --git a/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/KeyEventChannelTest.java b/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/KeyEventChannelTest.java index 81b34b3f3b5ad..23645ed9f2829 100644 --- a/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/KeyEventChannelTest.java +++ b/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/KeyEventChannelTest.java @@ -45,24 +45,24 @@ public void keyDownEventIsSentToFramework() throws JSONException { BinaryMessenger fakeMessenger = mock(BinaryMessenger.class); KeyEventChannel keyEventChannel = new KeyEventChannel(fakeMessenger); final boolean[] handled = {false}; - final long[] handledId = {-1}; + final KeyEvent[] handledKeyEvents = {null}; keyEventChannel.setEventResponseHandler( new KeyEventChannel.EventResponseHandler() { - public void onKeyEventHandled(@NonNull long id) { + public void onKeyEventHandled(@NonNull KeyEvent event) { handled[0] = true; - handledId[0] = id; + handledKeyEvents[0] = event; } - public void onKeyEventNotHandled(@NonNull long id) { + public void onKeyEventNotHandled(@NonNull KeyEvent event) { handled[0] = false; - handledId[0] = id; + handledKeyEvents[0] = event; } }); verify(fakeMessenger, times(0)).send(any(), any(), any()); KeyEvent event = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65); KeyEventChannel.FlutterKeyEvent flutterKeyEvent = - new KeyEventChannel.FlutterKeyEvent(event, null, 10); + new KeyEventChannel.FlutterKeyEvent(event, null); keyEventChannel.keyDown(flutterKeyEvent); ArgumentCaptor byteBufferArgumentCaptor = ArgumentCaptor.forClass(ByteBuffer.class); ArgumentCaptor replyArgumentCaptor = @@ -78,7 +78,7 @@ public void onKeyEventNotHandled(@NonNull long id) { // Simulate a reply, and see that it is handled. sendReply(true, replyArgumentCaptor.getValue()); assertTrue(handled[0]); - assertEquals(10, handledId[0]); + assertEquals(event, handledKeyEvents[0]); } @Test @@ -86,24 +86,24 @@ public void keyUpEventIsSentToFramework() throws JSONException { BinaryMessenger fakeMessenger = mock(BinaryMessenger.class); KeyEventChannel keyEventChannel = new KeyEventChannel(fakeMessenger); final boolean[] handled = {false}; - final long[] handledId = {-1}; + final KeyEvent[] handledKeyEvents = {null}; keyEventChannel.setEventResponseHandler( new KeyEventChannel.EventResponseHandler() { - public void onKeyEventHandled(long id) { + public void onKeyEventHandled(@NonNull KeyEvent event) { handled[0] = true; - handledId[0] = id; + handledKeyEvents[0] = event; } - public void onKeyEventNotHandled(long id) { + public void onKeyEventNotHandled(@NonNull KeyEvent event) { handled[0] = false; - handledId[0] = id; + handledKeyEvents[0] = event; } }); verify(fakeMessenger, times(0)).send(any(), any(), any()); KeyEvent event = new FakeKeyEvent(KeyEvent.ACTION_UP, 65); KeyEventChannel.FlutterKeyEvent flutterKeyEvent = - new KeyEventChannel.FlutterKeyEvent(event, null, 10); + new KeyEventChannel.FlutterKeyEvent(event, null); keyEventChannel.keyUp(flutterKeyEvent); ArgumentCaptor byteBufferArgumentCaptor = ArgumentCaptor.forClass(ByteBuffer.class); ArgumentCaptor replyArgumentCaptor = @@ -119,6 +119,6 @@ public void onKeyEventNotHandled(long id) { // Simulate a reply, and see that it is handled. sendReply(true, replyArgumentCaptor.getValue()); assertTrue(handled[0]); - assertEquals(10, handledId[0]); + assertEquals(event, handledKeyEvents[0]); } } diff --git a/shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java b/shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java index dfab9ecbe0db3..492f13fbc0899 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java @@ -9,6 +9,7 @@ import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -1028,19 +1029,47 @@ public void testCursorAnchorInfo() { assertNull(testImm.lastCursorAnchorInfo); } + @Test + public void testSendKeyEvent_sendSoftKeyEvents() { + ListenableEditingState editable = sampleEditable(5, 5); + AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class); + when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(true); + InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor); + + KeyEvent shiftKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SHIFT_LEFT); + + boolean didConsume = adaptor.sendKeyEvent(shiftKeyDown); + assertFalse(didConsume); + verify(mockKeyProcessor, never()).onKeyEvent(shiftKeyDown); + } + + @Test + public void testSendKeyEvent_sendHardwareKeyEvents() { + ListenableEditingState editable = sampleEditable(5, 5); + AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class); + when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(false); + when(mockKeyProcessor.onKeyEvent(any())).thenReturn(true); + InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor); + + KeyEvent shiftKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SHIFT_LEFT); + + boolean didConsume = adaptor.sendKeyEvent(shiftKeyDown); + assertTrue(didConsume); + verify(mockKeyProcessor, times(1)).onKeyEvent(shiftKeyDown); + } + @Test public void testSendKeyEvent_delKeyNotConsumed() { - int selStart = 29; - ListenableEditingState editable = sampleEditable(selStart, selStart, SAMPLE_RTL_TEXT); + ListenableEditingState editable = sampleEditable(5, 5); InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable); KeyEvent downKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL); - for (int i = 0; i < 10; i++) { + for (int i = 0; i < 4; i++) { boolean didConsume = adaptor.sendKeyEvent(downKeyDown); assertFalse(didConsume); } - assertEquals(29, Selection.getSelectionStart(editable)); + assertEquals(5, Selection.getSelectionStart(editable)); } @Test @@ -1097,11 +1126,15 @@ private static ListenableEditingState sampleEditable(int selStart, int selEnd, S private static InputConnectionAdaptor sampleInputConnectionAdaptor( ListenableEditingState editable) { + return sampleInputConnectionAdaptor(editable, mock(AndroidKeyProcessor.class)); + } + + private static InputConnectionAdaptor sampleInputConnectionAdaptor( + ListenableEditingState editable, AndroidKeyProcessor mockKeyProcessor) { View testView = new View(RuntimeEnvironment.application); int client = 0; TextInputChannel textInputChannel = mock(TextInputChannel.class); FlutterJNI mockFlutterJNI = mock(FlutterJNI.class); - AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class); when(mockFlutterJNI.nativeFlutterTextUtilsIsEmoji(anyInt())) .thenAnswer((invocation) -> Emoji.isEmoji((int) invocation.getArguments()[0])); when(mockFlutterJNI.nativeFlutterTextUtilsIsEmojiModifier(anyInt()))