Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 60c7c55c authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Detect broken IC#endBatchEdit() impl at runtime

This is a safeguard against cases like Bug 203086369, where the UI
thread can be locked up if IC#endBatchEdit() never returns false no
matter how many times it gets called.

With this CL, the system will have a max retry count on how many times
it attempts to call IC#endBatchEdit(), which is currently set to 16,
then fall back to InputMethodManager#restartInput(View) if it hits
such a limit.

This CL also introduces a volatile type cache to early fall back to
the fallback implementation if the same type of InputConnection
implementation class appears again.

As long as InputConnection#{begin,end}BatchEdit() is correctly
implemented, there should be no developer observable behavior change
in this CL.

See the corresponding test CL [1] about when the fallback
implementation is used.

 [1]: Ifb80015ab0f0c32c917a80c4e5f60e836648e7b4

Bug: 203086369
Bug: 208941904
Bug: 209008342
Test: atest CtsInputMethodTestCases:InputMethodStartInputLifecycleTest
Change-Id: I109e0c26d8249fc2e01323e3e1cb36395fa7cc97
parent 5270fe67
Loading
Loading
Loading
Loading
+121 −13
Original line number Original line Diff line number Diff line
@@ -25,6 +25,7 @@ import static com.android.internal.inputmethod.InputConnectionProtoDumper.buildG


import static java.lang.annotation.RetentionPolicy.SOURCE;
import static java.lang.annotation.RetentionPolicy.SOURCE;


import android.annotation.AnyThread;
import android.annotation.NonNull;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.Nullable;
import android.os.Bundle;
import android.os.Bundle;
@@ -69,6 +70,82 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub {
    private static final String TAG = "RemoteInputConnectionImpl";
    private static final String TAG = "RemoteInputConnectionImpl";
    private static final boolean DEBUG = false;
    private static final boolean DEBUG = false;


    /**
     * An upper limit of calling {@link InputConnection#endBatchEdit()}.
     *
     * <p>This is a safeguard against broken {@link InputConnection#endBatchEdit()} implementations,
     * which are real as we've seen in Bug 208941904.  If the retry count reaches to the number
     * defined here, we fall back into {@link InputMethodManager#restartInput(View)} as a
     * workaround.</p>
     */
    private static final int MAX_END_BATCH_EDIT_RETRY = 16;

    /**
     * A lightweight per-process type cache to remember classes that never returns {@code false}
     * from {@link InputConnection#endBatchEdit()}.  The implementation is optimized for simplicity
     * and speed with accepting false-negatives in {@link #contains(Class)}.
     */
    private static final class KnownAlwaysTrueEndBatchEditCache {
        @Nullable
        private static volatile Class<?> sElement;
        @Nullable
        private static volatile Class<?>[] sArray;

        /**
         * Query if the specified {@link InputConnection} implementation is known to be broken, with
         * allowing false-negative results.
         *
         * @param klass An implementation class of {@link InputConnection} to be tested.
         * @return {@code true} if the specified type was passed to {@link #add(Class)}.
         *         Note that there is a chance that you still receive {@code false} even if you
         *         called {@link #add(Class)} (false-negative).
         */
        @AnyThread
        static boolean contains(@NonNull Class<? extends InputConnection> klass) {
            if (klass == sElement) {
                return true;
            }
            final Class<?>[] array = sArray;
            if (array == null) {
                return false;
            }
            for (Class<?> item : array) {
                if (item == klass) {
                    return true;
                }
            }
            return false;
        }

        /**
         * Try to remember the specified {@link InputConnection} implementation as a known bad.
         *
         * <p>There is a chance that calling this method can accidentally overwrite existing
         * cache entries. See the document of {@link #contains(Class)} for details.</p>
         *
         * @param klass The implementation class of {@link InputConnection} to be remembered.
         */
        @AnyThread
        static void add(@NonNull Class<? extends InputConnection> klass) {
            if (sElement == null) {
                // OK to accidentally overwrite an existing element that was set by another thread.
                sElement = klass;
                return;
            }

            final Class<?>[] array = sArray;
            final int arraySize = array != null ? array.length : 0;
            final Class<?>[] newArray = new Class<?>[arraySize + 1];
            for (int i = 0; i < arraySize; ++i) {
                newArray[i] = array[i];
            }
            newArray[arraySize] = klass;

            // OK to accidentally overwrite an existing array that was set by another thread.
            sArray = newArray;
        }
    }

    @Retention(SOURCE)
    @Retention(SOURCE)
    private @interface Dispatching {
    private @interface Dispatching {
        boolean cancellable();
        boolean cancellable();
@@ -155,32 +232,63 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub {
            mH.post(() -> {
            mH.post(() -> {
                try {
                try {
                    if (isFinished()) {
                    if (isFinished()) {
                        // This is a stale request, which can happen.  No need to show a warning
                        // because this situation itself is not an error.
                        return;
                        return;
                    }
                    }
                    final InputConnection ic = getInputConnection();
                    final InputConnection ic = getInputConnection();
                    if (ic == null) {
                    if (ic == null) {
                        // This is a stale request, which can happen.  No need to show a warning
                        // because this situation itself is not an error.
                        return;
                    }
                    final View view = getServedView();
                    if (view == null) {
                        // This is a stale request, which can happen.  No need to show a warning
                        // because this situation itself is not an error.
                        return;
                        return;
                    }
                    }


                    final Class<? extends InputConnection> icClass = ic.getClass();

                    boolean alwaysTrueEndBatchEditDetected =
                            KnownAlwaysTrueEndBatchEditCache.contains(icClass);

                    if (!alwaysTrueEndBatchEditDetected) {
                        // Clean up composing text and batch edit.
                        // Clean up composing text and batch edit.
                        final boolean supportsBatchEdit = ic.beginBatchEdit();
                        ic.finishComposingText();
                        ic.finishComposingText();
                        if (supportsBatchEdit) {
                            // Also clean up batch edit.
                            // Also clean up batch edit.
                            int retryCount = 0;
                            while (true) {
                            while (true) {
                                if (!ic.endBatchEdit()) {
                                if (!ic.endBatchEdit()) {
                                    break;
                                    break;
                                }
                                }
                                ++retryCount;
                                if (retryCount > MAX_END_BATCH_EDIT_RETRY) {
                                    Log.e(TAG, icClass.getTypeName() + "#endBatchEdit() still"
                                            + " returns true even after retrying "
                                            + MAX_END_BATCH_EDIT_RETRY + " times.  Falling back to"
                                            + " InputMethodManager#restartInput(View)");
                                    alwaysTrueEndBatchEditDetected = true;
                                    KnownAlwaysTrueEndBatchEditCache.add(icClass);
                                    break;
                                }
                            }
                        }
                    }
                    }


                    if (!alwaysTrueEndBatchEditDetected) {
                        final TextSnapshot textSnapshot = ic.takeSnapshot();
                        final TextSnapshot textSnapshot = ic.takeSnapshot();
                    if (textSnapshot == null) {
                        if (textSnapshot != null) {
                        final View view = getServedView();
                            mParentInputMethodManager.doInvalidateInput(this, textSnapshot,
                        if (view == null) {
                                    nextSessionId);
                            return;
                            return;
                        }
                        }
                        mParentInputMethodManager.restartInput(view);
                        return;
                    }
                    }
                    mParentInputMethodManager.doInvalidateInput(this, textSnapshot, nextSessionId);

                    mParentInputMethodManager.restartInput(view);
                } finally {
                } finally {
                    mHasPendingInvalidation.set(false);
                    mHasPendingInvalidation.set(false);
                }
                }