Instantiate InputMethodManager for each display (2nd try)

InputMethodManager has been a per-process singleton object. In order
to support behavior changes for multi-display support in Android Q,
however, InputMethodManager now needs to be per-display objects.

With this CL, context.getSystemService(InputMethodManager.class) will
start returning per-display InputMethodManager (IMM) instance.

  Why?

There are two major reasons.
 1. To support per-display focused window.
 2. To support more simplified API for multi-session IME.

Currently per-process InputMethodManager instance directly receives
callback from ViewRootImpl upon windowFocusChanged, then it keeps
track of which Window is focused by storing its root view into
InputMethodManager#mCurRootView.

This design assumes that (within the same process) at most one Window
can have window focus, which is no longer true once we start
supporting per-display focused window (Bug 111361570).

  Why we need to do this to support per-display focused window:

For traditional non multi-session IME cases (e.g. apps that use
Virtual Display APIs on phones), internal state of IMM can be easily
messed up once the system starts sending per-display
windowFocusChanged events to the same process, because IMM still
doesn't know that now each display has focused window. It is hard to
precisely predict what kind of issues we would see simply because such
a use case is most likely not expected in the original design.

  Why we need to do this for multi-session IME:

For multi-session IME scenarios, in addition to the above concern in
InputMethodManager, the current design allows at most one IME session
per process. This means that if a process X is showing Activities to 3
different displays, only one Activity can interact with the
multi-session IME at the same time. If we do not change the current
design, the only way to work around is to ask app developers to
explicitly use different processes for each Activity, which may
require a lot of work (e.g. SharedPreference is not optimized for
multi-process use cases). This would also make multi-session IME
development complicated because the IME cannot know on which display
the IME is interacting until startInputOrWindowGainedFocus() is
actually called, and needs to do all the preparation and cleanup tasks
whenever startInputOrWindowGainedFocus() is called for a different
display than it's currently interacting with.

  Alternative solutions considered:

Another possible approach is to update InputMethodManager singleton to
be able to maintain multiple mCurRootView and mServedView for each
display. This approach was abandoned because those fields and methods
are already marked as @UnsupportedAppUsage.  I concluded that touching
@UnsupportedAppUsage things would have bigger compatibility risks than
per-display instance model.

  Implementation note:

* Public APIs in IMM that take View instance as the first parameter
  will verify whether the given View and IMM are associated with the
  same display ID or not.  If there is a display ID mismatch, such an
  API call will be automatically forwarded to the correct IMM instance
  IMM with a clear warning in logcat which tells that app developers
  should use the correct IMM instance to avoid unnecessary performance
  overhead.

* As a general rule, system server process cannot trust display ID
  reported from applications.  In order to enable IMMS to verify the
  reported display ID, this CL also exposes display ID verification
  logic from WMS to other system components via WindowManagerInternal.

* isInputMethodClientFocus() in WindowManagerService (WMS) is updated
  to use top-focused-display to determine whether a given IME client
  has IME focus or not.  This is now necessary because with a recent
  change [1] each display can have focused window.  The previous logic
  to check all the displays that belong to the given pid/uid [2] no
  longer makes sense.

* Currently per-display InputMethodManager instances will not be
  garbage collected because InputMethodManager#sInstanceMap keeps
  holding strong references to them.  Freeing those instances is
  technically possible, but we need to be careful because multiple
  processes (app, system, IME) are involved and at least system
  process has a strict verification logic that lets the calling
  process crash with SecurityException.  We need to carefully
  implement such a cleanup logic to avoid random process crash due to
  race condition.  Bug 116699479 will take care of this task.

Also to make sure that the performance regression (Bug 117434607) we
observed after my initial attempt [3] no longer exists, here are the
benchmark results with and without this CL.

  testExpandNotificationsLatency on taimen-userdebug
    without this CL:
      results=[55, 46, 61, 67, 50, 48, 57, 50, 55, 63]
      min:46.0, max:67.0, avg:55.2, median:55.0, std_dev:6.539
    with this CL:
      results=[45, 55, 58, 57, 47, 60, 59, 60, 56, 53]
      min:45.0, max:60.0, avg:55.0, median:56.5, std_dev:4.980

 [1]: I776cabaeaf41ff4240f504fb1430d3e40892023d
      1e5b10a21780e09d9f7c762edffbdee4565af52c
 [2]: I8da315936caebdc8b2c16cff4e24192c06743251
      90120a8b5b14d4d0830b3b5f478bb627a7ac06ea
 [3]: I7242e765426353672823fcc8277f20ac361930d7
      c53d78e992694e471ddaae73f9a30977db9cdb75

