Introduce ThreadedInputConnection behind a switch

content/public/android/java/src/org/chromium/content/browser/input/ThreadedInputConnectionFactory.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.content.Context;
+import android.os.Handler;
+import android.os.HandlerThread;
+import android.os.Looper;
+import android.os.Message;
+import android.os.MessageQueue;
+import android.view.View;
+import android.view.inputmethod.EditorInfo;
+import android.view.inputmethod.InputMethodManager;
+
+import org.chromium.base.Log;
+import org.chromium.content.browser.input.ChromiumBaseInputConnection.ThreadManager;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+/**
+ * Default factory for ChromiumBaseInputConnection classes.
+ */
+public class ThreadedInputConnectionFactory implements ChromiumBaseInputConnection.Factory {
+    private static final String TAG = "cr_Ime";
+
+    private ThreadedInputConnection mChromiumInputConnection;
+    private final Handler mHandler;
+    private final ThreadedInputConnection.ThreadManager mThreadManager;
+
+    ThreadedInputConnectionFactory() {
+        HandlerThread thread =
+                new HandlerThread("InputConnectionHandlerThread", HandlerThread.NORM_PRIORITY);
+        thread.start();
+        mHandler = new Handler(thread.getLooper());
+        mThreadManager = new ThreadManager(mHandler);
+    }
+
+    @Override
+    public ChromiumBaseInputConnection initializeAndGet(View view, ImeAdapter imeAdapter,
+            int inputType, int inputFlags, EditorInfo outAttrs) {
+        ImeUtils.assertOnUiThread();
+        if (mChromiumInputConnection == null) {
+            Log.d(TAG, "Creating ChromiumInputConnection...");
+            mChromiumInputConnection = new ThreadedInputConnection(imeAdapter, mThreadManager);
+        }
+        mChromiumInputConnection.initializeOutAttrsOnUiThread(inputType, inputFlags, outAttrs);
+        switchInputConnectionLooper(view.getContext(), view.getHandler());
+        return mChromiumInputConnection;
+    }
+
+    @Override
+    public ThreadManager getThreadManager() {
+        return mThreadManager;
+    }
+
+    /**
+     * Pump messages from a handler and add them to another handler.
+     *
+     * @param sourceHandler The source handler.
+     * @param targetHandler The target handler.
+     * @throws NoSuchFieldException
+     * @throws IllegalAccessException
+     * @throws IllegalArgumentException
+     * @throws NoSuchMethodException
+     * @throws InvocationTargetException
+     */
+    private void pumpHandlerMessages(Handler sourceHandler, Handler targetHandler)

[Ted C, 2016/02/02 23:14:43]

yikes...this method is...terrifying.

I know your document describes the fragility of this approach based on third
parties modifying the AOSP code, but this is beyond anything we've done in the
past.

In particular, we've done our best to ensure that anytime we need reflection it
is only a additional bonus but never for core functionality.  This definitely is
way past that line.

[aelias_OOO_until_Jul13, 2016/02/02 23:51:56]

I chatted offline with Ted about this.  Here's what we agreed on:

1) One possibility we haven't explored enough is creating a loose, invisible
View with a different Looper for communication with the IME thread.  This is the
cleanest solution in my mind, it's purely using existing public APIs.  I'm
almost certain this will work for Chrome, but I thought it couldn't for WebView
which generally never creates another View, so I dismissed it.  But Ted said
there's no reason WebView can't create a View, and looking at the API we usually
trigger keyboard input ourselves using an API like
InputMethodManager.restartInput(View), so maybe it would work after all?  Could
you give it a try?

2) Failing that, we need a plan to fully phase out the reflection in the longer
term.  We should at least switch to the stack-trace-examining approach for N and
up (we can change the WebView implementation to override getHandler).

3) Ted also wants a plan to roll it out gradually.  We could have a finch
experiment at 50/50, with a fallback and UMA log on failures (for the kinds of
failure that we can detect), and monitor the results before deleting the
ReplicaInputConnection.

[Changwan Ryu, 2016/02/03 00:02:47]

Thanks for the feedback and new idea. I'll investigate 1) first and get back to
you guys.

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

I've explored many different ideas, and switched to the new approach.

