Make android more lenient when it comes to out-of-order key event responses (flutter#23604)

Relaxes enforcement of key events being handled in order, to match similar code in the Linux and Windows implementations.

Author: gspencergoog

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

@@ -14,6 +14,7 @@
 import io.flutter.plugin.editing.TextInputPlugin;
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.Iterator;
 
 /**
  * A class to process key events from Android, passing them to the framework as messages using
@@ -93,11 +94,11 @@ public boolean onKeyEvent(@NonNull KeyEvent keyEvent) {
       // case the theory is wrong.
       return false;
     }
-    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();
+    if (isPendingEvent(keyEvent)) {
+      // If the keyEvent is in 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.removePendingEvent(keyEvent);
       return false;
     }
 
@@ -122,8 +123,8 @@ public boolean onKeyEvent(@NonNull KeyEvent keyEvent) {
    * @param event the event to check for being the current event.
    * @return
    */
-  public boolean isCurrentEvent(@NonNull KeyEvent event) {
-    return eventResponder.isHeadEvent(event);
+  public boolean isPendingEvent(@NonNull KeyEvent event) {
+    return eventResponder.findPendingEvent(event) != null;
   }
 
   /**
@@ -199,27 +200,19 @@ public EventResponder(@NonNull View view, @NonNull TextInputPlugin textInputPlug
     }
 
     /** Removes the first pending event from the cache of pending events. */
-    private KeyEvent removeHeadEvent() {
-      return pendingEvents.removeFirst();
+    private void removePendingEvent(KeyEvent event) {
+      pendingEvents.remove(event);
     }
 
-    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()
-                + " first. Instead, received "
-                + event);
+    private KeyEvent findPendingEvent(KeyEvent event) {
+      Iterator<KeyEvent> iter = pendingEvents.iterator();
+      while (iter.hasNext()) {
+        KeyEvent item = iter.next();
+        if (item == event) {
+          return item;
+        }
       }
-      return pendingEvents.getFirst();
-    }
-
-    private boolean isHeadEvent(KeyEvent event) {
-      return pendingEvents.size() > 0 && pendingEvents.getFirst() == event;
+      return null;
     }
 
     /**
@@ -229,7 +222,7 @@ private boolean isHeadEvent(KeyEvent event) {
      */
     @Override
     public void onKeyEventHandled(KeyEvent event) {
-      removeHeadEvent();
+      removePendingEvent(event);
     }
 
     /**
@@ -240,7 +233,7 @@ public void onKeyEventHandled(KeyEvent event) {
      */
     @Override
     public void onKeyEventNotHandled(KeyEvent event) {
-      redispatchKeyEvent(checkIsHeadEvent(event));
+      redispatchKeyEvent(findPendingEvent(event));
     }
 
     /** Adds an Android key event to the event responder to wait for a response. */
@@ -269,7 +262,7 @@ private void redispatchKeyEvent(KeyEvent event) {
           && textInputPlugin.getLastInputConnection() != null
           && textInputPlugin.getLastInputConnection().sendKeyEvent(event)) {
         // The event was handled, so we can remove it from the queue.
-        removeHeadEvent();
+        removePendingEvent(event);
         return;
       }
 

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

@@ -299,7 +299,7 @@ public boolean sendKeyEvent(KeyEvent event) {
     // 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.isPendingEvent(event)
         && keyProcessor.onKeyEvent(event)) {
       return true;
     }

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

@@ -74,6 +74,54 @@ public void destroyTest() {
         .setEventResponseHandler(isNull(KeyEventChannel.EventResponseHandler.class));
   }
 
+  public void removesPendingEventsWhenKeyDownHandled() {
+    FlutterEngine flutterEngine = mockFlutterEngine();
+    KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
+    View fakeView = mock(View.class);
+    View fakeRootView = mock(View.class);
+    when(fakeView.getRootView())
+        .then(
+            new Answer<View>() {
+              @Override
+              public View answer(InvocationOnMock invocation) throws Throwable {
+                return fakeRootView;
+              }
+            });
+
+    ArgumentCaptor<KeyEventChannel.EventResponseHandler> handlerCaptor =
+        ArgumentCaptor.forClass(KeyEventChannel.EventResponseHandler.class);
+    verify(fakeKeyEventChannel).setEventResponseHandler(handlerCaptor.capture());
+    AndroidKeyProcessor processor =
+        new AndroidKeyProcessor(fakeView, fakeKeyEventChannel, mock(TextInputPlugin.class));
+    ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> eventCaptor =
+        ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
+    FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);
+
+    boolean result = processor.onKeyEvent(fakeKeyEvent);
+    assertEquals(true, processor.isPendingEvent(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)))
+        .then(
+            new Answer<Boolean>() {
+              @Override
+              public Boolean answer(InvocationOnMock invocation) throws Throwable {
+                KeyEvent event = (KeyEvent) invocation.getArguments()[0];
+                assertEquals(fakeKeyEvent, event);
+                dispatchResult[0] = processor.onKeyEvent(event);
+                return dispatchResult[0];
+              }
+            });
+
+    // Fake a response from the framework.
+    handlerCaptor.getValue().onKeyEventHandled(eventCaptor.getValue().event);
+    assertEquals(false, processor.isPendingEvent(fakeKeyEvent));
+  }
+
   public void synthesizesEventsWhenKeyDownNotHandled() {
     FlutterEngine flutterEngine = mockFlutterEngine();
     KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
@@ -98,6 +146,7 @@ public View answer(InvocationOnMock invocation) throws Throwable {
     FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);
 
     boolean result = processor.onKeyEvent(fakeKeyEvent);
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
     assertEquals(true, result);
 
     // Capture the FlutterKeyEvent so we can find out its event ID to use when
@@ -118,6 +167,7 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable {
 
     // Fake a response from the framework.
     handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event);
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
     verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
     assertEquals(false, dispatchResult[0]);
     verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
@@ -148,6 +198,7 @@ public View answer(InvocationOnMock invocation) throws Throwable {
     FakeKeyEvent fakeKeyEvent = new FakeKeyEvent(KeyEvent.ACTION_UP, 65);
 
     boolean result = processor.onKeyEvent(fakeKeyEvent);
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
     assertEquals(true, result);
 
     // Capture the FlutterKeyEvent so we can find out its event ID to use when
@@ -168,12 +219,84 @@ public Boolean answer(InvocationOnMock invocation) throws Throwable {
 
     // Fake a response from the framework.
     handlerCaptor.getValue().onKeyEventNotHandled(eventCaptor.getValue().event);
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent));
     verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent);
     assertEquals(false, dispatchResult[0]);
     verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
     verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent);
   }
 
