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..f97411a2764a610d490afc45579ac93571a97b8c
--- /dev/null
+++ b/content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.java
@@ -0,0 +1,76 @@
+// 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.Log;
+import org.chromium.base.ThreadUtils;
+import org.chromium.content.browser.input.ChromiumBaseInputConnection.ThreadManager;
+
+/**
+ * Default factory for ChromiumBaseInputConnection classes.
+ */
+public class ThreadedInputConnectionFactory implements ChromiumBaseInputConnection.Factory {
+    private static final String TAG = "cr_Ime";
+    private static final boolean DEBUG_LOGS = false;
+
+    private ThreadedInputConnection mThreadedInputConnection;
+    private final Handler mHandler;
+    private final ThreadedInputConnection.ThreadManager mThreadManager;
+    private final InputMethodManagerWrapper mInputMethodManagerWrapper;
+    private ThreadedInputConnectionProxyView mProxyView;
+
+    ThreadedInputConnectionFactory(InputMethodManagerWrapper inputMethodManagerWrapper) {
+        HandlerThread thread =
+                new HandlerThread("InputConnectionHandlerThread", HandlerThread.NORM_PRIORITY);
+        thread.start();
+        mHandler = new Handler(thread.getLooper());
+        mThreadManager = new ThreadManager(mHandler);
+        mInputMethodManagerWrapper = inputMethodManagerWrapper;
+    }
+
+    @Override
+    public ChromiumBaseInputConnection initializeAndGet(View view, ImeAdapter imeAdapter,
+            int inputType, int inputFlags, EditorInfo outAttrs) {
+        if (ThreadUtils.runningOnUiThread()) {

[aelias_OOO_until_Jul13, 2016/02/08 23:17:14]

Is there a way to avoid this thread-based branching?  If I understand correctly,
InputMethodManager would always call real View on the UI thread and call the
proxy View on the IME thread.  Is that assumption correct?  If so, I'd prefer
the proxy View call a new private method called like
"IMEThreadCreateInputConnection()" on creation instead of
onCreateInputConnection.  Then the real View's onCreateInputConnection should
assertOnUiThread() instead.

[Changwan Ryu, 2016/02/11 16:21:08]

Some WebView users (app developers) may override onCreateInputConnection. If we
call 
a separate function, I'm afraid that the overridden version may not be called.

BTW, I just realized that onCreateInputConnection won't be called on UI thread.
Is this going to be ok?

[aelias_OOO_until_Jul13, 2016/02/11 23:13:15]

Subclassing WebView is very rare in the first place, and I suspect there is no
existing app that overrides onCreateInputConnection specifically.  (We don't
have a way to collect metrics on this kind of thing, unfortunately.).  If such
an app *does* exist, it is likely doing something so weird that this patch is
liable to break it anyway.  So let's assume such an app doesn't exist for now
and only think about it if we find one.

As a principle, I'm opposed to branching based on which thread we're on -- that
often blocks some rearchitecture or testing approach later.  Please rewrite it
to assert which thread the calls are on if possible.

[Changwan Ryu, 2016/02/12 00:36:03]

Unfortunately, there are apps that override webview and even
onCreateInputConnection.
https://crbug.com/584426 and https://crbug.com/571229 are such examples.

The current approach does not override WebView's methods in a weird way, so I
think most webview subclassing apps should still work.

Actually I've changed my implementation such that View methods can all be run on
UI thread.
Since we cannot branch on threads any more, I've changed the code to branch on
the call stack, instead.
I could not yet find any better / easier way to distinguish the initial
onCreateInputConnection() and the one called from proxyview.

+            if (DEBUG_LOGS) Log.w(TAG, "initializeAndGet: running on ui thread");
+            if (mThreadedInputConnection != null) {
+                mThreadedInputConnection.finishComposingTextOnUiThread();
+            }
+            triggerDelayedOnCreateInputConnection(view);
+            return null;

[aelias_OOO_until_Jul13, 2016/02/08 23:17:14]

What if instead of returning a null InputConnection, we returned a stub fake
InputConnection attached to nothing? (If we find there is a need, we could
record public method calls there and replay them on the real one later, which is
somewhat analogous to the task pumping in your original reflection approach.) 
Would there be any additional difficulty transitioning to the thread if we did
that?

Reasoning: my mental model is that it's correct to return null from
onCreateInputConnection if and only if no textbox is focused and we intend to
hide the keyboard.  (You alluded to "K and below flickering" in the design doc
-- is my mental model somehow inaccurate for L and up?)  If some non-WebView
textbox is currently focused, a likely outcome of this line is that the keyboard
will briefly hide and come back.

[Changwan Ryu, 2016/02/11 16:21:08]

That's something we could consider if this doesn't work well.
When experimented on many different keyboards, OSK still shows up when I return
null input connection and call showSoftInput, so I think it's probably safe, but
I could be wrong.

[aelias_OOO_until_Jul13, 2016/02/11 23:13:15]

OK, if immediately calling showSoftInput always prevents the hide, we can give
it a try.   But why is that, is it that the keyboard hide is on a delayed timer
and showSoftInput wins a race?  Is there a way to make sure the keyboard is not
hidden even if there are unusual delays in tasks waking up?

[aelias_OOO_until_Jul13, 2016/02/11 23:13:15]

OK, if immediately calling showSoftInput always prevents the hide, we can give
it a try.   But why is that, is it that the keyboard hide is on a delayed timer
and showSoftInput wins a race?  Is there a way to make sure the keyboard is not
hidden even if there are unusual delays in tasks waking up?

[Changwan Ryu, 2016/02/12 00:36:03]

Hmm... I think nullity of input connection is not directly connected to OSK show
and hide.
For example, physical keyboard should still work even when OSK is hidden.
If a keyboard app checks nullity input connection and update their window based
on it, then
there may flickering. However, that will also cause flicker when jumping from
view to view,
so it seems that no keyboard app does something like that (at least so for the
handful of apps I've checked with.)

+        }
+        if (DEBUG_LOGS) Log.w(TAG, "initializeAndGet: running on IME thread");
+        if (mThreadedInputConnection == null) {
+            if (DEBUG_LOGS) Log.w(TAG, "Creating ThreadedInputConnection...");
+            mThreadedInputConnection = new ThreadedInputConnection(imeAdapter, mThreadManager);
+        }
+        mThreadedInputConnection.initializeOutAttrs(inputType, inputFlags, outAttrs);
+        return mThreadedInputConnection;
+    }
+
+    private void triggerDelayedOnCreateInputConnection(final View view) {
+        if (DEBUG_LOGS) Log.w(TAG, "triggerDelayedOnCreateInputConnection");
+        mProxyView = new ThreadedInputConnectionProxyView(

[aelias_OOO_until_Jul13, 2016/02/08 23:17:14]

This ThreadedInputConnectionProxyView class is not defined anywhere in this
patch, it looks like you need to "git add" the file?

[Changwan Ryu, 2016/02/11 16:21:08]

Added now.

+                view.getContext(), mHandler, view);
+        mProxyView.requestFocus();
+        view.getHandler().post(new Runnable() {
+            @Override
+            public void run() {
+                mProxyView.onWindowFocusChanged(true);
+                mInputMethodManagerWrapper.isActive(mProxyView);
+            }
+        });
+    }
+
+    @Override
+    public ThreadManager getThreadManager() {
+        return mThreadManager;
+    }
+}