From 48441703770de186ae9e81aadd6be38233812a4d Mon Sep 17 00:00:00 2001 From: Kohsuke Yatoh Date: Tue, 12 Apr 2022 15:14:53 -0700 Subject: [PATCH] Notify IMMS synchronously on IME insets hidden. Currently, IMMS will be notified asynchronously when an IME hide animation finishes, via message dispatching through IMS (IMM#notifyImeHidden -> IMS#notifyImeHidden -> IMMS#hideMySoftInput). This creates a race condition when IMM#showSoftInput or WIC#show is called around the end of hide animation. This CL fixes the race condition by synchronously and directly calling IMMS#hideSoftInput from IMM#notifyImeHidden. Note that there is still another race condition for IMM#showSoftInput (not WIC#show) if it's called during an IME hide animation; IMM#showSoftInput ended up calling WIC#show asynchronously, but at that time the running IME hide animation may have already been finished successfully and WIC#show may fail to cancel the hide animation (then the cleanup IMM#notifyImeHidden hides the IME again disruptively). I will fix the latter issue in a separate CL. Bug: 221483132 Bug: 225674038 Test: atest InputMethodStressTest Test: atest CtsInputMethodTestCases Test: atest WindowInsetsAnimationControllerTests Change-Id: I7c71dc5a1d6b61aa79d1666f0e257e6401e4adb2 (cherry picked from commit 9065310f814d38c5d8dea5f01655b30e4996fef4) Merged-In: I7c71dc5a1d6b61aa79d1666f0e257e6401e4adb2 --- .../IInputMethodSessionWrapper.java | 10 ---------- .../inputmethodservice/InputMethodService.java | 12 ------------ core/java/android/view/ImeInsetsSourceConsumer.java | 4 ++-- .../android/view/inputmethod/InputMethodManager.java | 10 ++++++++-- .../android/view/inputmethod/InputMethodSession.java | 7 ------- .../view/inputmethod/InputMethodSessionWrapper.java | 9 --------- .../android/internal/view/IInputMethodSession.aidl | 2 -- .../stresstest/ImeOpenCloseStressTest.java | 4 ---- 8 files changed, 10 insertions(+), 48 deletions(-) diff --git a/core/java/android/inputmethodservice/IInputMethodSessionWrapper.java b/core/java/android/inputmethodservice/IInputMethodSessionWrapper.java index eccbb403b3068..9a7ccc64efcd1 100644 --- a/core/java/android/inputmethodservice/IInputMethodSessionWrapper.java +++ b/core/java/android/inputmethodservice/IInputMethodSessionWrapper.java @@ -53,7 +53,6 @@ class IInputMethodSessionWrapper extends IInputMethodSession.Stub private static final int DO_APP_PRIVATE_COMMAND = 100; private static final int DO_FINISH_SESSION = 110; private static final int DO_VIEW_CLICKED = 115; - private static final int DO_NOTIFY_IME_HIDDEN = 120; private static final int DO_REMOVE_IME_SURFACE = 130; private static final int DO_FINISH_INPUT = 140; private static final int DO_INVALIDATE_INPUT = 150; @@ -133,10 +132,6 @@ class IInputMethodSessionWrapper extends IInputMethodSession.Stub mInputMethodSession.viewClicked(msg.arg1 == 1); return; } - case DO_NOTIFY_IME_HIDDEN: { - mInputMethodSession.notifyImeHidden(); - return; - } case DO_REMOVE_IME_SURFACE: { mInputMethodSession.removeImeSurface(); return; @@ -197,11 +192,6 @@ class IInputMethodSessionWrapper extends IInputMethodSession.Stub mCaller.obtainMessageI(DO_VIEW_CLICKED, focusChanged ? 1 : 0)); } - @Override - public void notifyImeHidden() { - mCaller.executeOrSendMessage(mCaller.obtainMessage(DO_NOTIFY_IME_HIDDEN)); - } - @Override public void removeImeSurface() { mCaller.executeOrSendMessage(mCaller.obtainMessage(DO_REMOVE_IME_SURFACE)); diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index 4fdd534253283..a6ed42348af68 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -1058,10 +1058,6 @@ public class InputMethodService extends AbstractInputMethodService { return viewRoot == null ? null : viewRoot.getInputToken(); } - private void notifyImeHidden() { - requestHideSelf(0); - } - private void scheduleImeSurfaceRemoval() { if (mShowInputRequested || mWindowVisible || mWindow == null || mImeSurfaceScheduledForRemoval) { @@ -1224,14 +1220,6 @@ public class InputMethodService extends AbstractInputMethodService { InputMethodService.this.onUpdateCursorAnchorInfo(info); } - /** - * Notify IME that window is hidden. - * @hide - */ - public final void notifyImeHidden() { - InputMethodService.this.notifyImeHidden(); - } - /** * Notify IME that surface can be now removed. * @hide diff --git a/core/java/android/view/ImeInsetsSourceConsumer.java b/core/java/android/view/ImeInsetsSourceConsumer.java index d609fb8eb2342..a3e654884ca53 100644 --- a/core/java/android/view/ImeInsetsSourceConsumer.java +++ b/core/java/android/view/ImeInsetsSourceConsumer.java @@ -23,7 +23,6 @@ import static android.view.InsetsController.AnimationType; import static android.view.InsetsState.ITYPE_IME; import android.annotation.Nullable; -import android.inputmethodservice.InputMethodService; import android.os.IBinder; import android.os.Trace; import android.util.proto.ProtoOutputStream; @@ -104,7 +103,8 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { } /** - * Notify {@link InputMethodService} that IME window is hidden. + * Notify {@link com.android.server.inputmethod.InputMethodManagerService} that + * IME insets are hidden. */ @Override void notifyHidden() { diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index 84f13930e03a8..d7752150e7d86 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -2521,7 +2521,7 @@ public final class InputMethodManager { } /** - * Notify IME directly that it is no longer visible. + * Notify IMMS that IME insets are no longer visible. * * @param windowToken the window from which this request originates. If this doesn't match the * currently served view, the request is ignored. @@ -2533,7 +2533,13 @@ public final class InputMethodManager { synchronized (mH) { if (mCurrentInputMethodSession != null && mCurRootView != null && mCurRootView.getWindowToken() == windowToken) { - mCurrentInputMethodSession.notifyImeHidden(); + try { + mService.hideSoftInput(mClient, windowToken, 0 /* flags */, + null /* resultReceiver */, + SoftInputShowHideReason.HIDE_SOFT_INPUT); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } } } } diff --git a/core/java/android/view/inputmethod/InputMethodSession.java b/core/java/android/view/inputmethod/InputMethodSession.java index a178ee8ee866e..28c44507e43cb 100644 --- a/core/java/android/view/inputmethod/InputMethodSession.java +++ b/core/java/android/view/inputmethod/InputMethodSession.java @@ -194,13 +194,6 @@ public interface InputMethodSession { */ public void updateCursorAnchorInfo(CursorAnchorInfo cursorAnchorInfo); - /** - * Notifies {@link android.inputmethodservice.InputMethodService} that IME has been - * hidden from user. - * @hide - */ - public void notifyImeHidden(); - /** * Notify IME directly to remove surface as it is no longer visible. * @hide diff --git a/core/java/android/view/inputmethod/InputMethodSessionWrapper.java b/core/java/android/view/inputmethod/InputMethodSessionWrapper.java index a1995202485fc..ee22b65085494 100644 --- a/core/java/android/view/inputmethod/InputMethodSessionWrapper.java +++ b/core/java/android/view/inputmethod/InputMethodSessionWrapper.java @@ -105,15 +105,6 @@ final class InputMethodSessionWrapper { } } - @AnyThread - void notifyImeHidden() { - try { - mSession.notifyImeHidden(); - } catch (RemoteException e) { - Log.w(TAG, "IME died", e); - } - } - @AnyThread void viewClicked(boolean focusChanged) { try { diff --git a/core/java/com/android/internal/view/IInputMethodSession.aidl b/core/java/com/android/internal/view/IInputMethodSession.aidl index b9eb997b851b9..d505c1995deff 100644 --- a/core/java/com/android/internal/view/IInputMethodSession.aidl +++ b/core/java/com/android/internal/view/IInputMethodSession.aidl @@ -50,8 +50,6 @@ oneway interface IInputMethodSession { void updateCursorAnchorInfo(in CursorAnchorInfo cursorAnchorInfo); - void notifyImeHidden(); - void removeImeSurface(); void finishInput(); diff --git a/tests/InputMethodStressTest/src/com/android/inputmethod/stresstest/ImeOpenCloseStressTest.java b/tests/InputMethodStressTest/src/com/android/inputmethod/stresstest/ImeOpenCloseStressTest.java index 1c957d4d78cf8..f01b9b4371077 100644 --- a/tests/InputMethodStressTest/src/com/android/inputmethod/stresstest/ImeOpenCloseStressTest.java +++ b/tests/InputMethodStressTest/src/com/android/inputmethod/stresstest/ImeOpenCloseStressTest.java @@ -27,7 +27,6 @@ import android.app.Activity; import android.app.Instrumentation; import android.content.Intent; import android.os.Bundle; -import android.os.SystemClock; import android.platform.test.annotations.RootPermissionTest; import android.platform.test.rule.UnlockScreenRule; import android.view.WindowInsets; @@ -78,9 +77,6 @@ public final class ImeOpenCloseStressTest { instrumentation.runOnMainSync(activity::hideIme); waitOnMainUntil(msgPrefix + "IME should be hidden", () -> !activity.isAnimating() && !isImeShown(editText)); - // b/b/221483132, wait until IMS and IMMS handles IMM#notifyImeHidden. - // There is no good signal, so we just wait a second. - SystemClock.sleep(1000); } }