Ensure IC#finishComposingText() is called on the correct Handler.
This attempts to reland previously reverted CLs [1][2] due to an
unexpected regression (Bug 27824691).
The Bug 27868748 we want to address by this CL is that currently
InputConnection#finishComposingText() can be called on the root view's
Handler no matter what Handler is associated with
ControlledInputConnectionWrapper. Actually the root cause of
Bug 6789252 is the same, but there we worked around it by not calling
InputConnection#finishComposingText() in certain situations [3].
With this CL we should be able to logically revert that workaround.
This CL also removes redundant IMM#mServedInputConnection. This is safe
because the following two fields have the same lifetime.
- InputMethodManager#mServedInputConnection
- InputMethodManager#mServedInputConnectionWrapper
We do not need to maintain both of them. This also allows us to use a
strong refecente in IInputConnectionWrapper#mInputConnection instead of
a WeakReference. To understand why this is safe, we need to understand
how things previously worked, which is as follows:
1. InputMethodManager#mServedInputConnection becomes non-null.
-> IInputConnectionWrapper#mInputConnection.get() is guaranteed to
be alive.
2. InputMethodManager#mServedInputConnection becomes null or another
object.
-> IInputConnectionWrapper#mInputConnection.get() may not be alive.
Since we know exactly when InputMethodManager#mServedInputConnection is
updated, in theory we do not need to use WeakReference here, and
with this CL we do not use WeakReference anymore. Actually the initial
commit [1] accidentally removed the last strong reference to the active
InputConnection and WeakReference could be null at any time, which was
what we observed in Bug 27824691.
[1]: I1181e067aa5bedbdf0c7ec1bcec479257aea511c
afb6558c8f5e0ee797b252558d7e529e3d946d8f
[2]: Ibe94f115e607a198d12ecd3d4e4f91a7d9469c98
16e2c7b59aacf44df7aaa0d04e0228240907487f
[3]: I66f51da1299532793ef8fa700f35b0811670f235
4e5184f929d2498714bc7734fe10b9b8810cb071
Bug: 27868748
Change-Id: If2a03bc84d318775fd4a197fa43acde086eda442
diff --git a/core/java/com/android/internal/view/IInputConnectionWrapper.java b/core/java/com/android/internal/view/IInputConnectionWrapper.java
index 3be15f3..6c1ebb4 100644
--- a/core/java/com/android/internal/view/IInputConnectionWrapper.java
+++ b/core/java/com/android/internal/view/IInputConnectionWrapper.java
@@ -16,6 +16,10 @@
package com.android.internal.view;
+import com.android.internal.annotations.GuardedBy;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
@@ -23,6 +27,7 @@
import android.os.RemoteException;
import android.util.Log;
import android.view.KeyEvent;
+import android.view.inputmethod.BaseInputConnection;
import android.view.inputmethod.CompletionInfo;
import android.view.inputmethod.CorrectionInfo;
import android.view.inputmethod.ExtractedTextRequest;
@@ -56,11 +61,17 @@
private static final int DO_PERFORM_PRIVATE_COMMAND = 120;
private static final int DO_CLEAR_META_KEY_STATES = 130;
private static final int DO_REQUEST_UPDATE_CURSOR_ANCHOR_INFO = 140;
+ private static final int DO_REPORT_FINISH = 150;
- private WeakReference<InputConnection> mInputConnection;
+ @GuardedBy("mLock")
+ @Nullable
+ private InputConnection mInputConnection;
private Looper mMainLooper;
private Handler mH;
+ private Object mLock = new Object();
+ @GuardedBy("mLock")
+ private boolean mFinished = false;
static class SomeArgs {
Object arg1;
@@ -80,12 +91,25 @@
}
}
- public IInputConnectionWrapper(Looper mainLooper, InputConnection conn) {
- mInputConnection = new WeakReference<>(conn);
+ public IInputConnectionWrapper(Looper mainLooper, @NonNull InputConnection inputConnection) {
+ mInputConnection = inputConnection;
mMainLooper = mainLooper;
mH = new MyHandler(mMainLooper);
}
+ @Nullable
+ public InputConnection getInputConnection() {
+ synchronized (mLock) {
+ return mInputConnection;
+ }
+ }
+
+ protected boolean isFinished() {
+ synchronized (mLock) {
+ return mFinished;
+ }
+ }
+
abstract protected boolean isActive();
/**
@@ -198,6 +222,10 @@
seq, callback));
}
+ public void reportFinish() {
+ dispatchMessage(obtainMessage(DO_REPORT_FINISH));
+ }
+
void dispatchMessage(Message msg) {
// If we are calling this from the main thread, then we can call
// right through. Otherwise, we need to send the message to the
@@ -210,13 +238,13 @@
mH.sendMessage(msg);
}
-
+
void executeMessage(Message msg) {
switch (msg.what) {
case DO_GET_TEXT_AFTER_CURSOR: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "getTextAfterCursor on inactive InputConnection");
args.callback.setTextAfterCursor(null, args.seq);
@@ -232,7 +260,7 @@
case DO_GET_TEXT_BEFORE_CURSOR: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "getTextBeforeCursor on inactive InputConnection");
args.callback.setTextBeforeCursor(null, args.seq);
@@ -248,7 +276,7 @@
case DO_GET_SELECTED_TEXT: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "getSelectedText on inactive InputConnection");
args.callback.setSelectedText(null, args.seq);
@@ -264,7 +292,7 @@
case DO_GET_CURSOR_CAPS_MODE: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "getCursorCapsMode on inactive InputConnection");
args.callback.setCursorCapsMode(0, args.seq);
@@ -280,7 +308,7 @@
case DO_GET_EXTRACTED_TEXT: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "getExtractedText on inactive InputConnection");
args.callback.setExtractedText(null, args.seq);
@@ -294,7 +322,7 @@
return;
}
case DO_COMMIT_TEXT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "commitText on inactive InputConnection");
return;
@@ -304,7 +332,7 @@
return;
}
case DO_SET_SELECTION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "setSelection on inactive InputConnection");
return;
@@ -313,7 +341,7 @@
return;
}
case DO_PERFORM_EDITOR_ACTION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "performEditorAction on inactive InputConnection");
return;
@@ -322,7 +350,7 @@
return;
}
case DO_PERFORM_CONTEXT_MENU_ACTION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "performContextMenuAction on inactive InputConnection");
return;
@@ -331,7 +359,7 @@
return;
}
case DO_COMMIT_COMPLETION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "commitCompletion on inactive InputConnection");
return;
@@ -340,7 +368,7 @@
return;
}
case DO_COMMIT_CORRECTION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "commitCorrection on inactive InputConnection");
return;
@@ -349,7 +377,7 @@
return;
}
case DO_SET_COMPOSING_TEXT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "setComposingText on inactive InputConnection");
return;
@@ -359,7 +387,7 @@
return;
}
case DO_SET_COMPOSING_REGION: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "setComposingRegion on inactive InputConnection");
return;
@@ -368,7 +396,7 @@
return;
}
case DO_FINISH_COMPOSING_TEXT: {
- InputConnection ic = mInputConnection.get();
+ 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
@@ -381,7 +409,7 @@
return;
}
case DO_SEND_KEY_EVENT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "sendKeyEvent on inactive InputConnection");
return;
@@ -391,7 +419,7 @@
return;
}
case DO_CLEAR_META_KEY_STATES: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "clearMetaKeyStates on inactive InputConnection");
return;
@@ -400,7 +428,7 @@
return;
}
case DO_DELETE_SURROUNDING_TEXT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "deleteSurroundingText on inactive InputConnection");
return;
@@ -409,7 +437,7 @@
return;
}
case DO_DELETE_SURROUNDING_TEXT_IN_CODE_POINTS: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "deleteSurroundingTextInCodePoints on inactive InputConnection");
return;
@@ -418,7 +446,7 @@
return;
}
case DO_BEGIN_BATCH_EDIT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "beginBatchEdit on inactive InputConnection");
return;
@@ -427,7 +455,7 @@
return;
}
case DO_END_BATCH_EDIT: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "endBatchEdit on inactive InputConnection");
return;
@@ -436,7 +464,7 @@
return;
}
case DO_REPORT_FULLSCREEN_MODE: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null) {
Log.w(TAG, "reportFullscreenMode on inexistent InputConnection");
return;
@@ -447,7 +475,7 @@
return;
}
case DO_PERFORM_PRIVATE_COMMAND: {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "performPrivateCommand on inactive InputConnection");
return;
@@ -460,7 +488,7 @@
case DO_REQUEST_UPDATE_CURSOR_ANCHOR_INFO: {
SomeArgs args = (SomeArgs)msg.obj;
try {
- InputConnection ic = mInputConnection.get();
+ InputConnection ic = getInputConnection();
if (ic == null || !isActive()) {
Log.w(TAG, "requestCursorAnchorInfo on inactive InputConnection");
args.callback.setRequestUpdateCursorAnchorInfoResult(false, args.seq);
@@ -473,6 +501,37 @@
}
return;
}
+ case DO_REPORT_FINISH: {
+ // 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 DO_REPORT_FINISH messages will not be handled at the
+ // same time.
+ if (isFinished()) {
+ return;
+ }
+ 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;
+ }
+ ic.finishComposingText();
+ // TODO: Make reportFinish() public method of InputConnection to remove this
+ // check.
+ if (ic instanceof BaseInputConnection) {
+ ((BaseInputConnection) ic).reportFinish();
+ }
+ } finally {
+ synchronized (mLock) {
+ mInputConnection = null;
+ mFinished = true;
+ }
+ }
+ return;
+ }
}
Log.w(TAG, "Unhandled message code: " + msg.what);
}