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 mUpdatePendingTextInputState;

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

Please rename to mBatchEditPendingState.

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+    // Only accessible on UI thread.
+    private TextInputState mLastUpdatedTextInputState;
+
+    ChromiumInputConnection(ImeAdapter imeAdapter, ThreadManager threadManager) {
+        Log.d(TAG, "constructor");
+        ImeUtils.assertOnUiThread();
+        mImeAdapter = imeAdapter;
+        mThreadManager = threadManager;
+    }
+
+    void initializeOnUiThread(int inputType, int inputFlags, EditorInfo outAttrs) {

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

Please rename to initializeOutAttrsOnUiThread.

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+        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();
+
+        // TODO(changwan): this should be called at the end of each call, not after it.
+        final TextInputState newState =
+                new TextInputState(text, new Range(selectionStart, selectionEnd),
+                        new Range(compositionStart, compositionEnd), singleLine);
+        mLastUpdatedTextInputState = newState;
+
+        if (!isNonImeChange) {
+            onImeOriginatingStateUpdateOnUiThread(newState);
+        } else {
+            mThreadManager.post(new Runnable() {
+                @Override
+                public void run() {
+                    updateToInputMethodManager(newState);
+                }
+            });
+        }
+        Log.d(TAG, "updateState finished");
+    }
+
+    @Override
+    public ThreadManager getThreadManager() {
+        return mThreadManager;
+    }
+
+    @Override
+    public void onRestartInputOnUiThread() {}
+
+    private void updateToInputMethodManager(TextInputState textInputState) {
+        assertOnImeThread();
+        if (mNumNestedBatchEdits != 0) {
+            mUpdatePendingTextInputState = textInputState;
+            return;
+        }
+        ImeUtils.assertReally(textInputState != null);
+        Range selection = textInputState.selection();
+        Range composition = textInputState.composition();
+        mImeAdapter.updateSelection(

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

This call back into ImeAdapter from deep in the IME thread looks weird.  I
suggest passing in InputMethodManagerWrapper in the ChromiumInputConnection
constructor and using that here instead.

[Changwan Ryu, 2016/01/22 10:22:16]

Hmm... I've done this refactoring with AdapterInputConnection such that we don't
have to keep View in AdapterInputConnection. If that's reverted, we need to keep
mView here which you probably don't want.

+                selection.start(), selection.end(), composition.start(), composition.end());
+    }
+
+    private void updatePendingTextInputState() {

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

As I mentioned in endBatchEdit(), this can be deleted.

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+        if (mUpdatePendingTextInputState == null) return;
+        Range selection = mUpdatePendingTextInputState.selection();
+        Range composition = mUpdatePendingTextInputState.composition();
+        mImeAdapter.updateSelection(
+                selection.start(), selection.end(), composition.start(), composition.end());
+        mUpdatePendingTextInputState = null;
+    }
+
+    private TextInputState updateTextInputState() {

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

Please rename to requestTextInputState().

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                boolean result = mImeAdapter.requestTextInputStateUpdate();
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate();
+    }
+
+    private void onImeOriginatingStateUpdateOnUiThread(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();
+        onImeOriginatingStateUpdateOnUiThread(new TextInputState.Unblocker());
+    }
+
+    private void postConsumeStateUpdate() {
+        mThreadManager.post(new Runnable() {
+            @Override
+            public void run() {
+                blockAndGetStateUpdate();
+            }
+        });
+    }
+
+    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;
+                }
+                updateToInputMethodManager(result);
+                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() {
+                boolean result = mImeAdapter.setComposingText(text, newCursorPosition);
+                if (!result) unblock();

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

If you apply my comment in render_widget.cpp, than all the unblock() calls
except the one in updateTextInputState() can be removed.

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+            }
+        });
+        return blockAndGetStateUpdate() != null;

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

If you apply my comment in render_widget.cpp, than all calls like this should be
replaced by calls to updateTextInputState().

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+    }
+
+    /**
+     * @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);
+        // TODO(changwan): for some unknown reason, setComposingText() -> commitText()
+        // does not delete the original composition. Consider using commitText() after fixing it.
+        beginBatchEdit();
+        boolean result = setComposingText(text, newCursorPosition) && finishComposingText();
+        endBatchEdit();
+        return result;
+    }
+
+    /**
+     * @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 = updateTextInputState();
+        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) {
+            updatePendingTextInputState();

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

Please delete that method and just do:

updateToInputMethodManager(mPendingTextInputState);
mPendingTextInputState = null;

[Changwan Ryu, 2016/01/22 10:22:16]

Done.

+        }
+        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() {
+                boolean result = mImeAdapter.deleteSurroundingText(beforeLength, afterLength);
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate() != 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() {
+                boolean result = mImeAdapter.sendKeyEvent(event);
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate() != 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() {
+                boolean result = mImeAdapter.finishComposingText();
+                if (!result) unblock();
+            }
+        });
+
+        if (ThreadUtils.runningOnUiThread()) {
+            postConsumeStateUpdate();

[aelias_OOO_until_Jul13, 2016/01/21 07:43:04]

If you apply my comment in render_widget.cpp, than this can be deleted because
the renderer would not issue a FROM_IME state update for finishComposingText on
its own.

[Changwan Ryu, 2016/01/22 10:22:16]

Removed.

+            return true;
+        } else {
+            return blockAndGetStateUpdate() != 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() {
+                boolean result = mImeAdapter.setEditableSelectionOffsets(start, end);
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate() != 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() {
+                boolean result = mImeAdapter.setComposingRegion(start, end);
+                if (!result) unblock();
+            }
+        });
+        return blockAndGetStateUpdate() != 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 = updateTextInputState();
+        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 = updateTextInputState();
+        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 = updateTextInputState();
+        if (textInputState == null) return null;
+        return textInputState.getSelectedText();
+    }
+
+    @Override
+    public void moveCursorToSelectionEndOnUiThread() {
+        mThreadManager.post(new Runnable() {
+            @Override
+            public void run() {
+                TextInputState textInputState = updateTextInputState();
+                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);
+        assertOnImeThread();
+        // We ignore fullscreen mode for now. That's why we set
+        // EditorInfo.IME_FLAG_NO_FULLSCREEN in constructor.
+        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;
+    }
+}