From 1d113d041f113feabe0ff4cc57205fe8876b9e0b Mon Sep 17 00:00:00 2001 From: Tarandeep Singh Date: Fri, 13 Oct 2017 10:51:00 -0700 Subject: [PATCH] Skip blocking InputConnection APIs after unbind InputConnectionWrapper has several synchronous methods which have a timeout. If the application's UI thread hangs, all these synchronous methods are blocked and IME stays on-screen. This CL aims to improve the responsiveness of IMEs by rejecting any blocking calls of InputConnection APIs once IInputMethod#unbindInput() is issued by InputMethodManagerService (IMMS). Currently #unbindInput() is issued only from IMMS#unbindCurrentClientLocked(), which basically means that the previous application is losing the IME focus. Underlying #onUnbindInput() signal is still immediately delivered to the IME process, but it's just waiting to be consumed on the UI thread. Hence in theory we can change the behavior of InputConnection seen from the IME once the signal is delivered to the IME process. This CL does not interrupt already blocked API calls, which is one of future work for this scenario. This CL relies on: A. IInputMethod is marked as oneway B. IMMS guarantees that IInputMethod#bindInput() and IInputMethod#unbindInput() are always paired without nesting, and IInputMethod#startInput() is called 0 or more times only during that pair. In this case, the system guarantees that IInputMethod methods will be called back in the IME process in the same order, and only one IPC thread is handling those IPCs at the same time. See the JavaDoc of IBinder#FLAG_ONEWAY for details. Bug: 36897707 Test: Manual: using the apk in the linked bug: 1. Make sure that a valid InputConnection is established between the test app and a test IME. 2. Let the test app start blocking the UI thread. 3. Let the test IME call InputConnection#getTextBeforeCursor() three times. 4. Tap the Home button on the NavBar. 5. Make sure that the test app is immediately dismissed. 6. Make sure that InputConnection#getTextBeforeCursor() starts returning immediately, once after the initial call was timed- out after 2 sec (InputConnectionWrapper#MAX_WAIT_TIME_MILLIS) Change-Id: I0f816c6ca4c5c0664962432b913f074605fedd27 --- .../IInputMethodWrapper.java | 42 +++++++++++++++++-- .../internal/view/InputConnectionWrapper.java | 41 +++++++++++++++++- 2 files changed, 78 insertions(+), 5 deletions(-) diff --git a/core/java/android/inputmethodservice/IInputMethodWrapper.java b/core/java/android/inputmethodservice/IInputMethodWrapper.java index 2468225fe62f6..2c7e51a1db25c 100644 --- a/core/java/android/inputmethodservice/IInputMethodWrapper.java +++ b/core/java/android/inputmethodservice/IInputMethodWrapper.java @@ -48,6 +48,7 @@ import java.io.PrintWriter; import java.lang.ref.WeakReference; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; /** * Implements the internal IInputMethod interface to convert incoming calls @@ -76,6 +77,20 @@ class IInputMethodWrapper extends IInputMethod.Stub final WeakReference mInputMethod; final int mTargetSdkVersion; + /** + * This is not {@null} only between {@link #bindInput(InputBinding)} and {@link #unbindInput()} + * so that {@link InputConnectionWrapper} can query if {@link #unbindInput()} has already been + * called or not, mainly to avoid unnecessary blocking operations. + * + *

