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

1 files changed, 48 insertions, 28 deletions

diff --git a/core/java/android/inputmethodservice/RemoteInputConnection.java b/core/java/android/inputmethodservice/RemoteInputConnection.java
index 464b421d970d..1065d041713f 100644
--- a/core/java/android/inputmethodservice/RemoteInputConnection.java
+++ b/core/java/android/inputmethodservice/RemoteInputConnection.java
@@ -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 =