Introduce ThreadedInputConnection behind a switch

content/public/android/java/src/org/chromium/content/browser/input/ChromiumInputConnection.java

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.content.browser.input;
+
+import android.os.Bundle;
+import android.os.Handler;
+import android.os.Looper;
+import android.view.KeyEvent;
+import android.view.inputmethod.CompletionInfo;
+import android.view.inputmethod.CorrectionInfo;
+import android.view.inputmethod.EditorInfo;
+import android.view.inputmethod.ExtractedText;
+import android.view.inputmethod.ExtractedTextRequest;
+import android.view.inputmethod.InputConnection;
+
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * Chromium's implementation of {@link InputConnection} to communicate with external input method
+ * apps.
+ */
+public class ChromiumInputConnection implements ChromiumBaseInputConnection {
+    private static final String TAG = "cr_Ime";
+
+    static class ThreadManager implements ChromiumBaseInputConnection.ThreadManager {
+        private final Handler mHandler;
+
+        public ThreadManager(Handler handler) {
+            mHandler = handler;
+        }
+
+        public Handler getHandler() {
+            return mHandler;
+        }
+
+        @Override
+        public boolean runningOnThisThread() {
+            return getHandler().getLooper() == Looper.myLooper();
+        }
+
+        @Override
+        public void post(Runnable runnable) {
+            getHandler().post(runnable);
+        }
+    }
+
+    private final ImeAdapter mImeAdapter;
+    private final ThreadManager mThreadManager;
+
+    private int mNumNestedBatchEdits = 0;
+
+    private final BlockingQueue<TextInputState> mQueue = new LinkedBlockingQueue<>();
+    private TextInputState mBatchEditPendingState;
+    // Only accessible on UI thread.
+    private TextInputState mLastUpdatedTextInputState;
+
+    ChromiumInputConnection(ImeAdapter imeAdapter, ThreadManager threadManager) {
+        Log.d(TAG, "constructor");
+        ImeUtils.assertOnUiThread();
+        mImeAdapter = imeAdapter;
+        mThreadManager = threadManager;
+    }
+
+    void initializeOutAttrsOnUiThread(int inputType, int inputFlags, EditorInfo outAttrs) {
+        Log.d(TAG, "initialize");
+        ImeUtils.assertOnUiThread();
+        int initialSelStart = 0;
+        int initialSelEnd = 0;
+        if (mLastUpdatedTextInputState != null) {
+            initialSelStart = mLastUpdatedTextInputState.selection().start();
+            initialSelEnd = mLastUpdatedTextInputState.selection().end();
+        }
+        ImeUtils.computeEditorInfo(inputType, inputFlags, initialSelStart, initialSelEnd, outAttrs);
+    }
+
+    @Override
+    public void updateStateOnUiThread(final String text, final int selectionStart,
+            final int selectionEnd, final int compositionStart, final int compositionEnd,
+            boolean singleLine, final boolean isNonImeChange) {
+        Log.d(TAG, "updateState [%s] [%s %s] [%s %s] [%b] [%b]", text, selectionStart, selectionEnd,
+                compositionStart, compositionEnd, singleLine, isNonImeChange);
+        ImeUtils.assertOnUiThread();
+
+        final TextInputState newState =
+                new TextInputState(text, new Range(selectionStart, selectionEnd),
+                        new Range(compositionStart, compositionEnd), singleLine);
+        mLastUpdatedTextInputState = newState;
+
+        if (!isNonImeChange) {
+            addToQueueOnUiThread(newState);
+        }
+
+        mThreadManager.post(new Runnable() {

[aelias_OOO_until_Jul13, 2016/01/23 03:21:39]

By removing the "else" around this and removing the updateSelection() call in
blockAndGetStateUpdate(), you changed selection updates to happen asynchronously
after the public InputConnection methods return.  I guess this is point (2) you
mentioned fixing a finishComposingText spurious call, but I don't really
understand what the problem was.

Could you give more explanation about this?  It doesn't match
ReplicaInputConnection which calls updateSelectionIfRequired synchronously
before returning.  I also find this way harder to reason about.

[Changwan Ryu, 2016/01/25 09:07:27]

The problem was as follows:

1) IME-originating state updates go through mQueue, while non-IME-originating
state updates posts to the IME thread manager.
2) Depending on the timing, non-IME-originating state updates may call
updateSelection before IME-originating state updates calls it because they do
not go through mQueue.
3) In many cases non-IME-originating state updates are the ones that are
partially updated ones, so if they are sent later it's like going back in state
transition.
4) IME apps call finishComposingText() immediately when they see spurious calls
to updateSelection.