This field must be set and cleared only from the binder thread(s), where the system + * guarantees that {@link #bindInput(InputBinding)}, + * {@link #startInput(IBinder, IInputContext, int, EditorInfo, boolean)}, and + * {@link #unbindInput()} are called with the same order as the original calls + * in {@link com.android.server.InputMethodManagerService}. See {@link IBinder#FLAG_ONEWAY} + * for detailed semantics.

+ */ + AtomicBoolean mIsUnbindIssued = null; + // NOTE: we should have a cache of these. static final class InputMethodSessionCallbackWrapper implements InputMethod.SessionCallback { final Context mContext; @@ -163,8 +178,10 @@ class IInputMethodWrapper extends IInputMethod.Stub final IBinder startInputToken = (IBinder) args.arg1; final IInputContext inputContext = (IInputContext) args.arg2; final EditorInfo info = (EditorInfo) args.arg3; + final AtomicBoolean isUnbindIssued = (AtomicBoolean) args.arg4; final InputConnection ic = inputContext != null - ? new InputConnectionWrapper(mTarget, inputContext, missingMethods) : null; + ? new InputConnectionWrapper( + mTarget, inputContext, missingMethods, isUnbindIssued) : null; info.makeCompatible(mTargetSdkVersion); inputMethod.dispatchStartInputWithToken(ic, info, restarting /* restarting */, startInputToken); @@ -236,10 +253,15 @@ class IInputMethodWrapper extends IInputMethod.Stub @BinderThread @Override public void bindInput(InputBinding binding) { + if (mIsUnbindIssued != null) { + Log.e(TAG, "bindInput must be paired with unbindInput."); + } + mIsUnbindIssued = new AtomicBoolean(); // This IInputContext is guaranteed to implement all the methods. final int missingMethodFlags = 0; InputConnection ic = new InputConnectionWrapper(mTarget, - IInputContext.Stub.asInterface(binding.getConnectionToken()), missingMethodFlags); + IInputContext.Stub.asInterface(binding.getConnectionToken()), missingMethodFlags, + mIsUnbindIssued); InputBinding nu = new InputBinding(ic, binding); mCaller.executeOrSendMessage(mCaller.obtainMessageO(DO_SET_INPUT_CONTEXT, nu)); } @@ -247,6 +269,13 @@ class IInputMethodWrapper extends IInputMethod.Stub @BinderThread @Override public void unbindInput() { + if (mIsUnbindIssued != null) { + // Signal the flag then forget it. + mIsUnbindIssued.set(true); + mIsUnbindIssued = null; + } else { + Log.e(TAG, "unbindInput must be paired with bindInput."); + } mCaller.executeOrSendMessage(mCaller.obtainMessage(DO_UNSET_INPUT_CONTEXT)); } @@ -255,8 +284,13 @@ class IInputMethodWrapper extends IInputMethod.Stub public void startInput(IBinder startInputToken, IInputContext inputContext, @InputConnectionInspector.MissingMethodFlags final int missingMethods, EditorInfo attribute, boolean restarting) { - mCaller.executeOrSendMessage(mCaller.obtainMessageIIOOO(DO_START_INPUT, - missingMethods, restarting ? 1 : 0, startInputToken, inputContext, attribute)); + if (mIsUnbindIssued == null) { + Log.e(TAG, "startInput must be called after bindInput."); + mIsUnbindIssued = new AtomicBoolean(); + } + mCaller.executeOrSendMessage(mCaller.obtainMessageIIOOOO(DO_START_INPUT, + missingMethods, restarting ? 1 : 0, startInputToken, inputContext, attribute, + mIsUnbindIssued)); } @BinderThread diff --git a/core/java/com/android/internal/view/InputConnectionWrapper.java b/core/java/com/android/internal/view/InputConnectionWrapper.java index 2098ebd8814a1..5b65bbe1e90e6 100644 --- a/core/java/com/android/internal/view/InputConnectionWrapper.java +++ b/core/java/com/android/internal/view/InputConnectionWrapper.java @@ -36,6 +36,7 @@ import android.view.inputmethod.InputConnectionInspector.MissingMethodFlags; import android.view.inputmethod.InputContentInfo; import java.lang.ref.WeakReference; +import java.util.concurrent.atomic.AtomicBoolean; public class InputConnectionWrapper implements InputConnection { private static final int MAX_WAIT_TIME_MILLIS = 2000; @@ -46,6 +47,14 @@ public class InputConnectionWrapper implements InputConnection { @MissingMethodFlags private final int mMissingMethods; + /** + * {@code true} if the system already decided to take away IME focus from the target app. This + * can be signaled even when the corresponding signal is in the task queue and + * {@link InputMethodService#onUnbindInput()} is not yet called back on the UI thread. + */ + @NonNull + private final AtomicBoolean mIsUnbindIssued; + static class InputContextCallback extends IInputContextCallback.Stub { private static final String TAG = "InputConnectionWrapper.ICC"; public int mSeq; @@ -231,14 +240,20 @@ public class InputConnectionWrapper implements InputConnection { public InputConnectionWrapper( @NonNull WeakReference inputMethodService, - IInputContext inputContext, @MissingMethodFlags final int missingMethods) { + IInputContext inputContext, @MissingMethodFlags final int missingMethods, + @NonNull AtomicBoolean isUnbindIssued) { mInputMethodService = inputMethodService; mIInputContext = inputContext; mMissingMethods = missingMethods; + mIsUnbindIssued = isUnbindIssued; } @AnyThread public CharSequence getTextAfterCursor(int length, int flags) { + if (mIsUnbindIssued.get()) { + return null; + } + CharSequence value = null; try { InputContextCallback callback = InputContextCallback.getInstance(); @@ -258,6 +273,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public CharSequence getTextBeforeCursor(int length, int flags) { + if (mIsUnbindIssued.get()) { + return null; + } + CharSequence value = null; try { InputContextCallback callback = InputContextCallback.getInstance(); @@ -277,6 +296,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public CharSequence getSelectedText(int flags) { + if (mIsUnbindIssued.get()) { + return null; + } + if (isMethodMissing(MissingMethodFlags.GET_SELECTED_TEXT)) { // This method is not implemented. return null; @@ -300,6 +323,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public int getCursorCapsMode(int reqModes) { + if (mIsUnbindIssued.get()) { + return 0; + } + int value = 0; try { InputContextCallback callback = InputContextCallback.getInstance(); @@ -319,6 +346,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) { + if (mIsUnbindIssued.get()) { + return null; + } + ExtractedText value = null; try { InputContextCallback callback = InputContextCallback.getInstance(); @@ -516,6 +547,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public boolean requestCursorUpdates(int cursorUpdateMode) { + if (mIsUnbindIssued.get()) { + return false; + } + boolean result = false; if (isMethodMissing(MissingMethodFlags.REQUEST_CURSOR_UPDATES)) { // This method is not implemented. @@ -550,6 +585,10 @@ public class InputConnectionWrapper implements InputConnection { @AnyThread public boolean commitContent(InputContentInfo inputContentInfo, int flags, Bundle opts) { + if (mIsUnbindIssued.get()) { + return false; + } + boolean result = false; if (isMethodMissing(MissingMethodFlags.COMMIT_CONTENT)) { // This method is not implemented.