From 2b5fbda551cbf90c8e4de14f0404ad07c07ac4a3 Mon Sep 17 00:00:00 2001
From: Yohei Yukawa <yukawa@google.com>
Date: Tue, 8 Aug 2023 13:08:49 -0700
Subject: [PATCH] RemoteInputConnectionImpl should not call on IMM#isActive()
Historically RemoteInputConnectionImpl has been calling on
InputMethodManager#isActive()
to see if the InputConnection is considered to be active or inactive
when actually handling each incoming InputConnection operation.
However, InputMethodManager#isActive() is also known to be tricky
because it internally calls InputMethodManager#checkFocus(), which can
have non-trivial side effects.
With this CL, RemoteInputConnectionImpl keeps maintains its own
boolean state on whether #deactivate() is already called or not so
that it does not need to rely on IMM#isActive() any more.
For 99% cases there must be no observable behavior change, and for the
remaining 1% cases the new behavior should be more easily
understandable.
Bug: 291826769
Test: atest CtsInputMethodTestCases:InputConnectionHandlerTest#testInputConnectionSideEffect
Merged-In: I2fb9c549da19ff01e7cc3fd8bfc1f9c19aa0f0a8
Change-Id: I2fb9c549da19ff01e7cc3fd8bfc1f9c19aa0f0a8
---
.../RemoteInputConnectionImpl.java | 110 ++++++++----------
1 file changed, 49 insertions(+), 61 deletions(-)
diff --git a/core/java/android/view/inputmethod/RemoteInputConnectionImpl.java b/core/java/android/view/inputmethod/RemoteInputConnectionImpl.java
index 46ccef3c0102..c9ffdae890c5 100644
--- a/core/java/android/view/inputmethod/RemoteInputConnectionImpl.java
+++ b/core/java/android/view/inputmethod/RemoteInputConnectionImpl.java
@@ -166,6 +166,8 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
@NonNull
private final AtomicReference<InputConnection> mInputConnectionRef;
+ @NonNull
+ private final AtomicBoolean mDeactivateRequested = new AtomicBoolean(false);
@NonNull
private final Looper mLooper;
@@ -225,10 +227,6 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return mInputConnectionRef.get() == null;
}
- private boolean isActive() {
- return mParentInputMethodManager.isActive() && !isFinished();
- }
-
private View getServedView() {
return mServedView.get();
}
@@ -363,28 +361,18 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
*/
@Dispatching(cancellable = false)
public void deactivate() {
- if (isFinished()) {
+ if (mDeactivateRequested.getAndSet(true)) {
// This is a small performance optimization. Still only the 1st call of
- // reportFinish() will take effect.
+ // deactivate() will take effect.
return;
}
dispatch(() -> {
// Deactivate the notifier when finishing typing.
notifyTypingHint(false /* isTyping */, true /* deactivate */);
- // Note that we do not need to worry about race condition here, because 1) mFinished is
- // updated only inside this block, and 2) the code here is running on a Handler hence we
- // assume multiple closeConnection() tasks will not be handled at the same time.
- if (isFinished()) {
- return;
- }
Trace.traceBegin(Trace.TRACE_TAG_INPUT, "InputConnection#closeConnection");
try {
InputConnection ic = getInputConnection();
- // Note we do NOT check isActive() here, because this is safe
- // for an IME to call at any time, and we need to allow it
- // through to clean up our state after the IME has switched to
- // another client.
if (ic == null) {
return;
}
@@ -448,7 +436,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
public String toString() {
return "RemoteInputConnectionImpl{"
+ "connection=" + getInputConnection()
- + " mParentInputMethodManager.isActive()=" + mParentInputMethodManager.isActive()
+ + " mDeactivateRequested=" + mDeactivateRequested.get()
+ " mServedView=" + mServedView.get()
+ "}";
}
@@ -483,7 +471,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
public void dispatchReportFullscreenMode(boolean enabled) {
dispatch(() -> {
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
return;
}
ic.reportFullscreenMode(enabled);
@@ -499,7 +487,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getTextAfterCursor on inactive InputConnection");
return null;
}
@@ -521,7 +509,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getTextBeforeCursor on inactive InputConnection");
return null;
}
@@ -543,7 +531,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getSelectedText on inactive InputConnection");
return null;
}
@@ -565,7 +553,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getSurroundingText on inactive InputConnection");
return null;
}
@@ -593,7 +581,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return 0; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getCursorCapsMode on inactive InputConnection");
return 0;
}
@@ -610,7 +598,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getExtractedText on inactive InputConnection");
return null;
}
@@ -627,7 +615,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitText on inactive InputConnection");
return;
}
@@ -645,7 +633,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitText on inactive InputConnection");
return;
}
@@ -661,7 +649,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitCompletion on inactive InputConnection");
return;
}
@@ -677,7 +665,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitCorrection on inactive InputConnection");
return;
}
@@ -697,7 +685,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setSelection on inactive InputConnection");
return;
}
@@ -713,7 +701,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performEditorAction on inactive InputConnection");
return;
}
@@ -729,7 +717,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performContextMenuAction on inactive InputConnection");
return;
}
@@ -745,7 +733,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setComposingRegion on inactive InputConnection");
return;
}
@@ -766,7 +754,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setComposingRegion on inactive InputConnection");
return;
}
@@ -783,7 +771,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setComposingText on inactive InputConnection");
return;
}
@@ -801,7 +789,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setComposingText on inactive InputConnection");
return;
}
@@ -830,7 +818,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "finishComposingTextFromImm on inactive InputConnection");
return;
}
@@ -854,7 +842,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null && !isActive()) {
+ if (ic == null && mDeactivateRequested.get()) {
Log.w(TAG, "finishComposingText on inactive InputConnection");
return;
}
@@ -870,7 +858,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "sendKeyEvent on inactive InputConnection");
return;
}
@@ -886,7 +874,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "clearMetaKeyStates on inactive InputConnection");
return;
}
@@ -903,7 +891,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "deleteSurroundingText on inactive InputConnection");
return;
}
@@ -921,7 +909,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "deleteSurroundingTextInCodePoints on inactive InputConnection");
return;
}
@@ -941,7 +929,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "beginBatchEdit on inactive InputConnection");
return;
}
@@ -957,7 +945,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "endBatchEdit on inactive InputConnection");
return;
}
@@ -973,7 +961,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performSpellCheck on inactive InputConnection");
return;
}
@@ -990,7 +978,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performPrivateCommand on inactive InputConnection");
return;
}
@@ -1028,7 +1016,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performHandwritingGesture on inactive InputConnection");
if (resultReceiver != null) {
resultReceiver.send(
@@ -1068,7 +1056,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "previewHandwritingGesture on inactive InputConnection");
return; // cancelled
}
@@ -1116,7 +1104,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
@InputConnection.CursorUpdateMode int cursorUpdateMode,
@InputConnection.CursorUpdateFilter int cursorUpdateFilter, int imeDisplayId) {
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "requestCursorUpdates on inactive InputConnection");
return false;
}
@@ -1154,7 +1142,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "requestTextBoundsInfo on inactive InputConnection");
resultReceiver.send(TextBoundsInfoResult.CODE_CANCELLED, null);
return;
@@ -1198,7 +1186,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return false; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitContent on inactive InputConnection");
return false;
}
@@ -1223,7 +1211,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setImeConsumesInput on inactive InputConnection");
return;
}
@@ -1247,7 +1235,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "replaceText on inactive InputConnection");
return;
}
@@ -1266,7 +1254,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "commitText on inactive InputConnection");
return;
}
@@ -1286,7 +1274,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "setSelection on inactive InputConnection");
return;
}
@@ -1303,7 +1291,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return null; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getSurroundingText on inactive InputConnection");
return null;
}
@@ -1331,7 +1319,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "deleteSurroundingText on inactive InputConnection");
return;
}
@@ -1347,7 +1335,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "sendKeyEvent on inactive InputConnection");
return;
}
@@ -1363,7 +1351,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performEditorAction on inactive InputConnection");
return;
}
@@ -1379,7 +1367,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "performContextMenuAction on inactive InputConnection");
return;
}
@@ -1396,7 +1384,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return 0; // cancelled
}
final InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "getCursorCapsMode on inactive InputConnection");
return 0;
}
@@ -1412,7 +1400,7 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
return; // cancelled
}
InputConnection ic = getInputConnection();
- if (ic == null || !isActive()) {
+ if (ic == null || mDeactivateRequested.get()) {
Log.w(TAG, "clearMetaKeyStates on inactive InputConnection");
return;
}
--
GitLab