Bug: 111364446
Fix: 115893206
Test: atest ActivityManagerMultiDisplayTests
Test: atest CtsInputMethodTestCases CtsInputMethodServiceHostTestCases
Test: atest FrameworksCoreTests:android.view.inputmethod.InputMethodManagerTest
Test: No perf regression in LatencyTests#testExpandNotificationsLatency()
Change-Id: I78ad7cccb9586474c83f7e2f90c0bcabb221c47b

2 files changed, 157 insertions, 18 deletions

diff --git a/core/java/android/app/SystemServiceRegistry.java b/core/java/android/app/SystemServiceRegistry.java
index 0044005c51f2..77cebc8f408d 100644
--- a/core/java/android/app/SystemServiceRegistry.java
+++ b/core/java/android/app/SystemServiceRegistry.java
@@ -377,11 +377,15 @@ final class SystemServiceRegistry {
                 return new DisplayManager(ctx.getOuterContext());
             }});
 
+        // InputMethodManager has its own cache strategy based on display id to support apps that
+        // still assume InputMethodManager is a per-process singleton and it's safe to directly
+        // access internal fields via reflection.  Hence directly use ServiceFetcher instead of
+        // StaticServiceFetcher/CachedServiceFetcher.
         registerService(Context.INPUT_METHOD_SERVICE, InputMethodManager.class,
-                new StaticServiceFetcher<InputMethodManager>() {
+                new ServiceFetcher<InputMethodManager>() {
             @Override
-            public InputMethodManager createService() {
-                return InputMethodManager.getInstanceInternal();
+            public InputMethodManager getService(ContextImpl ctx) {
+                return InputMethodManager.forContext(ctx.getOuterContext());
             }});
 
         registerService(Context.TEXT_SERVICES_MANAGER_SERVICE, TextServicesManager.class,
diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java
index ca2ccaf224db..e8e4b4aba0d4 100644
--- a/core/java/android/view/inputmethod/InputMethodManager.java
+++ b/core/java/android/view/inputmethod/InputMethodManager.java
@@ -48,6 +48,7 @@ import android.util.Pools.SimplePool;
 import android.util.PrintWriterPrinter;
 import android.util.Printer;
 import android.util.SparseArray;
+import android.view.Display;
 import android.view.InputChannel;
 import android.view.InputEvent;
 import android.view.InputEventSender;
@@ -265,7 +266,7 @@ public final class InputMethodManager {
      * @hide
      */
     public static void ensureDefaultInstanceForDefaultDisplayIfNecessary() {
-        getInstanceInternal();
+        forContextInternal(Display.DEFAULT_DISPLAY, Looper.getMainLooper());
     }
 
     private static final Object sLock = new Object();
@@ -279,6 +280,17 @@ public final class InputMethodManager {
     static InputMethodManager sInstance;
 
     /**
+     * Global map between display to {@link InputMethodManager}.
+     *
+     * <p>Currently this map works like a so-called leaky singleton.  Once an instance is registered
+     * for the associated display ID, that instance will never be garbage collected.</p>
+     *
+     * <p>TODO(Bug 116699479): Implement instance clean up mechanism.</p>
+     */
+    @GuardedBy("sLock")
+    private static final SparseArray<InputMethodManager> sInstanceMap = new SparseArray<>();
+
+    /**
      * @hide Flag for IInputMethodManager.windowGainedFocus: a view in
      * the window has input focus.
      */
@@ -335,6 +347,8 @@ public final class InputMethodManager {
     // Our generic input connection if the current target does not have its own.
     final IInputContext mIInputContext;
 
+    private final int mDisplayId;
+
     /**
      * True if this input method client is active, initially false.
      */
@@ -452,6 +466,29 @@ public final class InputMethodManager {
         return afm != null && afm.isAutofillUiShowing();
     }
 
+    /**
+     * Checks the consistency between {@link InputMethodManager} state and {@link View} state.
+     *
+     * @param view {@link View} to be checked
+     * @return {@code true} if {@code view} is not {@code null} and there is a {@link Context}
+     *         mismatch between {@link InputMethodManager} and {@code view}
+     */
+    private boolean shouldDispatchToViewContext(@Nullable View view) {
+        if (view == null) {
+            return false;
+        }
+        final int viewDisplayId = view.getContext().getDisplayId();
+        if (viewDisplayId != mDisplayId) {
+            Log.w(TAG, "b/117267690: Context mismatch found. view=" + view + " belongs to"
+                    + " displayId=" + viewDisplayId
+                    + " but InputMethodManager belongs to displayId=" + mDisplayId
+                    + ". Use the right InputMethodManager instance to avoid performance overhead.",
+                    new Throwable());
+            return true;
+        }
+        return false;
+    }
+
     private static boolean canStartInput(View servedView) {
         // We can start input ether the servedView has window focus
         // or the activity is showing autofill ui.
@@ -733,33 +770,52 @@ public final class InputMethodManager {
                 });
     }
 
-    InputMethodManager(Looper looper) throws ServiceNotFoundException {
+    InputMethodManager(int displayId, Looper looper) throws ServiceNotFoundException {
         mService = getIInputMethodManager();
         mMainLooper = looper;
         mH = new H(looper);
+        mDisplayId = displayId;
         mIInputContext = new ControlledInputConnectionWrapper(looper,
                 mDummyInputConnection, this);
     }
 
     /**
-     * Retrieve the global {@link InputMethodManager} instance, creating it if it doesn't already
-     * exist.
+     * Retrieve an instance for the given {@link Context}, creating it if it doesn't already exist.
      *
-     * @return global {@link InputMethodManager} instance
+     * @param context {@link Context} for which IME APIs need to work
+     * @return {@link InputMethodManager} instance
      * @hide
      */
-    public static InputMethodManager getInstanceInternal() {
+    @Nullable
+    public static InputMethodManager forContext(Context context) {
+        final int displayId = context.getDisplayId();
+        // For better backward compatibility, we always use Looper.getMainLooper() for the default
+        // display case.
+        final Looper looper = displayId == Display.DEFAULT_DISPLAY
+                ? Looper.getMainLooper() : context.getMainLooper();
+        return forContextInternal(displayId, looper);
+    }
+
+    @Nullable
+    private static InputMethodManager forContextInternal(int displayId, Looper looper) {
+        final boolean isDefaultDisplay = displayId == Display.DEFAULT_DISPLAY;
         synchronized (sLock) {
-            if (sInstance == null) {
-                try {
-                    final InputMethodManager imm = new InputMethodManager(Looper.getMainLooper());
-                    imm.mService.addClient(imm.mClient, imm.mIInputContext);
-                    sInstance = imm;
-                } catch (ServiceNotFoundException | RemoteException e) {
-                    throw new IllegalStateException(e);
-                }
+            InputMethodManager instance = sInstanceMap.get(displayId);
+            if (instance != null) {
+                return instance;
             }
-            return sInstance;
+            try {
+                instance = new InputMethodManager(displayId, looper);
+                instance.mService.addClient(instance.mClient, instance.mIInputContext, displayId);
+            } catch (ServiceNotFoundException | RemoteException e) {
+                throw new IllegalStateException(e);
+            }
+            // For backward compatibility, store the instance also to sInstance for default display.
+            if (sInstance == null && isDefaultDisplay) {
+                sInstance = instance;
+            }
+            sInstanceMap.put(displayId, instance);
+            return instance;
         }
     }
 
@@ -916,6 +972,11 @@ public final class InputMethodManager {
      * input method.
      */
     public boolean isActive(View view) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            return view.getContext().getSystemService(InputMethodManager.class).isActive(view);
+        }
+
         checkFocus();
         synchronized (mH) {
             return (mServedView == view
@@ -1006,6 +1067,13 @@ public final class InputMethodManager {
     }
 
     public void displayCompletions(View view, CompletionInfo[] completions) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .displayCompletions(view, completions);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if (mServedView != view && (mServedView == null
@@ -1024,6 +1092,13 @@ public final class InputMethodManager {
     }
 
     public void updateExtractedText(View view, int token, ExtractedText text) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .updateExtractedText(view, token, text);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if (mServedView != view && (mServedView == null
@@ -1065,6 +1140,12 @@ public final class InputMethodManager {
      * 0 or have the {@link #SHOW_IMPLICIT} bit set.
      */
     public boolean showSoftInput(View view, int flags) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            return view.getContext().getSystemService(InputMethodManager.class)
+                    .showSoftInput(view, flags);
+        }
+
         return showSoftInput(view, flags, null);
     }
 
@@ -1127,6 +1208,12 @@ public final class InputMethodManager {
      * {@link #RESULT_HIDDEN}.
      */
     public boolean showSoftInput(View view, int flags, ResultReceiver resultReceiver) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            return view.getContext().getSystemService(InputMethodManager.class)
+                    .showSoftInput(view, flags, resultReceiver);
+        }
+
         checkFocus();
         synchronized (mH) {
             if (mServedView != view && (mServedView == null
@@ -1290,6 +1377,12 @@ public final class InputMethodManager {
      * @param view The view whose text has changed.
      */
     public void restartInput(View view) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class).restartInput(view);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if (mServedView != view && (mServedView == null
@@ -1714,6 +1807,13 @@ public final class InputMethodManager {
      */
     public void updateSelection(View view, int selStart, int selEnd,
             int candidatesStart, int candidatesEnd) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .updateSelection(view, selStart, selEnd, candidatesStart, candidatesEnd);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if ((mServedView != view && (mServedView == null
@@ -1751,6 +1851,12 @@ public final class InputMethodManager {
      * Notify the event when the user tapped or clicked the text view.
      */
     public void viewClicked(View view) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class).viewClicked(view);
+            return;
+        }
+
         final boolean focusChanged = mServedView != mNextServedView;
         checkFocus();
         synchronized (mH) {
@@ -1815,6 +1921,13 @@ public final class InputMethodManager {
      */
     @Deprecated
     public void updateCursor(View view, int left, int top, int right, int bottom) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .updateCursor(view, left, top, right, bottom);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if ((mServedView != view && (mServedView == null
@@ -1846,6 +1959,13 @@ public final class InputMethodManager {
         if (view == null || cursorAnchorInfo == null) {
             return;
         }
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .updateCursorAnchorInfo(view, cursorAnchorInfo);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if ((mServedView != view &&
@@ -1891,6 +2011,13 @@ public final class InputMethodManager {
      * @param data Any data to include with the command.
      */
     public void sendAppPrivateCommand(View view, String action, Bundle data) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(view)) {
+            view.getContext().getSystemService(InputMethodManager.class)
+                    .sendAppPrivateCommand(view, action, data);
+            return;
+        }
+
         checkFocus();
         synchronized (mH) {
             if ((mServedView != view && (mServedView == null
@@ -2062,6 +2189,13 @@ public final class InputMethodManager {
      */
     public void dispatchKeyEventFromInputMethod(@Nullable View targetView,
             @NonNull KeyEvent event) {
+        // Re-dispatch if there is a context mismatch.
+        if (shouldDispatchToViewContext(targetView)) {
+            targetView.getContext().getSystemService(InputMethodManager.class)
+                    .dispatchKeyEventFromInputMethod(targetView, event);
+            return;
+        }
+
         synchronized (mH) {
             ViewRootImpl viewRootImpl = targetView != null ? targetView.getViewRootImpl() : null;
             if (viewRootImpl == null) {
@@ -2551,6 +2685,7 @@ public final class InputMethodManager {
         sb.append(",windowFocus=" + view.hasWindowFocus());
         sb.append(",autofillUiShowing=" + isAutofillUIShowing(view));
         sb.append(",window=" + view.getWindowToken());
+        sb.append(",displayId=" + view.getContext().getDisplayId());
         sb.append(",temporaryDetach=" + view.isTemporarilyDetached());
         return sb.toString();
     }