Key input consumers by token and not by name

- Currently input consumers are singletons keyed by static name, which
  is problematic because the per-user Launcher process registers and
  unregisters the input consumer when SysUI binds to the current user's
  TouchInteractionService.  However, the ordering of service create
  and destroy when switching users is not guaranteed, so you can end up
  with user 2's service being created before user 1's service destroy is
  called.  Currently this crashes because we expect singletons.
- This change changes the tracking of input consumers to be by token
  (which is already passed by the caller), and uses the token for
  identifying the input consumer to destroy.

  In addition, it only enforces a singleton check for input consumers
  registered for the same user.  And when fetching an input consumer by
  name, the last registered consumer with that name takes precedence and
  is returned.  This allows multiple Launcher processes to register
  a consumer for the current user without preventing the closing Launcher
  service from removing its previously registered consumer.

Fixes: 300973954
Test: Verify with multiple users that there's no race between registering
      and destroying the recents input consumer



Change-Id: I64d2453c3671747b5799f26b628448fdf03bdb77

7 files changed, 45 insertions, 35 deletions

diff --git a/core/java/android/view/IWindowManager.aidl b/core/java/android/view/IWindowManager.aidl
index c10fc9f9cb09..6c3b8ab19792 100644
--- a/core/java/android/view/IWindowManager.aidl
+++ b/core/java/android/view/IWindowManager.aidl
@@ -525,11 +525,11 @@ interface IWindowManager
         out InputChannel inputChannel);
 
     /**
-     * Destroy an input consumer by name and display id.
+     * Destroy an input consumer by token and display id.
      * This method will also dispose the input channels associated with that InputConsumer.
      */
     @UnsupportedAppUsage
-    boolean destroyInputConsumer(String name, int displayId);
+    boolean destroyInputConsumer(IBinder token, int displayId);
 
     /**
      * Return the touch region for the current IME window, or an empty region if there is none.
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipInputConsumer.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipInputConsumer.java
index 8e3376f163c1..f6cab485fa2a 100644
--- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipInputConsumer.java
+++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipInputConsumer.java
@@ -139,7 +139,7 @@ public class PipInputConsumer {
         final InputChannel inputChannel = new InputChannel();
         try {
             // TODO(b/113087003): Support Picture-in-picture in multi-display.
-            mWindowManager.destroyInputConsumer(mName, DEFAULT_DISPLAY);
+            mWindowManager.destroyInputConsumer(mToken, DEFAULT_DISPLAY);
             mWindowManager.createInputConsumer(mToken, mName, DEFAULT_DISPLAY, inputChannel);
         } catch (RemoteException e) {
             ProtoLog.e(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
@@ -163,7 +163,7 @@ public class PipInputConsumer {
         }
         try {
             // TODO(b/113087003): Support Picture-in-picture in multi-display.
-            mWindowManager.destroyInputConsumer(mName, DEFAULT_DISPLAY);
+            mWindowManager.destroyInputConsumer(mToken, DEFAULT_DISPLAY);
         } catch (RemoteException e) {
             ProtoLog.e(ShellProtoLogGroup.WM_SHELL_PICTURE_IN_PICTURE,
                     "%s: Failed to destroy input consumer, %s", TAG, e);
diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/system/InputConsumerController.java b/packages/SystemUI/shared/src/com/android/systemui/shared/system/InputConsumerController.java
index ba0a6d149707..b406e72ffb98 100644
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/system/InputConsumerController.java
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/system/InputConsumerController.java
@@ -139,7 +139,7 @@ public class InputConsumerController {
         if (mInputEventReceiver == null) {
             final InputChannel inputChannel = new InputChannel();
             try {
-                mWindowManager.destroyInputConsumer(mName, DEFAULT_DISPLAY);
+                mWindowManager.destroyInputConsumer(mToken, DEFAULT_DISPLAY);
                 mWindowManager.createInputConsumer(mToken, mName, DEFAULT_DISPLAY, inputChannel);
             } catch (RemoteException e) {
                 Log.e(TAG, "Failed to create input consumer", e);
@@ -158,7 +158,7 @@ public class InputConsumerController {
     public void unregisterInputConsumer() {
         if (mInputEventReceiver != null) {
             try {
-                mWindowManager.destroyInputConsumer(mName, DEFAULT_DISPLAY);
+                mWindowManager.destroyInputConsumer(mToken, DEFAULT_DISPLAY);
             } catch (RemoteException e) {
                 Log.e(TAG, "Failed to destroy input consumer", e);
             }
diff --git a/services/core/java/com/android/server/wm/InputConsumerImpl.java b/services/core/java/com/android/server/wm/InputConsumerImpl.java
index 1fa7d2a2aa13..34d765117a57 100644
--- a/services/core/java/com/android/server/wm/InputConsumerImpl.java
+++ b/services/core/java/com/android/server/wm/InputConsumerImpl.java
@@ -160,7 +160,7 @@ class InputConsumerImpl implements IBinder.DeathRecipient {
             if (dc == null) {
                 return;
             }
-            dc.getInputMonitor().destroyInputConsumer(mName);
+            dc.getInputMonitor().destroyInputConsumer(mToken);
             unlinkFromDeathRecipient();
         }
     }
diff --git a/services/core/java/com/android/server/wm/InputMonitor.java b/services/core/java/com/android/server/wm/InputMonitor.java
index 5c0bc28779a8..61fea4d9212d 100644
--- a/services/core/java/com/android/server/wm/InputMonitor.java
+++ b/services/core/java/com/android/server/wm/InputMonitor.java
@@ -73,6 +73,7 @@ import com.android.server.inputmethod.InputMethodManagerInternal;
 
 import java.io.PrintWriter;
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 import java.util.Set;
 import java.util.function.Consumer;
 
@@ -104,7 +105,7 @@ final class InputMonitor {
      * The set of input consumer added to the window manager by name, which consumes input events
      * for the windows below it.
      */
