Converted to a deque, added comments

gspencergoog · gspencergoog · commit 3eb72ea69924 · 2020-06-12T14:50:40.000-07:00
diff --git a/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java b/shell/platform/android/io/flutter/embedding/android/AndroidKeyProcessor.java
@@ -5,10 +5,13 @@
 package io.flutter.embedding.android;
 
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.MapEntry;
+import java.util.Deque;
+import java.util.ArrayDeque;
+import java.util.AbstractMap.SimpleImmutableEntry;
 
 import android.app.Activity;
 import android.content.Context;
@@ -21,48 +24,65 @@
 import io.flutter.embedding.engine.systemchannels.KeyEventChannel;
 import io.flutter.plugin.editing.TextInputPlugin;
 
+/**
+ * A class to process key events from Android, passing them to the framework as
+ * messages using {@link KeyEventChannel}.
+ */
 public class AndroidKeyProcessor {
   private static final String TAG = "AndroidKeyProcessor";
 
-  @NonNull private final KeyEventChannel keyEventChannel;
-  @NonNull private final TextInputPlugin textInputPlugin;
-  @NonNull private final Context context;
+  @NonNull
+  private final KeyEventChannel keyEventChannel;
+  @NonNull
+  private final TextInputPlugin textInputPlugin;
+  @NonNull
   private int combiningCharacter;
+  @NonNull
+  private EventResponder eventResponder;
 
-  private Map<Long, KeyEvent> pendingEvents = new HashMap<Long, KeyEvent>();
-  private boolean dispatchingKeyEvent = false;
-
-  public AndroidKeyProcessor(@NonNull Context context, @NonNull KeyEventChannel keyEventChannel, @NonNull TextInputPlugin textInputPlugin) {
+  public AndroidKeyProcessor(@NonNull Context context, @NonNull KeyEventChannel keyEventChannel,
+      @NonNull TextInputPlugin textInputPlugin) {
     this.keyEventChannel = keyEventChannel;
     this.textInputPlugin = textInputPlugin;
-    this.context = context;
-    this.keyEventChannel.setKeyProcessor(this);
+    this.eventResponder = new EventResponder(context);
+    this.keyEventChannel.setEventResponseHandler(eventResponder);
   }
 
+  /**
+   * Called when a key up event is received by the {@link FlutterView}.
+   *
+   * @param keyEvent the Android key event to respond to.
+   * @return true if the key event was handled and should not be propagated.
+   */
   public boolean onKeyUp(@NonNull KeyEvent keyEvent) {
-    if (dispatchingKeyEvent) {
+    if (eventResponder.dispatchingKeyEvent) {
       // Don't handle it if it is from our own delayed event synthesis.
       return false;
     }
 
     Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar());
     KeyEventChannel.FlutterKeyEvent flutterEvent = new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter);
     keyEventChannel.keyUp(flutterEvent);
-    pendingEvents.put(flutterEvent.eventId, keyEvent);
+    eventResponder.addEvent(flutterEvent.eventId, keyEvent);
     return true;
   }
 
+  /**
+   * Called when a key down event is received by the {@link FlutterView}.
+   *
+   * @param keyEvent the Android key event to respond to.
+   * @return true if the key event was handled and should not be propagated.
+   */
   public boolean onKeyDown(@NonNull KeyEvent keyEvent) {
-    if (dispatchingKeyEvent) {
+    if (eventResponder.dispatchingKeyEvent) {
       // Don't handle it if it is from our own delayed event synthesis.
       return false;
     }
 
     // If the textInputPlugin is still valid and accepting text, then we'll try
     // and send the key event to it, assuming that if the event can be sent, that
     // it has been handled.
-    if (textInputPlugin.getLastInputConnection() != null
-        && textInputPlugin.getInputMethodManager().isAcceptingText()) {
+    if (textInputPlugin.getLastInputConnection() != null && textInputPlugin.getInputMethodManager().isAcceptingText()) {
       if (textInputPlugin.getLastInputConnection().sendKeyEvent(keyEvent)) {
         return true;
       }
@@ -71,80 +91,43 @@ public boolean onKeyDown(@NonNull KeyEvent keyEvent) {
     Character complexCharacter = applyCombiningCharacterToBaseCharacter(keyEvent.getUnicodeChar());
     KeyEventChannel.FlutterKeyEvent flutterEvent = new KeyEventChannel.FlutterKeyEvent(keyEvent, complexCharacter);
     keyEventChannel.keyDown(flutterEvent);
-    pendingEvents.put(flutterEvent.eventId, keyEvent);
+    eventResponder.addEvent(flutterEvent.eventId, keyEvent);
     return true;
   }
 
-  public void onKeyEventHandled(@NonNull long id) {
-    if (!pendingEvents.containsKey(id)) {
-      Log.e(TAG, "Key with id " + id + " not found in pending key events list. " +
-                 "There are " + pendingEvents.size() + " pending events.");
-      return;
-    }
-    // Since this event was already reported to Android as handled, we just
-    // remove it from the map of pending events.
-    pendingEvents.remove(id);
-    Log.e(TAG, "Removing handled key with id " + id + " from pending key events list. " +
-               "There are now " + pendingEvents.size() + " pending events.");
-  }
-
-  public void onKeyEventNotHandled(@NonNull long id) {
-    if (!pendingEvents.containsKey(id)) {
-      Log.e(TAG, "Key with id " + id + " not found in pending key events list. " +
-                 "There are " + pendingEvents.size() + " pending events.");
-      return;
-    }
-    // Since this event was NOT handled by the framework we now synthesize a
-    // new, identical, key event to pass along.
-    KeyEvent pendingEvent = pendingEvents.remove(id);
-    Log.e(TAG, "Removing unhandled key with id " + id + " from pending key events list. " +
-               "There are now " + pendingEvents.size() + " pending events.");
-    Activity activity = getActivity(context);
-    if (activity != null) {
-      // Turn on dispatchingKeyEvent so that we don't dispatch to ourselves and
-      // send it to the framework again.
-      dispatchingKeyEvent = true;
-      activity.dispatchKeyEvent(pendingEvent);
-      dispatchingKeyEvent = false;
-    }
-  }
-
-  private Activity getActivity(Context context) {
-    if (context instanceof Activity) {
-      return (Activity) context;
-    }
-    if (context instanceof ContextWrapper) {
-      // Recurse up chain of base contexts until we find an Activity.
-      return getActivity(((ContextWrapper) context).getBaseContext());
-    }
-    return null;
-  }
-
   /**
-   * 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
-   * exists.
+   * 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 exists.
    *
-   * <p>This method mutates {@link #combiningCharacter} over time to combine characters.
+   * <p>
+   * This method mutates {@link #combiningCharacter} over time to combine
+   * characters.
    *
-   * <p>One of the following things happens in this method:
+   * <p>
+   * One of the following things happens in this method:
    *
    * <ul>
-   *   <li>If no previous {@link #combiningCharacter} exists and the {@code newCharacterCodePoint}
-   *       is not a combining character, then {@code newCharacterCodePoint} is returned.
-   *   <li>If no previous {@link #combiningCharacter} exists and the {@code newCharacterCodePoint}
-   *       is a combining character, then {@code newCharacterCodePoint} is saved as the {@link
-   *       #combiningCharacter} and null is returned.
-   *   <li>If a previous {@link #combiningCharacter} exists and the {@code newCharacterCodePoint} is
-   *       also a combining character, then the {@code newCharacterCodePoint} is combined with the
-   *       existing {@link #combiningCharacter} and null is returned.
-   *   <li>If a previous {@link #combiningCharacter} exists and the {@code newCharacterCodePoint} is
-   *       not a combining character, then the {@link #combiningCharacter} is applied to the regular
-   *       {@code newCharacterCodePoint} and the resulting complex character is returned. The {@link
-   *       #combiningCharacter} is cleared.
+   * <li>If no previous {@link #combiningCharacter} exists and the
+   * {@code newCharacterCodePoint} is not a combining character, then
+   * {@code newCharacterCodePoint} is returned.
+   * <li>If no previous {@link #combiningCharacter} exists and the
+   * {@code newCharacterCodePoint} is a combining character, then
+   * {@code newCharacterCodePoint} is saved as the {@link #combiningCharacter} and
+   * null is returned.
+   * <li>If a previous {@link #combiningCharacter} exists and the
+   * {@code newCharacterCodePoint} is also a combining character, then the
+   * {@code newCharacterCodePoint} is combined with the existing
+   * {@link #combiningCharacter} and null is returned.
+   * <li>If a previous {@link #combiningCharacter} exists and the
+   * {@code newCharacterCodePoint} is not a combining character, then the
+   * {@link #combiningCharacter} is applied to the regular
+   * {@code newCharacterCodePoint} and the resulting complex character is
+   * returned. The {@link #combiningCharacter} is cleared.
    * </ul>
    *
-   * <p>The following reference explains the concept of a "combining character":
+   * <p>
+   * The following reference explains the concept of a "combining character":
    * https://en.wikipedia.org/wiki/Combining_character
    */
   @Nullable
@@ -154,8 +137,7 @@ private Character applyCombiningCharacterToBaseCharacter(int newCharacterCodePoi
     }
 
     Character complexCharacter = (char) newCharacterCodePoint;
-    boolean isNewCodePointACombiningCharacter =
-        (newCharacterCodePoint & KeyCharacterMap.COMBINING_ACCENT) != 0;
+    boolean isNewCodePointACombiningCharacter = (newCharacterCodePoint & KeyCharacterMap.COMBINING_ACCENT) != 0;
     if (isNewCodePointACombiningCharacter) {
       // If a combining character was entered before, combine this one with that one.
       int plainCodePoint = newCharacterCodePoint & KeyCharacterMap.COMBINING_ACCENT_MASK;
@@ -165,7 +147,8 @@ private Character applyCombiningCharacterToBaseCharacter(int newCharacterCodePoi
         combiningCharacter = plainCodePoint;
       }
     } else {
-      // The new character is a regular character. Apply combiningCharacter to it, if it exists.
+      // The new character is a regular character. Apply combiningCharacter to it, if
+      // it exists.
       if (combiningCharacter != 0) {
         int combinedChar = KeyCharacterMap.getDeadChar(combiningCharacter, newCharacterCodePoint);
         if (combinedChar > 0) {
@@ -177,4 +160,97 @@ private Character applyCombiningCharacterToBaseCharacter(int newCharacterCodePoi
 
     return complexCharacter;
   }
+
+  public static class EventResponder implements KeyEventChannel.EventResponseHandler {
+    // The maximum number of pending events that are held before starting to
+    // complain.
+    private static final long MAX_PENDING_EVENTS = 1000;
+    private final Deque<Entry<Long, KeyEvent>> pendingEvents = new ArrayDeque<Entry<Long, KeyEvent>>();
+    @NonNull
+    private final Context context;
+    public boolean dispatchingKeyEvent = false;
+
+    public EventResponder(@NonNull Context context) {
+      this.context = context;
+    }
+
+    /**
+     * 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(@NonNull long id) {
+      if (pendingEvents.getFirst().getKey() != id) {
+        throw new AssertionError("Event response received out of order");
+      }
+      return pendingEvents.removeFirst().getValue();
+    }
+
+    /**
+     * 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.
+     */
+    @Override
+    public void onKeyEventHandled(@NonNull long id) {
+      removePendingEvent(id);
+      Log.v(TAG, "Removed handled key with id " + id + " from pending key events list. " + "There are now "
+          + pendingEvents.size() + " pending events.");
+    }
+
+    /**
+     * 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.
+     */
+    @Override
+    public void onKeyEventNotHandled(@NonNull long id) {
+      KeyEvent pendingEvent = removePendingEvent(id);
+      Log.v(TAG, "Removed unhandled key with id " + id + " from pending key events list. " + "There are now "
+          + pendingEvents.size() + " pending events.");
+
+      // Since the framework didn't handle it, dispatch the key again.
+      Activity activity = getActivity(context);
+      if (activity != null) {
+        // Turn on dispatchingKeyEvent so that we don't dispatch to ourselves and
+        // send it to the framework again.
+        dispatchingKeyEvent = true;
+        activity.dispatchKeyEvent(pendingEvent);
+        dispatchingKeyEvent = false;
+      }
+    }
+
+    /**
+     * Adds the given event with the given id to the event manager to wait for a
+     * response.
+     */
+    public void addEvent(long id, @NonNull KeyEvent event) {
+      pendingEvents.addLast(new SimpleImmutableEntry<Long, KeyEvent>(id, event));
+      if (pendingEvents.size() > MAX_PENDING_EVENTS) {
+        Log.e(TAG, "There are " + pendingEvents.size() + " keyboard events "
+            + "that have not yet received a response. Are responses being sent?");
+      }
+    }
+
+    /**
+     * Gets the nearest ancestor Activity for the given Context.
+     *
+     * @param context the context to look in for the activity.
+     * @return null if no Activity found.
+     */
+    private Activity getActivity(Context context) {
+      if (context instanceof Activity) {
+        return (Activity) context;
+      }
+      if (context instanceof ContextWrapper) {
+        // Recurse up chain of base contexts until we find an Activity.
+        return getActivity(((ContextWrapper) context).getBaseContext());
+      }
+      return null;
+    }
+  }
 }
diff --git a/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java b/shell/platform/android/io/flutter/embedding/engine/systemchannels/KeyEventChannel.java
@@ -30,13 +30,35 @@ public class KeyEventChannel {
   private static long eventIdSerial = 0;
 
   /**
-   * Sets the key processor to be used to receive messages from the framework on
-   * this channel.
+   * Sets the event response handler to be used to receive key event response
+   * messages from the framework on this channel.
    */
-  public void setKeyProcessor(AndroidKeyProcessor processor) {
-    this.processor = processor;
+  public void setEventResponseHandler(EventResponseHandler handler) {
+    this.eventResponseHandler = handler;
+  }
+  private EventResponseHandler eventResponseHandler;
+
+  /** A handler of incoming key handling messages. */
+  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.
+     */
+      public void onKeyEventHandled(@NonNull long id);
+
+    /**
+     * 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.
+     */
+    public void onKeyEventNotHandled(@NonNull long id);
   }
-  private AndroidKeyProcessor processor;
 
   private final BasicMessageChannel.MessageHandler<Object> messageHandler =
       new BasicMessageChannel.MessageHandler<Object>() {
@@ -47,7 +69,7 @@ public void onMessage(
 
           // If there is no processor to respond to this message then we don't need to
           // parse it.
-          if (processor == null) {
+          if (eventResponseHandler == null) {
             return;
           }
 
@@ -56,15 +78,12 @@ public void onMessage(
             final String type = annotatedEvent.getString("type");
             final JSONObject data = annotatedEvent.getJSONObject("data");
 
-            Log.v(TAG, "Received " + type + " message.");
             switch (type) {
               case "keyHandled":
-              Log.w(TAG, "Handled key event " + data.getLong("eventId"));
-              processor.onKeyEventHandled(data.getLong("eventId"));
+              eventResponseHandler.onKeyEventHandled(data.getLong("eventId"));
                 break;
               case "keyNotHandled":
-              Log.w(TAG, "Did not handle key event " + data.getLong("eventId"));
-              processor.onKeyEventNotHandled(data.getLong("eventId"));
+              eventResponseHandler.onKeyEventNotHandled(data.getLong("eventId"));
                 break;
             }
           } catch (JSONException e) {
