Review changes

Author: gspencergoog

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,20 +93,19 @@ 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;
     }
 
     Character complexCharacter = applyCombiningCharacterToBaseCharacter(event.getUnicodeChar());
     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<Entry<Long, KeyEvent>> pendingEvents = new ArrayDeque<Entry<Long, KeyEvent>>();
+    final Deque<KeyEvent> pendingEvents = new ArrayDeque<KeyEvent>();
     @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<Long, KeyEvent>(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,15 +261,15 @@ 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.
       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);
+        removeHeadEvent();
         return;
       }
 

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,31 +67,31 @@ 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<Object> createReplyHandler(long eventId) {
+  BasicMessageChannel.Reply<Object> createReplyHandler(KeyEvent event) {
     return message -> {
       if (eventResponseHandler == null) {
         return;
       }
 
       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.
      *
-     * <p>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.
-     *
-     * <p>This id is used to identify pending key events when results are received from the
+     * <p>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) {

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));

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,25 +78,25 @@ 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
   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]);
   }
 }