From 63be4d3922bef9cbab63011f93bdc8025a1e206c Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 19 Nov 2020 18:17:04 -0800 Subject: [PATCH 1/3] Fix race condition in key event handling on Android. --- .../android/AndroidKeyProcessor.java | 89 ++++++++++++------- .../embedding/android/FlutterView.java | 5 -- .../systemchannels/KeyEventChannel.java | 23 +++-- .../editing/InputConnectionAdaptor.java | 19 ++-- .../android/io/flutter/view/FlutterView.java | 7 -- .../systemchannels/KeyEventChannelTest.java | 8 +- .../editing/InputConnectionAdaptorTest.java | 43 +++++++-- 7 files changed, 130 insertions(+), 64 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..3b2bf244a3bda 100644 --- a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java +++ b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java @@ -11,6 +11,7 @@ import androidx.annotation.Nullable; import io.flutter.Log; import io.flutter.embedding.engine.systemchannels.KeyEventChannel; +import io.flutter.embedding.engine.systemchannels.KeyEventChannel.FlutterKeyEvent; import io.flutter.plugin.editing.TextInputPlugin; import java.util.AbstractMap.SimpleImmutableEntry; import java.util.ArrayDeque; @@ -33,7 +34,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 +50,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. @@ -88,31 +88,49 @@ public void destroy() { * @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 onKeyEvent(@NonNull KeyEvent keyEvent) { - int action = keyEvent.getAction(); - if (action != KeyEvent.ACTION_DOWN && action != KeyEvent.ACTION_UP) { + public boolean onKeyEvent(@NonNull KeyEvent event) { + int action = event.getAction(); + if (action != event.ACTION_DOWN && action != event.ACTION_UP) { // There is theoretically a KeyEvent.ACTION_MULTIPLE, but theoretically // that isn't sent by Android anymore, so this is just for protection in // case the theory is wrong. return false; } - if (eventResponder.dispatchingKeyEvent) { - // Don't handle it if it is from our own delayed event dispatch. + long eventId = FlutterKeyEvent.computeEventId(event); + if (eventResponder.isHeadEvent(eventId)) { + // If the event 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 event, and + // we shouldn't respond to it, but we should remove it from tracking now. + eventResponder.removePendingEvent(eventId); return false; } - Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar()); + Character complexCharacter = applyCombiningCharacterToBaseCharacter(event.getUnicodeChar()); KeyEventChannel.FlutterKeyEvent flutterEvent = - new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter, eventIdSerial++); + new KeyEventChannel.FlutterKeyEvent(event, complexCharacter); + + eventResponder.addEvent(flutterEvent.eventId, event); 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) { + long id = FlutterKeyEvent.computeEventId(event); + return eventResponder.isHeadEvent(id); + } + /** * 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 @@ -179,7 +197,6 @@ private static class EventResponder implements KeyEventChannel.EventResponseHand 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; @@ -202,6 +219,25 @@ private KeyEvent removePendingEvent(long id) { return pendingEvents.removeFirst().getValue(); } + private KeyEvent findPendingEvent(long id) { + if (pendingEvents.size() == 0) { + throw new AssertionError( + "Event response received when no events are in the queue. Received id " + id); + } + if (pendingEvents.getFirst().getKey() != id) { + throw new AssertionError( + "Event response received out of order. Should have seen event " + + pendingEvents.getFirst().getKey() + + " first. Instead, received " + + id); + } + return pendingEvents.getFirst().getValue(); + } + + private boolean isHeadEvent(long id) { + return pendingEvents.size() > 0 && pendingEvents.getFirst().getKey() == id; + } + /** * Called whenever the framework responds that a given key event was handled by the framework. * @@ -222,18 +258,11 @@ public void onKeyEventHandled(long id) { */ @Override public void onKeyEventNotHandled(long id) { - dispatchKeyEvent(removePendingEvent(id)); + dispatchKeyEvent(findPendingEvent(id), id); } /** 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)); if (pendingEvents.size() > MAX_PENDING_EVENTS) { Log.e( @@ -250,27 +279,21 @@ public void addEvent(long id, @NonNull KeyEvent event) { * * @param event the event to be dispatched to the activity. */ - public void dispatchKeyEvent(KeyEvent event) { + public void dispatchKeyEvent(KeyEvent event, long id) { // 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. + removePendingEvent(id); + 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..434d0f0943735 100644 --- a/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java +++ b/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java @@ -225,16 +225,29 @@ public static class FlutterKeyEvent { * 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. + * This ID does not come from Android, but instead is derived from attributes of this event that + * can be recognized if the event is re-dispatched. */ public final long eventId; - public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent, long eventId) { - this(androidKeyEvent, null, eventId); + /** + * Creates a unique id for a Flutter key event from an Android KeyEvent. + * + *

This id is used to identify pending key events when results are received from the + * framework. + */ + public static long computeEventId(@NonNull KeyEvent event) { + return (event.getEventTime() & 0xffffffff) + | ((long) (event.getAction() == KeyEvent.ACTION_DOWN ? 0x1 : 0x0)) << 32 + | ((long) (event.getScanCode() & 0xffff)) << 33; + } + + public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent) { + this(androidKeyEvent, null); } public FlutterKeyEvent( - @NonNull KeyEvent androidKeyEvent, @Nullable Character complexCharacter, long eventId) { + @NonNull KeyEvent androidKeyEvent, @Nullable Character complexCharacter) { this( androidKeyEvent.getDeviceId(), androidKeyEvent.getFlags(), @@ -246,7 +259,7 @@ public FlutterKeyEvent( androidKeyEvent.getMetaState(), androidKeyEvent.getSource(), androidKeyEvent.getRepeatCount(), - eventId); + computeEventId(androidKeyEvent)); } public FlutterKeyEvent( 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/engine/systemchannels/KeyEventChannelTest.java b/shell/platform/android/test/io/flutter/embedding/engine/systemchannels/KeyEventChannelTest.java index 81b34b3f3b5ad..344c284786545 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 @@ -62,7 +62,7 @@ public void onKeyEventNotHandled(@NonNull long id) { 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(KeyEventChannel.FlutterKeyEvent.computeEventId(event), handledId[0]); } @Test @@ -103,7 +103,7 @@ public void onKeyEventNotHandled(long id) { 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(KeyEventChannel.FlutterKeyEvent.computeEventId(event), handledId[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())) From 311cef955ad96a7c4214f0a032fd7b29024a6df2 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 30 Nov 2020 08:58:52 -0800 Subject: [PATCH 2/3] Review changes --- .../android/AndroidKeyProcessor.java | 72 +++++++------------ .../systemchannels/KeyEventChannel.java | 53 +++++--------- .../android/AndroidKeyProcessorTest.java | 4 +- .../systemchannels/KeyEventChannelTest.java | 24 +++---- 4 files changed, 60 insertions(+), 93 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java index 3b2bf244a3bda..6bc4309c59a1d 100644 --- a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java +++ b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java @@ -11,12 +11,9 @@ import androidx.annotation.Nullable; import io.flutter.Log; import io.flutter.embedding.engine.systemchannels.KeyEventChannel; -import io.flutter.embedding.engine.systemchannels.KeyEventChannel.FlutterKeyEvent; 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 @@ -96,12 +93,11 @@ public boolean onKeyEvent(@NonNull KeyEvent event) { // case the theory is wrong. return false; } - long eventId = FlutterKeyEvent.computeEventId(event); - if (eventResponder.isHeadEvent(eventId)) { + if (eventResponder.isHeadEvent(event)) { // If the event 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 event, and // we shouldn't respond to it, but we should remove it from tracking now. - eventResponder.removePendingEvent(eventId); + eventResponder.removeHeadEvent(); return false; } @@ -109,7 +105,7 @@ public boolean onKeyEvent(@NonNull KeyEvent event) { KeyEventChannel.FlutterKeyEvent flutterEvent = new KeyEventChannel.FlutterKeyEvent(event, complexCharacter); - eventResponder.addEvent(flutterEvent.eventId, event); + eventResponder.addEvent(event); if (action == KeyEvent.ACTION_DOWN) { keyEventChannel.keyDown(flutterEvent); } else { @@ -127,8 +123,7 @@ public boolean onKeyEvent(@NonNull KeyEvent event) { * @return */ public boolean isCurrentEvent(@NonNull KeyEvent event) { - long id = FlutterKeyEvent.computeEventId(event); - return eventResponder.isHeadEvent(id); + return eventResponder.isHeadEvent(event); } /** @@ -194,7 +189,7 @@ 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; @@ -203,67 +198,54 @@ public EventResponder(@NonNull View view, @NonNull TextInputPlugin textInputPlug 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) { - throw new AssertionError( - "Event response received out of order. Should have seen event " - + pendingEvents.getFirst().getKey() - + " first. Instead, received " - + id); - } - return pendingEvents.removeFirst().getValue(); + /** Removes the first pending event from the cache of pending events. */ + private KeyEvent removeHeadEvent() { + return pendingEvents.removeFirst(); } - private KeyEvent findPendingEvent(long id) { + private KeyEvent checkIsHeadEvent(KeyEvent event) { if (pendingEvents.size() == 0) { throw new AssertionError( - "Event response received when no events are in the queue. Received id " + id); + "Event response received when no events are in the queue. Received event " + event); } - if (pendingEvents.getFirst().getKey() != id) { + 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.getFirst().getValue(); + return pendingEvents.getFirst(); } - private boolean isHeadEvent(long id) { - return pendingEvents.size() > 0 && pendingEvents.getFirst().getKey() == id; + 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(findPendingEvent(id), 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) { - 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, @@ -279,7 +261,7 @@ public void addEvent(long id, @NonNull KeyEvent event) { * * @param event the event to be dispatched to the activity. */ - public void dispatchKeyEvent(KeyEvent event, long id) { + 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. @@ -287,7 +269,7 @@ public void dispatchKeyEvent(KeyEvent event, long id) { && textInputPlugin.getLastInputConnection() != null && textInputPlugin.getLastInputConnection().sendKeyEvent(event)) { // The event was handled, so we can remove it from the queue. - removePendingEvent(id); + removeHeadEvent(); return; } 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 434d0f0943735..efcfb4e938b4d 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,7 +113,7 @@ 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( @@ -222,25 +220,12 @@ public static class FlutterKeyEvent { */ public final int repeatCount; /** - * The unique id for this Flutter key event. + * The Android key event that this Flutter key event was created from. * - *

This id is used to identify pending events when results are received from the framework. - * This ID does not come from Android, but instead is derived from attributes of this event that - * can be recognized if the event is re-dispatched. - */ - public final long eventId; - - /** - * Creates a unique id for a Flutter key event from an Android KeyEvent. - * - *

This id is used to identify pending key events when results are received from the + *

This event is used to identify pending events when results are received from the * framework. */ - public static long computeEventId(@NonNull KeyEvent event) { - return (event.getEventTime() & 0xffffffff) - | ((long) (event.getAction() == KeyEvent.ACTION_DOWN ? 0x1 : 0x0)) << 32 - | ((long) (event.getScanCode() & 0xffff)) << 33; - } + public final KeyEvent event; public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent) { this(androidKeyEvent, null); @@ -259,7 +244,7 @@ public FlutterKeyEvent( androidKeyEvent.getMetaState(), androidKeyEvent.getSource(), androidKeyEvent.getRepeatCount(), - computeEventId(androidKeyEvent)); + androidKeyEvent); } public FlutterKeyEvent( @@ -273,7 +258,7 @@ public FlutterKeyEvent( int metaState, int source, int repeatCount, - long eventId) { + KeyEvent event) { this.deviceId = deviceId; this.flags = flags; this.plainCodePoint = plainCodePoint; @@ -284,7 +269,7 @@ public FlutterKeyEvent( this.metaState = metaState; this.source = source; this.repeatCount = repeatCount; - this.eventId = eventId; + this.event = event; InputDevice device = InputDevice.getDevice(deviceId); if (device != null) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { 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 344c284786545..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,17 +45,17 @@ 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()); @@ -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(KeyEventChannel.FlutterKeyEvent.computeEventId(event), handledId[0]); + assertEquals(event, handledKeyEvents[0]); } @Test @@ -86,17 +86,17 @@ 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()); @@ -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(KeyEventChannel.FlutterKeyEvent.computeEventId(event), handledId[0]); + assertEquals(event, handledKeyEvents[0]); } } From 9d434d5c0df837e2910ab5f879e8659661f0b295 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 30 Nov 2020 13:39:15 -0800 Subject: [PATCH 3/3] Simplify FlutterKeyEvent class --- .../android/AndroidKeyProcessor.java | 18 +- .../systemchannels/KeyEventChannel.java | 174 +++--------------- 2 files changed, 37 insertions(+), 155 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java index 6bc4309c59a1d..33988c83aaf0c 100644 --- a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java +++ b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java @@ -85,27 +85,27 @@ public void destroy() { * @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 onKeyEvent(@NonNull KeyEvent event) { - int action = event.getAction(); - if (action != event.ACTION_DOWN && action != event.ACTION_UP) { + 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 theoretically // that isn't sent by Android anymore, so this is just for protection in // case the theory is wrong. return false; } - if (eventResponder.isHeadEvent(event)) { - // If the event 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 event, and + 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(event.getUnicodeChar()); + Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar()); KeyEventChannel.FlutterKeyEvent flutterEvent = - new KeyEventChannel.FlutterKeyEvent(event, complexCharacter); + new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter); - eventResponder.addEvent(event); + eventResponder.addEvent(keyEvent); if (action == KeyEvent.ACTION_DOWN) { keyEventChannel.keyDown(flutterEvent); } else { 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 efcfb4e938b4d..dbd6bf7f9c924 100644 --- a/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java +++ b/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java @@ -117,108 +117,34 @@ public void keyDown(@NonNull FlutterKeyEvent keyEvent) { } 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. - * - * @see KeyEvent.getUnicodeChar() - */ - public final int codePoint; - /** - * The Android key code for this event. - * - * @see KeyEvent.getKeyCode() - */ - public final int keyCode; - /** - * 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 Android key event that this Flutter key event was created from. * @@ -226,6 +152,10 @@ public static class FlutterKeyEvent { * framework. */ public final KeyEvent event; + /** + * The character produced by this event, including any combining characters pressed before it. + */ + @Nullable public final Character complexCharacter; public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent) { this(androidKeyEvent, null); @@ -233,56 +163,8 @@ public FlutterKeyEvent(@NonNull KeyEvent androidKeyEvent) { public FlutterKeyEvent( @NonNull KeyEvent androidKeyEvent, @Nullable Character complexCharacter) { - this( - androidKeyEvent.getDeviceId(), - androidKeyEvent.getFlags(), - androidKeyEvent.getUnicodeChar(0x0), - androidKeyEvent.getUnicodeChar(), - androidKeyEvent.getKeyCode(), - complexCharacter, - androidKeyEvent.getScanCode(), - androidKeyEvent.getMetaState(), - androidKeyEvent.getSource(), - androidKeyEvent.getRepeatCount(), - androidKeyEvent); - } - - public FlutterKeyEvent( - int deviceId, - int flags, - int plainCodePoint, - int codePoint, - int keyCode, - @Nullable Character complexCharacter, - int scanCode, - int metaState, - int source, - int repeatCount, - KeyEvent event) { - this.deviceId = deviceId; - this.flags = flags; - this.plainCodePoint = plainCodePoint; - this.codePoint = codePoint; - this.keyCode = keyCode; + this.event = androidKeyEvent; this.complexCharacter = complexCharacter; - this.scanCode = scanCode; - this.metaState = metaState; - this.source = source; - this.repeatCount = repeatCount; - this.event = event; - 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; - } } } }