Log errors if RemoteInputConnection is used after IMS#onDestroy()

This CL consolidates weak-ref handlings in RemoteInputConnection so
that IME developers can find more useful messages in logcat when they
are using RemoteInputConnection after InputMethodService#onDestroy().

RemoteInputConnection has been designed to not hold a strong reference
to its parent InputMethodService instance.
While IME developers can manually keep a strong reference to
(Remote)InputConnection object that they obtained from
InputMethodService#getCurrentInputConnection(), doing so does not
prevent the system from garbage-collecting InputMethodService
instance.

With this CL, an error message will be shown in logcat if somehow
RemoteInputConnection object is still touched after InputMethodService
is destroyed, which hopefully can help IME developers realize they
might be doing something unexpected.

Other than showing error messages in logcat, there is no behavior
change in this CL.

Bug: 192412909
Bug: 194110780
Test: atest CtsInputMethodTestCases
Change-Id: I514630d27c8953f62fdb34cd3133a662b3fbbf76
This commit is contained in:
Yohei Yukawa
2021-09-10 11:37:30 -07:00
parent e5c0299689
commit 95d4f0dc17

View File

@@ -22,6 +22,7 @@ import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.Bundle;
import android.os.Handler;
import android.util.Log;
import android.view.KeyEvent;
import android.view.inputmethod.CompletionInfo;
import android.view.inputmethod.CorrectionInfo;
@@ -62,8 +63,29 @@ final class RemoteInputConnection implements InputConnection {
@NonNull
private final IInputContextInvoker mInvoker;
private static final class InputMethodServiceInternalHolder {
@NonNull
private final WeakReference<InputMethodServiceInternal> mServiceRef;
private InputMethodServiceInternalHolder(
@NonNull WeakReference<InputMethodServiceInternal> ims) {
mServiceRef = ims;
}
@AnyThread
@Nullable
public InputMethodServiceInternal getAndWarnIfNull() {
final InputMethodServiceInternal ims = mServiceRef.get();
if (ims == null) {
Log.e(TAG, "InputMethodService is already destroyed. InputConnection instances"
+ " cannot be used beyond InputMethodService lifetime.", new Throwable());
}
return ims;
}
}
@NonNull
private final WeakReference<InputMethodServiceInternal> mInputMethodService;
private final InputMethodServiceInternalHolder mImsInternal;
@MissingMethodFlags
private final int mMissingMethods;
@@ -81,7 +103,7 @@ final class RemoteInputConnection implements InputConnection {
@NonNull WeakReference<InputMethodServiceInternal> inputMethodService,
IInputContext inputContext, @MissingMethodFlags int missingMethods,
@NonNull CancellationGroup cancellationGroup) {
mInputMethodService = inputMethodService;
mImsInternal = new InputMethodServiceInternalHolder(inputMethodService);
mInvoker = IInputContextInvoker.create(inputContext);
mMissingMethods = missingMethods;
mCancellationGroup = cancellationGroup;
@@ -101,11 +123,11 @@ final class RemoteInputConnection implements InputConnection {
final CharSequence result = CompletableFutureUtil.getResultOrNull(
value, TAG, "getTextAfterCursor()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetTextAfterCursorProto(length,
flags, result);
inputMethodService.triggerServiceDump(TAG + "#getTextAfterCursor", icProto);
imsInternal.triggerServiceDump(TAG + "#getTextAfterCursor", icProto);
}
return result;
@@ -125,11 +147,11 @@ final class RemoteInputConnection implements InputConnection {
final CharSequence result = CompletableFutureUtil.getResultOrNull(
value, TAG, "getTextBeforeCursor()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetTextBeforeCursorProto(length,
flags, result);
inputMethodService.triggerServiceDump(TAG + "#getTextBeforeCursor", icProto);
imsInternal.triggerServiceDump(TAG + "#getTextBeforeCursor", icProto);
}
return result;
@@ -149,11 +171,11 @@ final class RemoteInputConnection implements InputConnection {
final CharSequence result = CompletableFutureUtil.getResultOrNull(
value, TAG, "getSelectedText()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetSelectedTextProto(flags,
result);
inputMethodService.triggerServiceDump(TAG + "#getSelectedText", icProto);
imsInternal.triggerServiceDump(TAG + "#getSelectedText", icProto);
}
return result;
@@ -187,11 +209,11 @@ final class RemoteInputConnection implements InputConnection {
final SurroundingText result = CompletableFutureUtil.getResultOrNull(
value, TAG, "getSurroundingText()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetSurroundingTextProto(
beforeLength, afterLength, flags, result);
inputMethodService.triggerServiceDump(TAG + "#getSurroundingText", icProto);
imsInternal.triggerServiceDump(TAG + "#getSurroundingText", icProto);
}
return result;
@@ -207,11 +229,11 @@ final class RemoteInputConnection implements InputConnection {
final int result = CompletableFutureUtil.getResultOrZero(
value, TAG, "getCursorCapsMode()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetCursorCapsModeProto(
reqModes, result);
inputMethodService.triggerServiceDump(TAG + "#getCursorCapsMode", icProto);
imsInternal.triggerServiceDump(TAG + "#getCursorCapsMode", icProto);
}
return result;
@@ -227,11 +249,11 @@ final class RemoteInputConnection implements InputConnection {
final ExtractedText result = CompletableFutureUtil.getResultOrNull(
value, TAG, "getExtractedText()", mCancellationGroup, MAX_WAIT_TIME_MILLIS);
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService != null && ImeTracing.getInstance().isEnabled()) {
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal != null && ImeTracing.getInstance().isEnabled()) {
final byte[] icProto = InputConnectionProtoDumper.buildGetExtractedTextProto(
request, flags, result);
inputMethodService.triggerServiceDump(TAG + "#getExtractedText", icProto);
imsInternal.triggerServiceDump(TAG + "#getExtractedText", icProto);
}
return result;
@@ -248,12 +270,11 @@ final class RemoteInputConnection implements InputConnection {
@AnyThread
private void notifyUserActionIfNecessary() {
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService == null) {
// This basically should not happen, because it's the the caller of this method.
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal == null) {
return;
}
inputMethodService.notifyUserActionIfNecessary();
imsInternal.notifyUserActionIfNecessary();
}
@AnyThread
@@ -400,12 +421,11 @@ final class RemoteInputConnection implements InputConnection {
}
if ((flags & InputConnection.INPUT_CONTENT_GRANT_READ_URI_PERMISSION) != 0) {
final InputMethodServiceInternal inputMethodService = mInputMethodService.get();
if (inputMethodService == null) {
// This basically should not happen, because it's the caller of this method.
final InputMethodServiceInternal imsInternal = mImsInternal.getAndWarnIfNull();
if (imsInternal == null) {
return false;
}
inputMethodService.exposeContent(inputContentInfo, this);
imsInternal.exposeContent(inputContentInfo, this);
}
final CompletableFuture<Boolean> value =