+            throws NoSuchFieldException, IllegalAccessException, IllegalArgumentException,
+                   NoSuchMethodException, InvocationTargetException {
+        Looper looper = sourceHandler.getLooper();
+        Method getQueueMethod = Looper.class.getDeclaredMethod("getQueue");
+        getQueueMethod.setAccessible(true);
+        MessageQueue queue = (MessageQueue) getQueueMethod.invoke(looper);
+        getQueueMethod.setAccessible(false);
+
+        Field messagesField = MessageQueue.class.getDeclaredField("mMessages");
+        Field nextField = Message.class.getDeclaredField("next");
+
+        synchronized (queue) {
+            // Copy messages first.
+            messagesField.setAccessible(true);
+            nextField.setAccessible(true);
+            Message msg = (Message) messagesField.get(queue);
+            while (msg != null) {
+                if (msg.getTarget() == sourceHandler) {
+                    Log.d(TAG, "Copying a message to the new handler...");
+                    Message newMsg = targetHandler.obtainMessage();
+                    newMsg.copyFrom(msg);
+                    targetHandler.sendMessage(newMsg);

[Ted C, 2016/02/02 23:14:43]

This looks like it would clobber any delayed-ness of messages.

In particular, sendMessage looks to clobber msg.when.

Should we care about that?

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

I've switched to another approach. I'll fix this only if this old approach
proves to be better.

+                }
+                msg = (Message) nextField.get(msg);
+            }
+            messagesField.setAccessible(false);
+            nextField.setAccessible(false);
+
+            // Now remove messages from the first handler.
+            sourceHandler.removeCallbacksAndMessages(null);
+        }
+    }
+
+    /**
+     * Switch the looper in InputMethodManager so that future calls to InputConnection
+     * can run in IME thread.
+     *
+     * @param context The context
+     * @param viewHandler The current handler for the View that this InputConnection will be
+     *                    attached to.
+     */
+    private void switchInputConnectionLooper(Context context, Handler viewHandler) {
+        Log.d(TAG, "switchInputConnectionLooper");
+        // Retrieve the same singleton instance of InputMethodManager that called
+        // onCreateInputConnection().
+        final InputMethodManager inputMethodManager =
+                (InputMethodManager) context.getSystemService(Context.INPUT_METHOD_SERVICE);
+        final Handler icHandler = mHandler;
+        viewHandler.post(new Runnable() {
+            @Override
+            public void run() {
+                Log.d(TAG, "switchInputConnectionLooper: replacing looper and handler");
+                try {
+                    // inputMethodManager.mServedInputConnectionWrapper.mMainLooper =
+                    // icHandler.getLooper();
+                    Field inputConnectionWrapperField = InputMethodManager.class.getDeclaredField(
+                            "mServedInputConnectionWrapper");
+                    inputConnectionWrapperField.setAccessible(true);
+                    Object inputConnectionWrapper =
+                            inputConnectionWrapperField.get(inputMethodManager);
+                    Class<?> iinputConnectionWrapperClass =
+                            Class.forName("com.android.internal.view.IInputConnectionWrapper");
+                    Field looperField =
+                            iinputConnectionWrapperClass.getDeclaredField("mMainLooper");
+                    looperField.setAccessible(true);
+                    looperField.set(inputConnectionWrapper, icHandler.getLooper());
+                    looperField.setAccessible(false);
+
+                    // inputMethodManager.mServedInputConnectionWrapper.mH =
+                    //         new IInputConnectionWrapper.MyHandler(icHandler.getLooper());
+                    Class<?> myHandlerClass = Class.forName(
+                            "com.android.internal.view.IInputConnectionWrapper$MyHandler");
+                    Constructor<?> ctor = myHandlerClass.getDeclaredConstructor(
+                            iinputConnectionWrapperClass, Looper.class);
+                    ctor.setAccessible(true);
+                    Handler myHandler = (Handler) ctor.newInstance(
+                            inputConnectionWrapper, icHandler.getLooper());
+                    Field handlerField = iinputConnectionWrapperClass.getDeclaredField("mH");
+                    ctor.setAccessible(false);
+                    handlerField.setAccessible(true);
+                    Handler oldHandler = (Handler) handlerField.get(inputConnectionWrapper);
+                    handlerField.set(inputConnectionWrapper, myHandler);
+                    handlerField.setAccessible(false);
+
+                    pumpHandlerMessages(oldHandler, myHandler);
+                } catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException
+                        | ClassNotFoundException | NoSuchMethodException | InstantiationException
+                        | InvocationTargetException | NullPointerException e) {
+                    e.printStackTrace();

[Ted C, 2016/02/02 23:14:43]

I would use Log.wtf here.

This is unrecoverable right and the user now has no ability to input text. 
Should we crash here?  Tell the user they are broken (potentially forever)? 
Have a fallback?

I wonder if we should attempt to get all the reflective-y dependencies on the UI
thread and if it fails, go back to the old approach?

We should definitely do that when we attempt this on users in the wild, as I'd
want to gather any sort of UMA about this.

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

I've added verification & crashing logic in ThreadedInputConnectionFactory. With
the new approach,
I wonder if releasing through channels would suffice. We will get crash
information, and we can stop release process
or change flag if crashes are unexpectedly frequent. What do you think?

+                    return;
+                }
+                Log.d(TAG, "switchInputConnectionLooper: replacing done.");
+            }
+        });
+    }
+}