Fixed a bug where sometimes unhandled handler would consume all keys

In a situation where a focused view consumed only the UP of a key
and the unhandled key manager would focus a listener, it wouldn't
drop focus unless the original key was pressed/released again.

This updates the record of captured keys before it can be consumed
in the view hierarchy.

Bug: 79993136
Test: Added a test for this to cts ViewTest#testUnhandledKeys
Change-Id: I5dfdcf16c5c41e9ad51cb62b385580c5493e8520

1 files changed, 66 insertions, 41 deletions

diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 239185e5c25d..5f933d89125f 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -76,7 +76,6 @@ import android.util.LongArray;
 import android.util.MergedConfiguration;
 import android.util.Slog;
 import android.util.SparseArray;
-import android.util.SparseBooleanArray;
 import android.util.TimeUtils;
 import android.util.TypedValue;
 import android.view.Surface.OutOfResourcesException;
@@ -4980,10 +4979,7 @@ public final class ViewRootImpl implements ViewParent,
         private int processKeyEvent(QueuedInputEvent q) {
             final KeyEvent event = (KeyEvent)q.mEvent;
 
-            mUnhandledKeyManager.mDispatched = false;
-
-            if (mUnhandledKeyManager.hasFocus()
-                    && mUnhandledKeyManager.dispatchUnique(mView, event)) {
+            if (mUnhandledKeyManager.preViewDispatch(event)) {
                 return FINISH_HANDLED;
             }
 
@@ -4996,7 +4992,10 @@ public final class ViewRootImpl implements ViewParent,
                 return FINISH_NOT_HANDLED;
             }
 
-            if (mUnhandledKeyManager.dispatchUnique(mView, event)) {
+            // This dispatch is for windows that don't have a Window.Callback. Otherwise,
+            // the Window.Callback usually will have already called this (see
+            // DecorView.superDispatchKeyEvent) leaving this call a no-op.
+            if (mUnhandledKeyManager.dispatch(mView, event)) {
                 return FINISH_HANDLED;
             }
 
@@ -7034,6 +7033,10 @@ public final class ViewRootImpl implements ViewParent,
             stage = q.shouldSkipIme() ? mFirstPostImeInputStage : mFirstInputStage;
         }
 
+        if (q.mEvent instanceof KeyEvent) {
+            mUnhandledKeyManager.preDispatch((KeyEvent) q.mEvent);
+        }
+
         if (stage != null) {
             handleWindowFocusChanged();
             stage.deliver(q);
@@ -7802,7 +7805,7 @@ public final class ViewRootImpl implements ViewParent,
      * @param event
      * @return {@code true} if the event was handled, {@code false} otherwise.
      */
-    public boolean dispatchKeyFallbackEvent(KeyEvent event) {
+    public boolean dispatchUnhandledKeyEvent(KeyEvent event) {
         return mUnhandledKeyManager.dispatch(mView, event);
     }
 
@@ -8394,35 +8397,74 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     private static class UnhandledKeyManager {
-
         // This is used to ensure that unhandled events are only dispatched once. We attempt
         // to dispatch more than once in order to achieve a certain order. Specifically, if we
         // are in an Activity or Dialog (and have a Window.Callback), the unhandled events should
-        // be dispatched after the view hierarchy, but before the Activity. However, if we aren't
+        // be dispatched after the view hierarchy, but before the Callback. However, if we aren't
         // in an activity, we still want unhandled keys to be dispatched.
-        boolean mDispatched = false;
+        private boolean mDispatched = true;
 
-        SparseBooleanArray mCapturedKeys = new SparseBooleanArray();
-        WeakReference<View> mCurrentReceiver = null;
+        // Keeps track of which Views have unhandled key focus for which keys. This doesn't
+        // include modifiers.
+        private final SparseArray<WeakReference<View>> mCapturedKeys = new SparseArray<>();
 
-        private void updateCaptureState(KeyEvent event) {
-            if (event.getAction() == KeyEvent.ACTION_DOWN) {
-                mCapturedKeys.append(event.getKeyCode(), true);
+        // The current receiver. This value is transient and used between the pre-dispatch and
+        // pre-view phase to ensure that other input-stages don't interfere with tracking.
+        private WeakReference<View> mCurrentReceiver = null;
+
+        boolean dispatch(View root, KeyEvent event) {
+            if (mDispatched) {
+                return false;
             }
-            if (event.getAction() == KeyEvent.ACTION_UP) {
-                mCapturedKeys.delete(event.getKeyCode());
+            View consumer;
+            try {
+                Trace.traceBegin(Trace.TRACE_TAG_VIEW, "UnhandledKeyEvent dispatch");
+                mDispatched = true;
+
+                consumer = root.dispatchUnhandledKeyEvent(event);
+
+                // If an unhandled listener handles one, then keep track of it so that the
+                // consuming view is first to receive its repeats and release as well.
+                if (event.getAction() == KeyEvent.ACTION_DOWN) {
+                    int keycode = event.getKeyCode();
+                    if (consumer != null && !KeyEvent.isModifierKey(keycode)) {
+                        mCapturedKeys.put(keycode, new WeakReference<>(consumer));
+                    }
+                }
+            } finally {
+                Trace.traceEnd(Trace.TRACE_TAG_VIEW);
             }
+            return consumer != null;
         }
 
-        boolean dispatch(View root, KeyEvent event) {
-            Trace.traceBegin(Trace.TRACE_TAG_VIEW, "KeyFallback dispatch");
-            mDispatched = true;
-
-            updateCaptureState(event);
+        /**
+         * Called before the event gets dispatched to anything
+         */
+        void preDispatch(KeyEvent event) {
+            // Always clean-up 'up' events since it's possible for earlier dispatch stages to
+            // consume them without consuming the corresponding 'down' event.
+            mCurrentReceiver = null;
+            if (event.getAction() == KeyEvent.ACTION_UP) {
+                int idx = mCapturedKeys.indexOfKey(event.getKeyCode());
+                if (idx >= 0) {
+                    mCurrentReceiver = mCapturedKeys.valueAt(idx);
+                    mCapturedKeys.removeAt(idx);
+                }
+            }
+        }
 
+        /**
+         * Called before the event gets dispatched to the view hierarchy
+         * @return {@code true} if an unhandled handler has focus and consumed the event
+         */
+        boolean preViewDispatch(KeyEvent event) {
+            mDispatched = false;
+            if (mCurrentReceiver == null) {
+                mCurrentReceiver = mCapturedKeys.get(event.getKeyCode());
+            }
             if (mCurrentReceiver != null) {
                 View target = mCurrentReceiver.get();
-                if (mCapturedKeys.size() == 0) {
+                if (event.getAction() == KeyEvent.ACTION_UP) {
                     mCurrentReceiver = null;
                 }
                 if (target != null && target.isAttachedToWindow()) {
@@ -8431,24 +8473,7 @@ public final class ViewRootImpl implements ViewParent,
                 // consume anyways so that we don't feed uncaptured key events to other views
                 return true;
             }
-
-            View consumer = root.dispatchUnhandledKeyEvent(event);
-            if (consumer != null) {
-                mCurrentReceiver = new WeakReference<>(consumer);
-            }
-            Trace.traceEnd(Trace.TRACE_TAG_VIEW);
-            return consumer != null;
-        }
-
-        boolean hasFocus() {
-            return mCurrentReceiver != null;
-        }
-
-        boolean dispatchUnique(View root, KeyEvent event) {
-            if (mDispatched) {
-                return false;
-            }
-            return dispatch(root, event);
+            return false;
         }
     }
 }