-    private final ArrayMap<String, InputConsumerImpl> mInputConsumers = new ArrayMap();
+    private final ArrayList<InputConsumerImpl> mInputConsumers = new ArrayList<>();
 
     /**
      * Set when recents (overview) is active as part of a shell transition. While set, any focus
@@ -164,31 +165,35 @@ final class InputMonitor {
         mDisplayRemoved = true;
     }
 
-    private void addInputConsumer(String name, InputConsumerImpl consumer) {
-        mInputConsumers.put(name, consumer);
+    private void addInputConsumer(InputConsumerImpl consumer) {
+        mInputConsumers.add(consumer);
         consumer.linkToDeathRecipient();
         consumer.layout(mInputTransaction, mDisplayWidth, mDisplayHeight);
         updateInputWindowsLw(true /* force */);
     }
 
-    boolean destroyInputConsumer(String name) {
-        if (disposeInputConsumer(mInputConsumers.remove(name))) {
-            updateInputWindowsLw(true /* force */);
-            return true;
-        }
-        return false;
-    }
-
-    private boolean disposeInputConsumer(InputConsumerImpl consumer) {
-        if (consumer != null) {
-            consumer.disposeChannelsLw(mInputTransaction);
-            return true;
+    boolean destroyInputConsumer(IBinder token) {
+        for (int i = 0; i < mInputConsumers.size(); i++) {
+            final InputConsumerImpl consumer = mInputConsumers.get(i);
+            if (consumer != null && consumer.mToken == token) {
+                consumer.disposeChannelsLw(mInputTransaction);
+                mInputConsumers.remove(consumer);
+                updateInputWindowsLw(true /* force */);
+                return true;
+            }
         }
         return false;
     }
 
     InputConsumerImpl getInputConsumer(String name) {
-        return mInputConsumers.get(name);
+        // Search in reverse order as the latest input consumer with the name takes precedence
+        for (int i = mInputConsumers.size() - 1; i >= 0; i--) {
+            final InputConsumerImpl consumer = mInputConsumers.get(i);
+            if (consumer.mName.equals(name)) {
+                return consumer;
+            }
+        }
+        return null;
     }
 
     void layoutInputConsumers(int dw, int dh) {
@@ -200,7 +205,7 @@ final class InputMonitor {
         try {
             Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "layoutInputConsumer");
             for (int i = mInputConsumers.size() - 1; i >= 0; i--) {
-                mInputConsumers.valueAt(i).layout(mInputTransaction, dw, dh);
+                mInputConsumers.get(i).layout(mInputTransaction, dw, dh);
             }
         } finally {
             Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
@@ -212,15 +217,16 @@ final class InputMonitor {
     // (set so by this function) and must meet some condition for visibility on each update.
     void resetInputConsumers(SurfaceControl.Transaction t) {
         for (int i = mInputConsumers.size() - 1; i >= 0; i--) {
-            mInputConsumers.valueAt(i).hide(t);
+            mInputConsumers.get(i).hide(t);
         }
     }
 
     void createInputConsumer(IBinder token, String name, InputChannel inputChannel, int clientPid,
             UserHandle clientUser) {
-        if (mInputConsumers.containsKey(name)) {
+        final InputConsumerImpl existingConsumer = getInputConsumer(name);
+        if (existingConsumer != null && existingConsumer.mClientUser.equals(clientUser)) {
             throw new IllegalStateException("Existing input consumer found with name: " + name
-                    + ", display: " + mDisplayId);
+                    + ", display: " + mDisplayId + ", user: " + clientUser);
         }
 
         final InputConsumerImpl consumer = new InputConsumerImpl(mService, token, name,
@@ -239,7 +245,7 @@ final class InputMonitor {
                 throw new IllegalArgumentException("Illegal input consumer : " + name
                         + ", display: " + mDisplayId);
         }
-        addInputConsumer(name, consumer);
+        addInputConsumer(consumer);
     }
 
     @VisibleForTesting
@@ -541,11 +547,11 @@ final class InputMonitor {
     }
 
     void dump(PrintWriter pw, String prefix) {
-        final Set<String> inputConsumerKeys = mInputConsumers.keySet();
-        if (!inputConsumerKeys.isEmpty()) {
+        if (!mInputConsumers.isEmpty()) {
             pw.println(prefix + "InputConsumers:");
-            for (String key : inputConsumerKeys) {
-                mInputConsumers.get(key).dump(pw, key, prefix);
+            for (int i = 0; i < mInputConsumers.size(); i++) {
+                final InputConsumerImpl consumer = mInputConsumers.get(i);
+                consumer.dump(pw, consumer.mName, prefix);
             }
         }
     }
diff --git a/services/core/java/com/android/server/wm/RecentsAnimationController.java b/services/core/java/com/android/server/wm/RecentsAnimationController.java
index 82d4b90d06be..ef2572665281 100644
--- a/services/core/java/com/android/server/wm/RecentsAnimationController.java
+++ b/services/core/java/com/android/server/wm/RecentsAnimationController.java
@@ -1021,7 +1021,11 @@ public class RecentsAnimationController implements DeathRecipient {
         synchronized (mService.getWindowManagerLock()) {
             // Clear associated input consumers on runner death
             final InputMonitor inputMonitor = mDisplayContent.getInputMonitor();
-            inputMonitor.destroyInputConsumer(INPUT_CONSUMER_RECENTS_ANIMATION);
+            final InputConsumerImpl consumer = inputMonitor.getInputConsumer(
+                    INPUT_CONSUMER_RECENTS_ANIMATION);
+            if (consumer != null) {
+                inputMonitor.destroyInputConsumer(consumer.mToken);
+            }
         }
     }
 
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 4a074ff25c74..30808bcc840c 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -6553,7 +6553,7 @@ public class WindowManagerService extends IWindowManager.Stub
     }
 
     @Override
-    public boolean destroyInputConsumer(String name, int displayId) {
+    public boolean destroyInputConsumer(IBinder token, int displayId) {
         if (!mAtmService.isCallerRecents(Binder.getCallingUid())
                 && mContext.checkCallingOrSelfPermission(INPUT_CONSUMER) != PERMISSION_GRANTED) {
             throw new SecurityException("destroyInputConsumer requires INPUT_CONSUMER permission");
@@ -6562,7 +6562,7 @@ public class WindowManagerService extends IWindowManager.Stub
         synchronized (mGlobalLock) {
             DisplayContent display = mRoot.getDisplayContent(displayId);
             if (display != null) {
-                return display.getInputMonitor().destroyInputConsumer(name);
+                return display.getInputMonitor().destroyInputConsumer(token);
             }
             return false;
         }