From 8821afef81ad0e09d77805dadea5b03725785bca Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 16 Sep 2021 16:17:09 -0700 Subject: [PATCH] Preserve invocation order in RemoteInputConnection This is a follow up CL to our previous CL [1], which introduced early-exit paths to some RemoteInputConnection methods to protect innocent IME clients from unexpected process crashes when an IME is calling InputConnection APIs with invalid parameters. Although protecting IME clients from crashes still makes much sense, implementing it as an early-exit style in RemoteInputConnection may expose observable inconsistency to IME developers in terms of the fact that InputConnection#getText{Before,After}Cursor() can also work as a fence operation that would not return until all the previously issued InputConnection API invocations are handled in the IME client side. With this CL, the following methods start behaving as a fence operation even when an invalid parameter is passed, by checking the parameters in the IME client side. * RemoteInputConnection#getTextAfterCursor() * RemoteInputConnection#getTextBeforeCursor() * RemoteInputConnection#getSurroundingText() There should be no performance impact for IMEs that do not make such an invalid (and unnecessary) API calls. [1]: I95169735198f8363c981a61e20234dfebfd645b1 1e72ef28933823594bf2b1993fed2d4895b20a67 Bug: 169114026 Fix: 194110780 Test: atest CtsInputMethodTestCases:InputConnectionEndToEndTest Change-Id: Ie0c18d0c9b8bf8f02f2fcdca5aac7e580c6bf2cd --- .../RemoteInputConnection.java | 24 ++++++++++++++++--- .../RemoteInputConnectionImpl.java | 20 ++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/core/java/android/inputmethodservice/RemoteInputConnection.java b/core/java/android/inputmethodservice/RemoteInputConnection.java index 657f8428179f0..ae97fe7df7293 100644 --- a/core/java/android/inputmethodservice/RemoteInputConnection.java +++ b/core/java/android/inputmethodservice/RemoteInputConnection.java @@ -108,7 +108,11 @@ final class RemoteInputConnection implements InputConnection { @Nullable @AnyThread public CharSequence getTextAfterCursor(@IntRange(from = 0) int length, int flags) { - if (length < 0 || mCancellationGroup.isCanceled()) { + if (length < 0) { + // TODO: Should we throw an InvalidParameterException() based on targetSdkVersion? + Log.e(TAG, "length=" + length + " is invalid and always results in null result."); + } + if (mCancellationGroup.isCanceled()) { return null; } @@ -132,7 +136,11 @@ final class RemoteInputConnection implements InputConnection { @Nullable @AnyThread public CharSequence getTextBeforeCursor(@IntRange(from = 0) int length, int flags) { - if (length < 0 || mCancellationGroup.isCanceled()) { + if (length < 0) { + // TODO: Should we throw an InvalidParameterException() based on targetSdkVersion? + Log.e(TAG, "length=" + length + " is invalid and always results in null result."); + } + if (mCancellationGroup.isCanceled()) { return null; } @@ -185,7 +193,17 @@ final class RemoteInputConnection implements InputConnection { @AnyThread public SurroundingText getSurroundingText( @IntRange(from = 0) int beforeLength, @IntRange(from = 0) int afterLength, int flags) { - if (beforeLength < 0 || afterLength < 0 || mCancellationGroup.isCanceled()) { + if (beforeLength < 0) { + // TODO: Should we throw an InvalidParameterException() based on targetSdkVersion? + Log.e(TAG, "beforeLength=" + beforeLength + + " is invalid and always results in null result."); + } + if (afterLength < 0) { + // TODO: Should we throw an InvalidParameterException() based on targetSdkVersion? + Log.e(TAG, "afterLength=" + afterLength + + " is invalid and always results in null result."); + } + if (mCancellationGroup.isCanceled()) { return null; } diff --git a/core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java b/core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java index 29c1b1b2b604e..7a668fb74cb76 100644 --- a/core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java +++ b/core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java @@ -233,6 +233,11 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub { Log.w(TAG, "getTextAfterCursor on inactive InputConnection"); return null; } + if (length < 0) { + Log.i(TAG, "Returning null to getTextAfterCursor due to an invalid length=" + + length); + return null; + } return ic.getTextAfterCursor(length, flags); }, useImeTracing() ? result -> buildGetTextAfterCursorProto(length, flags, result) : null); } @@ -246,6 +251,11 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub { Log.w(TAG, "getTextBeforeCursor on inactive InputConnection"); return null; } + if (length < 0) { + Log.i(TAG, "Returning null to getTextBeforeCursor due to an invalid length=" + + length); + return null; + } return ic.getTextBeforeCursor(length, flags); }, useImeTracing() ? result -> buildGetTextBeforeCursorProto(length, flags, result) : null); } @@ -276,6 +286,16 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub { Log.w(TAG, "getSurroundingText on inactive InputConnection"); return null; } + if (beforeLength < 0) { + Log.i(TAG, "Returning null to getSurroundingText due to an invalid" + + " beforeLength=" + beforeLength); + return null; + } + if (afterLength < 0) { + Log.i(TAG, "Returning null to getSurroundingText due to an invalid" + + " afterLength=" + afterLength); + return null; + } return ic.getSurroundingText(beforeLength, afterLength, flags); }, useImeTracing() ? result -> buildGetSurroundingTextProto( beforeLength, afterLength, flags, result) : null);