I'm trying a couple of ideas but couldn't find a clean solution yet.

[aelias_OOO_until_Jul13, 2016/01/26 00:33:05]

Got it.  Sorry about not considering that when I proposed the design change. 
How about this solution:

1. Add a new boolean fromNonIme to class TextInputState.
2. Push non_ime TextInputStates on the BlockingQueue as well, and post a task
"CheckQueueForNonImeUpdate" that checks the queue for such TextInputState update
and exits without doing anything if the queue is currently empty.
3. Change blockAndGetStateUpdate to ignore fromNonIme TextInputStates and keep
polling until it sees one from IME.

[Changwan Ryu, 2016/01/26 03:25:28]

Thanks, that's what I was considering, too. It was not working because of
mQueue.poll() which has been replaced by mQueue.take(). Now it's done. :)

+            @Override
+            public void run() {
+                updateSelection(newState);
+            }
+        });
+        Log.d(TAG, "updateState finished");
+    }
+
+    @Override
+    public ThreadManager getThreadManager() {
+        return mThreadManager;
+    }
+
+    @Override
+    public void onRestartInputOnUiThread() {}
+
+    private void updateSelection(TextInputState textInputState) {
+        assertOnImeThread();
+        if (mNumNestedBatchEdits != 0) {
+            mBatchEditPendingState = textInputState;
+            return;
+        }
+        ImeUtils.assertReally(textInputState != null);
+        Range selection = textInputState.selection();
+        Range composition = textInputState.composition();
+        mImeAdapter.updateSelection(
+                selection.start(), selection.end(), composition.start(), composition.end());
+    }
+
+    private TextInputState requestTextInputState() {
+        Log.d(TAG, "requestTextInputState");
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                boolean result = mImeAdapter.requestTextInputStateUpdate();
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate();
+    }
+
+    private void addToQueueOnUiThread(TextInputState textInputState) {
+        ImeUtils.assertOnUiThread();
+        try {
+            mQueue.put(textInputState);
+        } catch (InterruptedException e) {
+            Log.e(TAG, "onImeOriginatingStateUpdateOnUiThread interrupted", e);
+        }
+        Log.d(TAG, "onImeOriginatingStateUpdateOnUiThread finished: %d", mQueue.size());
+    }
+
+    /**
+     * Unblock the blockAndGetStateUpdate() function if we found that we will
+     * never get state update.
+     */
+    private void unblock() {
+        ImeUtils.assertOnUiThread();
+        addToQueueOnUiThread(new TextInputState.Unblocker());
+    }
+
+    private void assertOnImeThread() {
+        ImeUtils.assertReally(mThreadManager.runningOnThisThread());
+    }
+
+    /**
+     * Block until we get the expected state update.
+     * @return TextInputState if we get it successfully. null otherwise.
+     */
+    private TextInputState blockAndGetStateUpdate() {
+        assertOnImeThread();
+        while (true) {
+            TextInputState result = mQueue.poll();
+            if (result != null) {
+                if (result.shouldUnblock()) {
+                    Log.d(TAG, "blockAndGetStateUpdate: unblocked");
+                    return null;
+                }
+                Log.d(TAG, "blockAndGetStateUpdate finished: %s", result.toString());
+                return result;
+            }
+        }
+    }
+
+    /**
+     * @see InputConnection#setComposingText(java.lang.CharSequence, int)
+     */
+    @Override
+    public boolean setComposingText(final CharSequence text, final int newCursorPosition) {
+        Log.d(TAG, "setComposingText [%s] [%d]", text, newCursorPosition);
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.sendCompositionToNative(text, newCursorPosition, false);
+            }
+        });
+
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see InputConnection#commitText(java.lang.CharSequence, int)
+     */
+    @Override
+    public boolean commitText(final CharSequence text, final int newCursorPosition) {
+        Log.d(TAG, "commitText [%s] [%d]", text, newCursorPosition);
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.sendCompositionToNative(text, newCursorPosition, text.length() > 0);
+            }
+        });
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see InputConnection#performEditorAction(int)
+     */
+    @Override
+    public boolean performEditorAction(final int actionCode) {
+        Log.d(TAG, "performEditorAction [%d]", actionCode);
+        assertOnImeThread();
+        try {
+            return ThreadUtils.runOnUiThreadBlocking(new Callable<Boolean>() {
+                @Override
+                public Boolean call() {
+                    return mImeAdapter.performEditorAction(actionCode);
+                }
+            });
+        } catch (ExecutionException e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
+
+    /**
+     * @see InputConnection#performContextMenuAction(int)
+     */
+    @Override
+    public boolean performContextMenuAction(final int id) {
+        Log.d(TAG, "performContextMenuAction [%d]", id);
+        assertOnImeThread();
+        try {
+            return ThreadUtils.runOnUiThreadBlocking(new Callable<Boolean>() {
+                @Override
+                public Boolean call() {
+                    return mImeAdapter.performContextMenuAction(id);
+                }
+            });
+        } catch (ExecutionException e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
+
+    /**
+     * @see InputConnection#getExtractedText(android.view.inputmethod.ExtractedTextRequest,
+     *                                           int)
+     */
+    @Override
+    public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) {
+        Log.d(TAG, "getExtractedText");
+        assertOnImeThread();
+        TextInputState textInputState = requestTextInputState();
+        if (textInputState == null) return null;
+        ExtractedText extractedText = new ExtractedText();
+        extractedText.text = textInputState.text();
+        extractedText.partialEndOffset = textInputState.text().length();
+        extractedText.selectionStart = textInputState.selection().start();
+        extractedText.selectionEnd = textInputState.selection().end();
+        extractedText.flags = textInputState.singleLine() ? ExtractedText.FLAG_SINGLE_LINE : 0;
+        return extractedText;
+    }
+
+    /**
+     * @see InputConnection#beginBatchEdit()
+     */
+    @Override
+    public boolean beginBatchEdit() {
+        Log.d(TAG, "beginBatchEdit [%b]", (mNumNestedBatchEdits == 0));
+        assertOnImeThread();
+        mNumNestedBatchEdits++;
+        return true;
+    }
+
+    /**
+     * @see InputConnection#endBatchEdit()
+     */
+    @Override
+    public boolean endBatchEdit() {
+        assertOnImeThread();
+        if (mNumNestedBatchEdits == 0) return false;
+        --mNumNestedBatchEdits;
+        Log.d(TAG, "endBatchEdit [%b]", (mNumNestedBatchEdits == 0));
+        if (mNumNestedBatchEdits == 0) {
+            if (mBatchEditPendingState != null) {
+                // TODO(changwan): check if there is a chance that this gets called

[aelias_OOO_until_Jul13, 2016/01/23 03:21:39]

Yes, this looks like a race condition.  I propose calling
updateSelection(requestTextInputState()); here.  Then, you can even delete
mBatchEditPendingState, as there's no need to cache it.

[Changwan Ryu, 2016/01/26 03:25:28]

Good idea. Done.

+                // before updateSelection from a nested command.
+                updateSelection(mBatchEditPendingState);
+                mBatchEditPendingState = null;
+            }
+        }
+        return mNumNestedBatchEdits != 0;
+    }
+
+    /**
+     * @see InputConnection#deleteSurroundingText(int, int)
+     */
+    @Override
+    public boolean deleteSurroundingText(final int beforeLength, final int afterLength) {
+        Log.d(TAG, "deleteSurroundingText [%d %d]", beforeLength, afterLength);
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.deleteSurroundingText(beforeLength, afterLength);
+            }
+        });
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see ChromiumBaseInputConnection#sendKeyEventOnUiThread(KeyEvent)
+     */
+    @Override
+    public boolean sendKeyEventOnUiThread(final KeyEvent event) {
+        ImeUtils.assertOnUiThread();
+        mThreadManager.post(new Runnable() {
+            @Override
+            public void run() {
+                sendKeyEvent(event);
+            }
+        });
+        return true;
+    }
+
+    /**
+     * @see InputConnection#sendKeyEvent(android.view.KeyEvent)
+     */
+    @Override
+    public boolean sendKeyEvent(final KeyEvent event) {
+        Log.d(TAG, "sendKeyEvent [%d %d]", event.getAction(), event.getKeyCode());
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.sendKeyEvent(event);
+            }
+        });
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see InputConnection#finishComposingText()
+     */
+    @Override
+    public boolean finishComposingText() {
+        Log.d(TAG, "finishComposingText");
+        // This is the only function that may be called on UI thread because
+        // of direct calls from InputMethodManager.
+
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.finishComposingText();
+            }
+        });
+
+        if (ThreadUtils.runningOnUiThread()) {
+            Log.d(TAG, "finishComposingText was called on UI thread.");
+            return true;
+        } else {
+            return requestTextInputState() != null;
+        }
+    }
+
+    /**
+     * @see InputConnection#setSelection(int, int)
+     */
+    @Override
+    public boolean setSelection(final int start, final int end) {
+        Log.d(TAG, "setSelection [%d %d]", start, end);
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.setEditableSelectionOffsets(start, end);
+            }
+        });
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see InputConnection#setComposingRegion(int, int)
+     */
+    @Override
+    public boolean setComposingRegion(final int start, final int end) {
+        Log.d(TAG, "setComposingRegion [%d %d]", start, end);
+        assertOnImeThread();
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                mImeAdapter.setComposingRegion(start, end);
+            }
+        });
+        return requestTextInputState() != null;
+    }
+
+    /**
+     * @see InputConnection#getTextBeforeCursor(int, int)
+     */
+    @Override
+    public CharSequence getTextBeforeCursor(int maxChars, int flags) {
+        Log.d(TAG, "getTextBeforeCursor [%d %x]", maxChars, flags);
+        assertOnImeThread();
+        TextInputState textInputState = requestTextInputState();
+        if (textInputState == null) return null;
+        return textInputState.getTextBeforeSelection(maxChars);
+    }
+
+    /**
+     * @see InputConnection#getTextAfterCursor(int, int)
+     */
+    @Override
+    public CharSequence getTextAfterCursor(int maxChars, int flags) {
+        Log.d(TAG, "getTextAfterCursor [%d %x]", maxChars, flags);
+        assertOnImeThread();
+        TextInputState textInputState = requestTextInputState();
+        if (textInputState == null) return null;
+        return textInputState.getTextAfterSelection(maxChars);
+    }
+
+    /**
+     * @see InputConnection#getSelectedText(int)
+     */
+    @Override
+    public CharSequence getSelectedText(int flags) {
+        Log.d(TAG, "getSelectedText [%x]", flags);
+        assertOnImeThread();
+        TextInputState textInputState = requestTextInputState();
+        if (textInputState == null) return null;
+        return textInputState.getSelectedText();
+    }
+
+    @Override
+    public void moveCursorToSelectionEndOnUiThread() {
+        mThreadManager.post(new Runnable() {
+            @Override
+            public void run() {
+                TextInputState textInputState = requestTextInputState();
+                if (textInputState == null) return;
+                Range selection = textInputState.selection();
+                setSelection(selection.end(), selection.end());
+            }
+        });
+    }
+
+    /**
+     * @see InputConnection#getCursorCapsMode(int)
+     */
+    @Override
+    public int getCursorCapsMode(int reqModes) {
+        Log.d(TAG, "getCursorCapsMode [%x]", reqModes);
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return 0;
+    }
+
+    /**
+     * @see InputConnection#commitCompletion(android.view.inputmethod.CompletionInfo)
+     */
+    @Override
+    public boolean commitCompletion(CompletionInfo text) {
+        Log.d(TAG, "commitCompletion [%s]", text);
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return false;
+    }
+
+    /**
+     * @see InputConnection#commitCorrection(android.view.inputmethod.CorrectionInfo)
+     */
+    @Override
+    public boolean commitCorrection(CorrectionInfo correctionInfo) {
+        Log.d(TAG, "commitCorrection [%s]", ImeUtils.dumpCorrectionInfo(correctionInfo));
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return false;
+    }
+
+    /**
+     * @see InputConnection#clearMetaKeyStates(int)
+     */
+    @Override
+    public boolean clearMetaKeyStates(int states) {
+        Log.d(TAG, "clearMetaKeyStates [%x]", states);
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return false;
+    }
+
+    /**
+     * @see InputConnection#reportFullscreenMode(boolean)
+     */
+    @Override
+    public boolean reportFullscreenMode(boolean enabled) {
+        Log.d(TAG, "reportFullscreenMode [%b]", enabled);
+        // We ignore fullscreen mode for now. That's why we set
+        // EditorInfo.IME_FLAG_NO_FULLSCREEN in constructor.
+        // Note that this may be called on UI thread.
+        return false;
+    }
+
+    /**
+     * @see InputConnection#performPrivateCommand(java.lang.String, android.os.Bundle)
+     */
+    @Override
+    public boolean performPrivateCommand(String action, Bundle data) {
+        Log.d(TAG, "performPrivateCommand [%s]", action);
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return false;
+    }
+
+    /**
+     * @see InputConnection#requestCursorUpdates(int)
+     */
+    @Override
+    public boolean requestCursorUpdates(int cursorUpdateMode) {
+        Log.d(TAG, "requestCursorUpdates [%x]", cursorUpdateMode);
+        assertOnImeThread();
+        // TODO(changwan): Auto-generated method stub
+        return false;
+    }
+}