From 10ba1119c7f1218d439f855b2de4c5dd593e382a Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Mon, 30 Jan 2023 16:28:45 +0000 Subject: [PATCH] Clean up ImeVisibilityApplier#perform{show, hide}Ime To solve a ping-ponging between the computer state and the applier that the ideal flow should as below to apply IME visiblity directly rather than backing to computer to set another state (i.e. In applier, setting show or hide input token to the computer): (app window) -> Computer -> Applier -> (IME) Fix: 246309664 Test: atest CtsInputMethodTestCases Test: atest DefaultImeVisibilityApplierTest Change-Id: I448bbb6770dcb8983ffd62979f64ced3b6c368d4 --- .../inputmethod/DefaultImeVisibilityApplier.java | 14 ++++---------- .../server/inputmethod/ImeVisibilityApplier.java | 8 ++++---- .../inputmethod/ImeVisibilityStateComputer.java | 11 +++-------- .../inputmethod/InputMethodManagerService.java | 12 +++++------- .../DefaultImeVisibilityApplierTest.java | 9 +++++---- 5 files changed, 21 insertions(+), 33 deletions(-) diff --git a/services/core/java/com/android/server/inputmethod/DefaultImeVisibilityApplier.java b/services/core/java/com/android/server/inputmethod/DefaultImeVisibilityApplier.java index e21895aced1fa..298f6ad6ac040 100644 --- a/services/core/java/com/android/server/inputmethod/DefaultImeVisibilityApplier.java +++ b/services/core/java/com/android/server/inputmethod/DefaultImeVisibilityApplier.java @@ -27,7 +27,6 @@ import static com.android.server.inputmethod.ImeVisibilityStateComputer.STATE_SH import static com.android.server.inputmethod.ImeVisibilityStateComputer.STATE_SHOW_IME_IMPLICIT; import android.annotation.Nullable; -import android.os.Binder; import android.os.IBinder; import android.os.ResultReceiver; import android.util.EventLog; @@ -65,13 +64,10 @@ final class DefaultImeVisibilityApplier implements ImeVisibilityApplier { @GuardedBy("ImfLock.class") @Override - public void performShowIme(IBinder windowToken, @Nullable ImeTracker.Token statsToken, + public void performShowIme(IBinder showInputToken, @Nullable ImeTracker.Token statsToken, int showFlags, ResultReceiver resultReceiver, @SoftInputShowHideReason int reason) { final IInputMethodInvoker curMethod = mService.getCurMethodLocked(); if (curMethod != null) { - // create a placeholder token for IMS so that IMS cannot inject windows into client app. - final IBinder showInputToken = new Binder(); - mService.setRequestImeTokenToWindow(windowToken, showInputToken); if (DEBUG) { Slog.v(TAG, "Calling " + curMethod + ".showSoftInput(" + showInputToken + ", " + showFlags + ", " + resultReceiver + ") for reason: " @@ -86,7 +82,7 @@ final class DefaultImeVisibilityApplier implements ImeVisibilityApplier { InputMethodDebug.softInputModeToString( mService.mCurFocusedWindowSoftInputMode)); } - mService.onShowHideSoftInputRequested(true /* show */, windowToken, reason, + mService.onShowHideSoftInputRequested(true /* show */, showInputToken, reason, statsToken); } } @@ -94,12 +90,10 @@ final class DefaultImeVisibilityApplier implements ImeVisibilityApplier { @GuardedBy("ImfLock.class") @Override - public void performHideIme(IBinder windowToken, @Nullable ImeTracker.Token statsToken, + public void performHideIme(IBinder hideInputToken, @Nullable ImeTracker.Token statsToken, ResultReceiver resultReceiver, @SoftInputShowHideReason int reason) { final IInputMethodInvoker curMethod = mService.getCurMethodLocked(); if (curMethod != null) { - final Binder hideInputToken = new Binder(); - mService.setRequestImeTokenToWindow(windowToken, hideInputToken); // The IME will report its visible state again after the following message finally // delivered to the IME process as an IPC. Hence the inconsistency between // IMMS#mInputShown and IMMS#mImeWindowVis should be resolved spontaneously in @@ -118,7 +112,7 @@ final class DefaultImeVisibilityApplier implements ImeVisibilityApplier { InputMethodDebug.softInputModeToString( mService.mCurFocusedWindowSoftInputMode)); } - mService.onShowHideSoftInputRequested(false /* show */, windowToken, reason, + mService.onShowHideSoftInputRequested(false /* show */, hideInputToken, reason, statsToken); } } diff --git a/services/core/java/com/android/server/inputmethod/ImeVisibilityApplier.java b/services/core/java/com/android/server/inputmethod/ImeVisibilityApplier.java index e97ec93b23c83..f03e985c07e99 100644 --- a/services/core/java/com/android/server/inputmethod/ImeVisibilityApplier.java +++ b/services/core/java/com/android/server/inputmethod/ImeVisibilityApplier.java @@ -31,26 +31,26 @@ interface ImeVisibilityApplier { /** * Performs showing IME on top of the given window. * - * @param windowToken The token of a window that currently has focus. + * @param showInputToken A token that represents the requester to show IME. * @param statsToken A token that tracks the progress of an IME request. * @param showFlags Provides additional operating flags to show IME. * @param resultReceiver If non-null, this will be called back to the caller when * it has processed request to tell what it has done. * @param reason The reason for requesting to show IME. */ - default void performShowIme(IBinder windowToken, @Nullable ImeTracker.Token statsToken, + default void performShowIme(IBinder showInputToken, @Nullable ImeTracker.Token statsToken, int showFlags, ResultReceiver resultReceiver, @SoftInputShowHideReason int reason) {} /** * Performs hiding IME to the given window * - * @param windowToken The token of a window that currently has focus. + * @param hideInputToken A token that represents the requester to hide IME. * @param statsToken A token that tracks the progress of an IME request. * @param resultReceiver If non-null, this will be called back to the caller when * it has processed request to tell what it has done. * @param reason The reason for requesting to hide IME. */ - default void performHideIme(IBinder windowToken, @Nullable ImeTracker.Token statsToken, + default void performHideIme(IBinder hideInputToken, @Nullable ImeTracker.Token statsToken, ResultReceiver resultReceiver, @SoftInputShowHideReason int reason) {} /** diff --git a/services/core/java/com/android/server/inputmethod/ImeVisibilityStateComputer.java b/services/core/java/com/android/server/inputmethod/ImeVisibilityStateComputer.java index db61e954893e3..eaca8425cd45d 100644 --- a/services/core/java/com/android/server/inputmethod/ImeVisibilityStateComputer.java +++ b/services/core/java/com/android/server/inputmethod/ImeVisibilityStateComputer.java @@ -35,6 +35,7 @@ import android.accessibilityservice.AccessibilityService; import android.annotation.IntDef; import android.annotation.NonNull; import android.content.res.Configuration; +import android.os.Binder; import android.os.IBinder; import android.util.PrintWriterPrinter; import android.util.Printer; @@ -263,6 +264,8 @@ public final class ImeVisibilityStateComputer { // policy. mPolicy.mPendingA11yRequestingHideKeyboard = false; } + // create a placeholder token for IMS so that IMS cannot inject windows into client app. + state.setRequestImeToken(new Binder()); setWindowStateInner(windowToken, state); } @@ -279,14 +282,6 @@ public final class ImeVisibilityStateComputer { return state; } - void setRequestImeTokenToWindow(IBinder windowToken, IBinder token) { - ImeTargetWindowState state = getWindowStateOrNull(windowToken); - if (state != null) { - state.setRequestImeToken(token); - setWindowStateInner(windowToken, state); - } - } - void setWindowState(IBinder windowToken, @NonNull ImeTargetWindowState newState) { final ImeTargetWindowState state = mRequestWindowStateMap.get(windowToken); if (state != null && newState.hasEdiorFocused()) { diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index 2ced9889f7cd8..27dacebec46ca 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -3344,11 +3344,6 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub } } - @GuardedBy("ImfLock.class") - void setRequestImeTokenToWindow(IBinder windowToken, IBinder token) { - mVisibilityStateComputer.setRequestImeTokenToWindow(windowToken, token); - } - @BinderThread @Override public void reportPerceptibleAsync(IBinder windowToken, boolean perceptible) { @@ -4603,10 +4598,13 @@ public final class InputMethodManagerService extends IInputMethodManager.Stub } } - /** Called right after {@link com.android.internal.inputmethod.IInputMethod#showSoftInput}. */ + /** + * Called right after {@link IInputMethod#showSoftInput} or {@link IInputMethod#hideSoftInput}. + */ @GuardedBy("ImfLock.class") - void onShowHideSoftInputRequested(boolean show, IBinder requestToken, + void onShowHideSoftInputRequested(boolean show, IBinder requestImeToken, @SoftInputShowHideReason int reason, @Nullable ImeTracker.Token statsToken) { + final IBinder requestToken = mVisibilityStateComputer.getWindowTokenFrom(requestImeToken); final WindowManagerInternal.ImeTargetInfo info = mWindowManagerInternal.onToggleImeRequested( show, mCurFocusedWindow, requestToken, mCurTokenDisplayId); diff --git a/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/DefaultImeVisibilityApplierTest.java b/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/DefaultImeVisibilityApplierTest.java index 720f486a72664..7e440494a1e45 100644 --- a/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/DefaultImeVisibilityApplierTest.java +++ b/services/tests/InputMethodSystemServerTests/src/com/android/server/inputmethod/DefaultImeVisibilityApplierTest.java @@ -34,6 +34,7 @@ import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import android.os.Binder; import android.os.RemoteException; import android.view.inputmethod.InputMethodManager; @@ -66,8 +67,8 @@ public class DefaultImeVisibilityApplierTest extends InputMethodManagerServiceTe @Test public void testPerformShowIme() throws Exception { synchronized (ImfLock.class) { - mVisibilityApplier.performShowIme(mWindowToken, null /* statsToken */, - InputMethodManager.SHOW_IMPLICIT, null, SHOW_SOFT_INPUT); + mVisibilityApplier.performShowIme(new Binder() /* showInputToken */, + null /* statsToken */, InputMethodManager.SHOW_IMPLICIT, null, SHOW_SOFT_INPUT); } verifyShowSoftInput(false, true, InputMethodManager.SHOW_IMPLICIT); } @@ -75,8 +76,8 @@ public class DefaultImeVisibilityApplierTest extends InputMethodManagerServiceTe @Test public void testPerformHideIme() throws Exception { synchronized (ImfLock.class) { - mVisibilityApplier.performHideIme(mWindowToken, null /* statsToken */, null, - HIDE_SOFT_INPUT); + mVisibilityApplier.performHideIme(new Binder() /* hideInputToken */, + null /* statsToken */, null, HIDE_SOFT_INPUT); } verifyHideSoftInput(false, true); }