+  public void respondsCorrectlyWhenEventsAreReturnedOutOfOrder() {
+    FlutterEngine flutterEngine = mockFlutterEngine();
+    KeyEventChannel fakeKeyEventChannel = flutterEngine.getKeyEventChannel();
+    View fakeView = mock(View.class);
+    View fakeRootView = mock(View.class);
+    when(fakeView.getRootView())
+        .then(
+            new Answer<View>() {
+              @Override
+              public View answer(InvocationOnMock invocation) throws Throwable {
+                return fakeRootView;
+              }
+            });
+
+    ArgumentCaptor<KeyEventChannel.EventResponseHandler> handlerCaptor =
+        ArgumentCaptor.forClass(KeyEventChannel.EventResponseHandler.class);
+    verify(fakeKeyEventChannel).setEventResponseHandler(handlerCaptor.capture());
+    AndroidKeyProcessor processor =
+        new AndroidKeyProcessor(fakeView, fakeKeyEventChannel, mock(TextInputPlugin.class));
+    ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> event1Captor =
+        ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
+    ArgumentCaptor<KeyEventChannel.FlutterKeyEvent> event2Captor =
+        ArgumentCaptor.forClass(KeyEventChannel.FlutterKeyEvent.class);
+    FakeKeyEvent fakeKeyEvent1 = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 65);
+    FakeKeyEvent fakeKeyEvent2 = new FakeKeyEvent(KeyEvent.ACTION_DOWN, 20);
+
+    boolean result1 = processor.onKeyEvent(fakeKeyEvent1);
+    boolean result2 = processor.onKeyEvent(fakeKeyEvent2);
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent1));
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent2));
+    assertEquals(true, result1);
+    assertEquals(true, result2);
+
+    // Capture the FlutterKeyEvent so we can find out its event ID to use when
+    // faking our response.
+    verify(fakeKeyEventChannel, times(1)).keyDown(event1Captor.capture());
+    verify(fakeKeyEventChannel, times(1)).keyDown(event2Captor.capture());
+    boolean[] dispatchResult = {true, true};
+    when(fakeView.dispatchKeyEvent(any(KeyEvent.class)))
+        .then(
+            new Answer<Boolean>() {
+              @Override
+              public Boolean answer(InvocationOnMock invocation) throws Throwable {
+                KeyEvent event = (KeyEvent) invocation.getArguments()[0];
+                assertEquals(true, fakeKeyEvent1 == event || fakeKeyEvent2 == event);
+                if (fakeKeyEvent1 == event) {
+                  dispatchResult[0] = processor.onKeyEvent(fakeKeyEvent1);
+                  return dispatchResult[0];
+                } else {
+                  dispatchResult[1] = processor.onKeyEvent(fakeKeyEvent2);
+                  return dispatchResult[1];
+                }
+              }
+            });
+
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent1));
+    assertEquals(true, processor.isPendingEvent(fakeKeyEvent2));
+
+    // Fake a "handled" response from the framework, but do it in reverse order.
+    handlerCaptor.getValue().onKeyEventNotHandled(event2Captor.getValue().event);
+    handlerCaptor.getValue().onKeyEventNotHandled(event1Captor.getValue().event);
+
+    verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent1);
+    verify(fakeView, times(1)).dispatchKeyEvent(fakeKeyEvent2);
+    assertEquals(false, dispatchResult[0]);
+    assertEquals(false, dispatchResult[1]);
+    verify(fakeKeyEventChannel, times(0)).keyUp(any(KeyEventChannel.FlutterKeyEvent.class));
+    verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent1);
+    verify(fakeRootView, times(1)).dispatchKeyEvent(fakeKeyEvent2);
+  }
+
   @NonNull
   private FlutterEngine mockFlutterEngine() {
     // Mock FlutterEngine and all of its required direct calls.

shell/platform/android/test/io/flutter/plugin/editing/InputConnectionAdaptorTest.java

@@ -1033,7 +1033,7 @@ public void testCursorAnchorInfo() {
   public void testSendKeyEvent_sendSoftKeyEvents() {
     ListenableEditingState editable = sampleEditable(5, 5);
     AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class);
-    when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(true);
+    when(mockKeyProcessor.isPendingEvent(any())).thenReturn(true);
     InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor);
 
     KeyEvent shiftKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_SHIFT_LEFT);
@@ -1047,7 +1047,7 @@ public void testSendKeyEvent_sendSoftKeyEvents() {
   public void testSendKeyEvent_sendHardwareKeyEvents() {
     ListenableEditingState editable = sampleEditable(5, 5);
     AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class);
-    when(mockKeyProcessor.isCurrentEvent(any())).thenReturn(false);
+    when(mockKeyProcessor.isPendingEvent(any())).thenReturn(false);
     when(mockKeyProcessor.onKeyEvent(any())).thenReturn(true);
     InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable, mockKeyProcessor);