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

Commit 0df41fe0 authored by Taran Singh's avatar Taran Singh
Browse files

Fix race between invalidateInput and restartInput

Since ZeroJankProxy, we allowed startInput to be asynchronous, which
allows two conflicting operations of #invalidateInput and #restartInput
to race with each other.
In order to fix the race, invalidateInput could fallback to calling
restartInput() if there is an ongoing restartInput.

Bug: 396066692
Test: atest InputConnectionLifecycleTest
Flag: android.view.inputmethod.invalidate_input_calls_restart

Change-Id: Id87a6165fa9eda6d328165e9a6ba88ca56bea610
parent c0fccde1
Loading
Loading
Loading
Loading
+33 −7
Original line number Diff line number Diff line
@@ -620,6 +620,12 @@ public final class InputMethodManager {
    @GuardedBy("mH")
    private CompletionInfo[] mCompletions;

    /**
     * Tracks last pending {@link #startInputInner(int, IBinder, int, int, int)} sequenceId.
     */
    @GuardedBy("mH")
    private int mLastPendingStartSeqId = INVALID_SEQ_ID;

    // Cursor position on the screen.
    @GuardedBy("mH")
    @UnsupportedAppUsage
@@ -652,6 +658,8 @@ public final class InputMethodManager {
    private static final String CACHE_KEY_CONNECTIONLESS_STYLUS_HANDWRITING_PROPERTY =
            "cache_key.system_server.connectionless_stylus_handwriting";

    static final int INVALID_SEQ_ID = -1;

    @GuardedBy("mH")
    private int mCursorSelStart;
    @GuardedBy("mH")
@@ -1193,12 +1201,18 @@ public final class InputMethodManager {
                case MSG_START_INPUT_RESULT: {
                    final InputBindResult res = (InputBindResult) msg.obj;
                    final int startInputSeq = msg.arg1;
                    synchronized (mH) {
                        if (mLastPendingStartSeqId == startInputSeq) {
                            // last pending startInput has been completed. reset.
                            mLastPendingStartSeqId = INVALID_SEQ_ID;
                        }

                        if (res == null) {
                            // IMMS logs .wtf already.
                            return;
                        }

                        if (DEBUG) Log.v(TAG, "Starting input: Bind result=" + res);
                    synchronized (mH) {
                        if (res.id != null) {
                            updateInputChannelLocked(res.channel);
                            mCurMethod = res.method; // for @UnsupportedAppUsage
@@ -2220,6 +2234,7 @@ public final class InputMethodManager {
            }
            mCompletions = null;
            mServedConnecting = false;
            mLastPendingStartSeqId = INVALID_SEQ_ID;
            clearConnectionLocked();
        }
        mReportInputConnectionOpenedRunner = null;
@@ -3274,6 +3289,9 @@ public final class InputMethodManager {
     * @param view The view whose text has changed.
     */
    public void restartInput(View view) {
        if (DEBUG) {
            Log.d(TAG, "restartInput()");
        }
        // Re-dispatch if there is a context mismatch.
        final InputMethodManager fallbackImm = getFallbackInputMethodManagerIfNecessary(view);
        if (fallbackImm != null) {
@@ -3351,6 +3369,9 @@ public final class InputMethodManager {
     */
    public void invalidateInput(@NonNull View view) {
        Objects.requireNonNull(view);
        if (DEBUG) {
            Log.d(TAG, "IMM#invaldateInput()");
        }

        // Re-dispatch if there is a context mismatch.
        final InputMethodManager fallbackImm = getFallbackInputMethodManagerIfNecessary(view);
@@ -3363,7 +3384,8 @@ public final class InputMethodManager {
            if (mServedInputConnection == null || getServedViewLocked() != view) {
                return;
            }
            mServedInputConnection.scheduleInvalidateInput();
            mServedInputConnection.scheduleInvalidateInput(
                    mLastPendingStartSeqId != INVALID_SEQ_ID);
        }
    }

@@ -3532,7 +3554,7 @@ public final class InputMethodManager {
                    ? editorInfo.targetInputMethodUser.getIdentifier() : UserHandle.myUserId();
            Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "IMM.startInputOrWindowGainedFocus");

            int startInputSeq = -1;
            int startInputSeq = INVALID_SEQ_ID;
            if (Flags.useZeroJankProxy()) {
                // async result delivered via MSG_START_INPUT_RESULT.
                startInputSeq = IInputMethodManagerGlobalInvoker.startInputOrWindowGainedFocusAsync(
@@ -3557,6 +3579,9 @@ public final class InputMethodManager {
                // initialized and ready for use.
                if (ic != null) {
                    final int seqId = startInputSeq;
                    if (Flags.invalidateInputCallsRestart()) {
                        mLastPendingStartSeqId = seqId;
                    }
                    mReportInputConnectionOpenedRunner =
                            new ReportInputConnectionOpenedRunner(startInputSeq) {
                                @Override
@@ -5047,6 +5072,7 @@ public final class InputMethodManager {
        }
        p.println("  mServedInputConnection=" + mServedInputConnection);
        p.println("  mServedInputConnectionHandler=" + mServedInputConnectionHandler);
        p.println("  mLastPendingStartSeqId=" + mLastPendingStartSeqId);
        p.println("  mCompletions=" + Arrays.toString(mCompletions));
        p.println("  mCursorRect=" + mCursorRect);
        p.println("  mCursorSelStart=" + mCursorSelStart
+22 −1
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@

package android.view.inputmethod;

import static android.view.inputmethod.InputMethodManager.INVALID_SEQ_ID;

import static com.android.internal.inputmethod.InputConnectionProtoDumper.buildGetCursorCapsModeProto;
import static com.android.internal.inputmethod.InputConnectionProtoDumper.buildGetExtractedTextProto;
import static com.android.internal.inputmethod.InputConnectionProtoDumper.buildGetSelectedTextProto;
@@ -276,8 +278,19 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
     * make sure that application code is not modifying text context in a reentrant manner.</p>
     */
    public void scheduleInvalidateInput() {
        scheduleInvalidateInput(false /* isRestarting */);
    }

    /**
     * @see #scheduleInvalidateInput()
     * @param isRestarting when {@code true}, there is an in-progress restartInput that could race
     *                    with {@link InputMethodManager#invalidateInput(View)}. To prevent race,
     *                    fallback to calling {@link InputMethodManager#restartInput(View)}.
     */
    void scheduleInvalidateInput(boolean isRestarting) {
        if (mHasPendingInvalidation.compareAndSet(false, true)) {
            final int nextSessionId = mCurrentSessionId.incrementAndGet();
            final int nextSessionId =
                    isRestarting ? INVALID_SEQ_ID : mCurrentSessionId.incrementAndGet();
            // By calling InputConnection#takeSnapshot() directly from the message loop, we can make
            // sure that application code is not modifying text context in a reentrant manner.
            // e.g. We may see methods like EditText#setText() in the callstack here.
@@ -330,6 +343,14 @@ final class RemoteInputConnectionImpl extends IRemoteInputConnection.Stub {
                            }
                        }
                    }
                    if (isRestarting) {
                        if (DEBUG) {
                            Log.d(TAG, "scheduleInvalidateInput called with ongoing restartInput."
                                    + " Fallback to calling restartInput().");
                        }
                        mParentInputMethodManager.restartInput(view);
                        return;
                    }

                    if (!alwaysTrueEndBatchEditDetected) {
                        final TextSnapshot textSnapshot = ic.takeSnapshot();