Introduce ThreadedInputConnection behind a switch

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

Index: content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
new file mode 100644
index 0000000000000000000000000000000000000000..fe212178473c5947517c6eadc9f448f19f0c6b00
--- /dev/null
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
@@ -0,0 +1,151 @@
+// 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.Handler;
+import android.os.HandlerThread;
+import android.view.View;
+import android.view.inputmethod.EditorInfo;
+
+import org.chromium.base.CommandLine;
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.metrics.RecordHistogram;
+import org.chromium.content.common.ContentSwitches;
+
+/**
+ * A factory class for {@link ThreadedInputConnection}. The class also includes triggering
+ * mechanism (hack) to run our InputConnection on non-UI thread.
+ */
+public class ThreadedInputConnectionFactory implements ChromiumBaseInputConnection.Factory {
+    private static final String TAG = "cr_Ime";
+    private static final boolean DEBUG_LOGS = true;

[Ted C, 2016/02/19 18:26:17]

don't forget to set this to false before committing

[Changwan Ryu, 2016/02/22 06:28:24]

Done.

+
+    private final Handler mHandler;
+    private final InputMethodManagerWrapper mInputMethodManagerWrapper;
+    private final ImeAdapter mImeAdapter;
+    private ThreadedInputConnectionProxyView mProxyView;
+    private ThreadedInputConnection mThreadedInputConnection;
+
+    ThreadedInputConnectionFactory(
+            InputMethodManagerWrapper inputMethodManagerWrapper, ImeAdapter imeAdapter) {
+        mImeAdapter = imeAdapter;
+        HandlerThread thread =
+                new HandlerThread("InputConnectionHandlerThread", HandlerThread.NORM_PRIORITY);
+        thread.start();
+        mHandler = new Handler(thread.getLooper());
+        mInputMethodManagerWrapper = inputMethodManagerWrapper;
+    }
+
+    private boolean shouldTriggerDelayedOnCreateInputConnection() {
+        for (StackTraceElement ste : Thread.currentThread().getStackTrace()) {
+            String className = ste.getClassName();
+            if (className != null
+                    && (className.contains(ThreadedInputConnectionProxyView.class.getName())
+                    || className.contains("TestInputMethodManagerWrapper"))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    @Override
+    public ChromiumBaseInputConnection initializeAndGet(
+            View view, ImeAdapter imeAdapter, int inputType, int inputFlags, int selectionStart,
+            int selectionEnd, EditorInfo outAttrs) {
+        ImeUtils.checkOnUiThread();
+        if (shouldTriggerDelayedOnCreateInputConnection()) {
+            triggerDelayedOnCreateInputConnection(view);
+            return null;
+        }
+        if (DEBUG_LOGS) Log.w(TAG, "initializeAndGet: called from proxy view");
+        if (mThreadedInputConnection == null) {
+            if (DEBUG_LOGS) Log.w(TAG, "Creating ThreadedInputConnection...");
+            mThreadedInputConnection = new ThreadedInputConnection(imeAdapter, mHandler);
+        }
+        mThreadedInputConnection.initializeOutAttrsOnUiThread(inputType, inputFlags,
+                selectionStart, selectionEnd, outAttrs);
+        return mThreadedInputConnection;
+    }
+
+    private void triggerDelayedOnCreateInputConnection(final View view) {
+        if (DEBUG_LOGS) Log.w(TAG, "triggerDelayedOnCreateInputConnection");
+        if (mProxyView == null) {
+            mProxyView = new ThreadedInputConnectionProxyView(view.getContext(), mHandler, view);
+        }
+        mProxyView.requestFocus();
+        view.getHandler().post(new Runnable() {
+            @Override
+            public void run() {
+                // This is a hack to make InputMethodManager believe that the proxy view
+                // now has a focus. As a result, InputMethodManager will think that mProxyView
+                // is focused, and will call getHandler() of the view when creating input
+                // connection.
+
+                // Step 1: Set mProxyView as InputMethodManager#mNextServedView.
+                // mProxyView.onWindowFocusChanged(true);
+
+                // Step 2: Have InputMethodManager focus in on mNextServedView.
+                // As a result, IMM will call onCreateInputConnection() on mProxyView on the same
+                // thread as mProxyView.getHandler(). It will also call subsequent InputConnection
+                // methods on this IME thread.
+                mInputMethodManagerWrapper.isActive(view);
+
+                // Step 3: Check that the above hack worked.
+                mHandler.post(new Runnable() {
+                    @Override
+                    public void run() {
+                        // Some other view already took focus. Container view should be active
+                        // otherwise regardless of whether proxy view is registered or not.
+                        if (!mInputMethodManagerWrapper.isActive(view)) return;
+
+                        // Success.
+                        if (mInputMethodManagerWrapper.isActive(mProxyView)) {
+                            RecordHistogram.recordEnumeratedHistogram(
+                                    InputMethodUma.UMA_REGISTER_PROXYVIEW,
+                                    InputMethodUma.UMA_PROXYVIEW_SUCCESS,
+                                    InputMethodUma.UMA_PROXYVIEW_COUNT);
+                            return;
+                        }
+
+                        if (mThreadedInputConnection == null) {
+                            // First time and failed. It is highly likely that this does not work
+                            // systematically.
+                            onProxyViewFailedToRegisterOnFirstTry(view);
+                        } else {
+                            // Most likely that we already lost view focus.
+                            RecordHistogram.recordEnumeratedHistogram(
+                                    InputMethodUma.UMA_REGISTER_PROXYVIEW,
+                                    InputMethodUma.UMA_PROXYVIEW_DETECTION_FAILURE,
+                                    InputMethodUma.UMA_PROXYVIEW_COUNT);
+                        }
+                    }
+                });
+            }
+        });
+    }
+
+    private void onProxyViewFailedToRegisterOnFirstTry(final View view) {

[Ted C, 2016/02/19 18:26:17]

would it be possible add a test for this (i.e. the fallback works)?

[Changwan Ryu, 2016/02/22 06:28:24]

Hmm.. We're not using real InputMethodManager in tests because we cannot bind
our test input method service just for testing. (i.e. change system keyboard
from default to ours).

Instead, I've written some robolectric tests verify that fallback gets called in
one scenario.
However, it seems that SDK level > 18 is not supported yet
on robolectric. I've left a TODO and filed crbug for it instead.
https://codereview.chromium.org/1721613002/ has the code if you're interested.

+        Log.w(TAG, "Failed to register proxy view. Falling back to ReplicaInputConnection...");
+        ThreadUtils.postOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                RecordHistogram.recordEnumeratedHistogram(InputMethodUma.UMA_REGISTER_PROXYVIEW,
+                        InputMethodUma.UMA_PROXYVIEW_FAILURE, InputMethodUma.UMA_PROXYVIEW_COUNT);
+                // Disable IME thread until Chrome gets closed. Note that this is not a permanent
+                // change.

[Ted C, 2016/02/19 18:26:18]

what about it isn't permanent?  are you clearing the switch elsewhere?

Oooohhh...are you saying that disabling in general is not permanent as we are
only doing this to test our workaround above works?  i.e. we'd delete this if
the rollout sees no code hit this?

[Changwan Ryu, 2016/02/19 22:17:11]

I just wanted to comment that switch enabled by appendswitch onlt lives
for the current run. (And it does not get committed to the
global store.)

+                CommandLine.getInstance().appendSwitch(ContentSwitches.DISABLE_IME_THREAD);
+                mImeAdapter.resetInputConnectionFactory();
+                mInputMethodManagerWrapper.restartInput(view);
+            }
+
+        });
+    }
+
+    @Override
+    public Handler getHandler() {
+        return mHandler;
+    }
+}