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/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 9418aaf..fdf6979 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -3300,7 +3300,6 @@
private final static int MSG_DISPATCH_APP_VISIBILITY = 8;
private final static int MSG_DISPATCH_GET_NEW_SURFACE = 9;
private final static int MSG_DISPATCH_KEY_FROM_IME = 11;
- private final static int MSG_FINISH_INPUT_CONNECTION = 12;
private final static int MSG_CHECK_FOCUS = 13;
private final static int MSG_CLOSE_SYSTEM_DIALOGS = 14;
private final static int MSG_DISPATCH_DRAG_EVENT = 15;
@@ -3340,8 +3339,6 @@
return "MSG_DISPATCH_GET_NEW_SURFACE";
case MSG_DISPATCH_KEY_FROM_IME:
return "MSG_DISPATCH_KEY_FROM_IME";
- case MSG_FINISH_INPUT_CONNECTION:
- return "MSG_FINISH_INPUT_CONNECTION";
case MSG_CHECK_FOCUS:
return "MSG_CHECK_FOCUS";
case MSG_CLOSE_SYSTEM_DIALOGS:
@@ -3562,12 +3559,6 @@
}
enqueueInputEvent(event, null, QueuedInputEvent.FLAG_DELIVER_POST_IME, true);
} break;
- case MSG_FINISH_INPUT_CONNECTION: {
- InputMethodManager imm = InputMethodManager.peekInstance();
- if (imm != null) {
- imm.reportFinishInputConnection((InputConnection)msg.obj);
- }
- } break;
case MSG_CHECK_FOCUS: {
InputMethodManager imm = InputMethodManager.peekInstance();
if (imm != null) {
@@ -5878,11 +5869,6 @@
}
}
- public void dispatchFinishInputConnection(InputConnection connection) {
- Message msg = mHandler.obtainMessage(MSG_FINISH_INPUT_CONNECTION, connection);
- mHandler.sendMessage(msg);
- }
-
public void dispatchResized(Rect frame, Rect overscanInsets, Rect contentInsets,
Rect visibleInsets, Rect stableInsets, Rect outsets, boolean reportDraw,
Configuration newConfig, Rect backDropFrame, boolean forceLayout,
diff --git a/core/java/android/view/inputmethod/BaseInputConnection.java b/core/java/android/view/inputmethod/BaseInputConnection.java
index 6a830f8..a3c49c5 100644
--- a/core/java/android/view/inputmethod/BaseInputConnection.java
+++ b/core/java/android/view/inputmethod/BaseInputConnection.java
@@ -158,8 +158,8 @@
*
* @hide
*/
- protected void reportFinish() {
- // Intentionaly empty
+ public void reportFinish() {
+ // Intentionally empty
}
/**
diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java
index cf5cc3e..4bb0c3c 100644
--- a/core/java/android/view/inputmethod/InputMethodManager.java
+++ b/core/java/android/view/inputmethod/InputMethodManager.java
@@ -317,7 +317,6 @@
/**
* The InputConnection that was last retrieved from the served view.
*/
- InputConnection mServedInputConnection;
ControlledInputConnectionWrapper mServedInputConnectionWrapper;
/**
* The completions that were last provided by the served view.
@@ -498,7 +497,7 @@
// from a thread that created mServedView. That could happen
// the current activity is running in the system process.
// In that case, we really should not call
- // mServedInputConnection.finishComposingText.
+ // mServedInputConnectionWrapper.finishComposingText().
if (checkFocusNoStartInput(mHasBeenInactive, false)) {
final int reason = active ?
InputMethodClient.START_INPUT_REASON_ACTIVATED_BY_IMMS :
@@ -532,22 +531,25 @@
private static class ControlledInputConnectionWrapper extends IInputConnectionWrapper {
private final InputMethodManager mParentInputMethodManager;
- private boolean mActive;
public ControlledInputConnectionWrapper(final Looper mainLooper, final InputConnection conn,
final InputMethodManager inputMethodManager) {
super(mainLooper, conn);
mParentInputMethodManager = inputMethodManager;
- mActive = true;
}
@Override
public boolean isActive() {
- return mParentInputMethodManager.mActive && mActive;
+ return mParentInputMethodManager.mActive && !isFinished();
}
void deactivate() {
- mActive = false;
+ if (isFinished()) {
+ // This is a small performance optimization. Still only the 1st call of
+ // reportFinish() will take effect.
+ return;
+ }
+ reportFinish();
}
@Override
@@ -562,7 +564,9 @@
@Override
public String toString() {
- return "ControlledInputConnectionWrapper{mActive=" + mActive
+ return "ControlledInputConnectionWrapper{"
+ + "connection=" + getInputConnection()
+ + " finished=" + isFinished()
+ " mParentInputMethodManager.mActive=" + mParentInputMethodManager.mActive
+ "}";
}
@@ -780,7 +784,8 @@
*/
public boolean isAcceptingText() {
checkFocus();
- return mServedInputConnection != null;
+ return mServedInputConnectionWrapper != null &&
+ mServedInputConnectionWrapper.getInputConnection() != null;
}
/**
@@ -815,7 +820,6 @@
*/
void clearConnectionLocked() {
mCurrentTextBoxAttribute = null;
- mServedInputConnection = null;
if (mServedInputConnectionWrapper != null) {
mServedInputConnectionWrapper.deactivate();
mServedInputConnectionWrapper = null;
@@ -836,7 +840,6 @@
throw e.rethrowFromSystemServer();
}
}
- notifyInputConnectionFinished();
mServedView = null;
mCompletions = null;
mServedConnecting = false;
@@ -844,37 +847,6 @@
}
}
- /**
- * Notifies the served view that the current InputConnection will no longer be used.
- */
- private void notifyInputConnectionFinished() {
- if (mServedView != null && mServedInputConnection != null) {
- // We need to tell the previously served view that it is no
- // longer the input target, so it can reset its state. Schedule
- // this call on its window's Handler so it will be on the correct
- // thread and outside of our lock.
- ViewRootImpl viewRootImpl = mServedView.getViewRootImpl();
- if (viewRootImpl != null) {
- // This will result in a call to reportFinishInputConnection() below.
- viewRootImpl.dispatchFinishInputConnection(mServedInputConnection);
- }
- }
- }
-
- /**
- * Called from the FINISH_INPUT_CONNECTION message above.
- * @hide
- */
- public void reportFinishInputConnection(InputConnection ic) {
- if (mServedInputConnection != ic) {
- ic.finishComposingText();
- // To avoid modifying the public InputConnection interface
- if (ic instanceof BaseInputConnection) {
- ((BaseInputConnection) ic).reportFinish();
- }
- }
- }
-
public void displayCompletions(View view, CompletionInfo[] completions) {
checkFocus();
synchronized (mH) {
@@ -1240,9 +1212,10 @@
// Hook 'em up and let 'er rip.
mCurrentTextBoxAttribute = tba;
mServedConnecting = false;
- // Notify the served view that its previous input connection is finished
- notifyInputConnectionFinished();
- mServedInputConnection = ic;
+ if (mServedInputConnectionWrapper != null) {
+ mServedInputConnectionWrapper.deactivate();
+ mServedInputConnectionWrapper = null;
+ }
ControlledInputConnectionWrapper servedContext;
final int missingMethodFlags;
if (ic != null) {
@@ -1267,9 +1240,6 @@
servedContext = null;
missingMethodFlags = 0;
}
- if (mServedInputConnectionWrapper != null) {
- mServedInputConnectionWrapper.deactivate();
- }
mServedInputConnectionWrapper = servedContext;
try {
@@ -1413,7 +1383,7 @@
return false;
}
- InputConnection ic = null;
+ final ControlledInputConnectionWrapper ic;
synchronized (mH) {
if (mServedView == mNextServedView && !forceNewFocus) {
return false;
@@ -1433,7 +1403,7 @@
return false;
}
- ic = mServedInputConnection;
+ ic = mServedInputConnectionWrapper;
mServedView = mNextServedView;
mCurrentTextBoxAttribute = null;
@@ -2282,7 +2252,7 @@
} else {
p.println(" mCurrentTextBoxAttribute: null");
}
- p.println(" mServedInputConnection=" + mServedInputConnection);
+ p.println(" mServedInputConnectionWrapper=" + mServedInputConnectionWrapper);
p.println(" mCompletions=" + Arrays.toString(mCompletions));
p.println(" mCursorRect=" + mCursorRect);
p.println(" mCursorSelStart=" + mCursorSelStart
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);
}
diff --git a/core/java/com/android/internal/widget/EditableInputConnection.java b/core/java/com/android/internal/widget/EditableInputConnection.java
index f211ff2..a96b5a0 100644
--- a/core/java/com/android/internal/widget/EditableInputConnection.java
+++ b/core/java/com/android/internal/widget/EditableInputConnection.java
@@ -83,8 +83,11 @@
return false;
}
+ /**
+ * @hide
+ */
@Override
- protected void reportFinish() {
+ public void reportFinish() {
super.reportFinish();
synchronized(this) {