Merge "Ensure View#onInputConnectionClosedInternal() timing"

core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java

@@ -121,25 +121,54 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub {
             // reportFinish() will take effect.
             return;
         }
-        closeConnection();
-
-        // Notify the app that the InputConnection was closed.
-        final View servedView = mServedView.get();
-        if (servedView != null) {
-            final Handler handler = servedView.getHandler();
-            // The handler is null if the view is already detached. When that's the case, for
-            // now, we simply don't dispatch this callback.
-            if (handler != null) {
-                if (DEBUG) {
-                    Log.v(TAG, "Calling View.onInputConnectionClosed: view=" + servedView);
+        dispatch(() -> {
+            // Note that we do not need to worry about race condition here, because 1) mFinished is
+            // updated only inside this block, and 2) the code here is running on a Handler hence we
+            // assume multiple closeConnection() tasks will not be handled at the same time.
+            if (isFinished()) {
+                return;
+            }
+            Trace.traceBegin(Trace.TRACE_TAG_INPUT, "InputConnection#closeConnection");
+            try {
+                InputConnection ic = getInputConnection();
+                // Note we do NOT check isActive() here, because this is safe
+                // for an IME to call at any time, and we need to allow it
+                // through to clean up our state after the IME has switched to
+                // another client.
+                if (ic == null) {
+                    return;
                 }
-                if (handler.getLooper().isCurrentThread()) {
-                    servedView.onInputConnectionClosedInternal();
-                } else {
-                    handler.post(servedView::onInputConnectionClosedInternal);
+                @MissingMethodFlags
+                final int missingMethods = InputConnectionInspector.getMissingMethodFlags(ic);
+                if ((missingMethods & MissingMethodFlags.CLOSE_CONNECTION) == 0) {
+                    ic.closeConnection();
+                }
+            } finally {
+                synchronized (mLock) {
+                    mInputConnection = null;
+                    mFinished = true;
+                }
+                Trace.traceEnd(Trace.TRACE_TAG_INPUT);
+            }
+
+            // Notify the app that the InputConnection was closed.
+            final View servedView = mServedView.get();
+            if (servedView != null) {
+                final Handler handler = servedView.getHandler();
+                // The handler is null if the view is already detached. When that's the case, for
+                // now, we simply don't dispatch this callback.
+                if (handler != null) {
+                    if (DEBUG) {
+                        Log.v(TAG, "Calling View.onInputConnectionClosed: view=" + servedView);
+                    }
+                    if (handler.getLooper().isCurrentThread()) {
+                        servedView.onInputConnectionClosedInternal();
+                    } else {
+                        handler.post(servedView::onInputConnectionClosedInternal);
+                    }
                 }
             }
-        }
+        });
     }
 
     @Override
@@ -705,39 +734,6 @@ public final class RemoteInputConnectionImpl extends IInputContext.Stub {
         });
     }
 
-    private void closeConnection() {
-        dispatch(() -> {
-            // Note that we do not need to worry about race condition here, because 1) mFinished is
-            // updated only inside this block, and 2) the code here is running on a Handler hence we
-            // assume multiple closeConnection() tasks will not be handled at the same time.
-            if (isFinished()) {
-                return;
-            }
-            Trace.traceBegin(Trace.TRACE_TAG_INPUT, "InputConnection#closeConnection");
-            try {
-                InputConnection ic = getInputConnection();
-                // Note we do NOT check isActive() here, because this is safe
-                // for an IME to call at any time, and we need to allow it
-                // through to clean up our state after the IME has switched to
-                // another client.
-                if (ic == null) {
-                    return;
-                }
-                @MissingMethodFlags
-                final int missingMethods = InputConnectionInspector.getMissingMethodFlags(ic);
-                if ((missingMethods & MissingMethodFlags.CLOSE_CONNECTION) == 0) {
-                    ic.closeConnection();
-                }
-            } finally {
-                synchronized (mLock) {
-                    mInputConnection = null;
-                    mFinished = true;
-                }
-                Trace.traceEnd(Trace.TRACE_TAG_INPUT);
-            }
-        });
-    }
-
     @Override
     public void commitContent(InputContentInfo inputContentInfo, int flags, Bundle opts,
             IBooleanResultCallback callback) {

core/tests/coretests/src/android/view/ViewInputConnectionTest.java

@@ -19,13 +19,25 @@ package android.view;
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
+import android.annotation.DurationMillisLong;
 import android.app.Instrumentation;
 import android.content.Context;
+import android.graphics.Color;
+import android.os.Bundle;
 import android.os.Handler;
+import android.os.HandlerThread;
+import android.os.Process;
+import android.text.InputType;
 import android.text.format.DateUtils;
+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 android.view.inputmethod.InputContentInfo;
 import android.view.inputmethod.InputMethodManager;
 import android.widget.Button;
 import android.widget.EditText;
@@ -45,12 +57,23 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+
 /**
  * Tests for internal APIs/behaviors of {@link View} and {@link InputConnection}.
  */
 @MediumTest
 @RunWith(AndroidJUnit4.class)
 public class ViewInputConnectionTest {
+    @DurationMillisLong
+    private static final long TIMEOUT = 5000;
+    @DurationMillisLong
+    private static final long EXPECTED_TIMEOUT = 500;
+
     @Rule
     public ActivityTestRule<ViewInputConnectionTestActivity> mActivityRule =
             new ActivityTestRule<>(ViewInputConnectionTestActivity.class);
@@ -289,4 +312,282 @@ public class ViewInputConnectionTest {
             super.onInputConnectionClosedInternal();
         }
     }
+
+
+    @Test
+    public void testInputConnectionCallbacks_nonUiThread() throws Throwable {
+        try (InputConnectionHandlingThread thread = new InputConnectionHandlingThread()) {
+            final ViewGroup viewGroup = getOnMainSync(() -> mActivity.findViewById(R.id.root));
+            final TestOffThreadEditor editor = getOnMainSync(() -> {
+                final TestOffThreadEditor myEditor =
+                        new TestOffThreadEditor(viewGroup.getContext(), thread.getHandler());
+                viewGroup.addView(myEditor);
+                myEditor.requestFocus();
+                return myEditor;
+            });
+
+            mInstrumentation.waitForIdleSync();
+
+            assertThat(editor.mOnCreateInputConnectionCalled.await(TIMEOUT, TimeUnit.MILLISECONDS))
+                    .isTrue();
+
+            // Invalidate the currently used InputConnection by moving the focus to a new EditText.
+            mActivityRule.runOnUiThread(() -> {
+                final EditText editText = new EditText(viewGroup.getContext());
+                viewGroup.addView(editText);
+                editText.requestFocus();
+            });
+
+            // Make sure that InputConnection#closeConnection() gets called on the handler thread.
+            assertThat(editor.mInputConnectionClosedCalled.await(TIMEOUT, TimeUnit.MILLISECONDS))
+                    .isTrue();
+            assertThat(editor.mInputConnectionClosedCallingThreadId.get())
+                    .isEqualTo(thread.getThreadId());
+
+            // Make sure that View#onInputConnectionClosed() is not yet dispatched, because
+            // InputConnection#closeConnection() is still blocked.
+            assertThat(editor.mOnInputConnectionClosedCalled.await(
+                    EXPECTED_TIMEOUT, TimeUnit.MILLISECONDS)).isFalse();
+
+            // Unblock InputConnection#closeConnection()
+            editor.mInputConnectionClosedBlocker.countDown();
+
+            // Make sure that View#onInputConnectionClosed() is dispatched on the main thread.
+            assertThat(editor.mOnInputConnectionClosedCalled.await(TIMEOUT, TimeUnit.MILLISECONDS))
+                    .isTrue();
+            assertThat(editor.mInputConnectionClosedBlockerTimedOut.get()).isFalse();
+            assertThat(editor.mOnInputConnectionClosedCallingThreadId.get())
+                    .isEqualTo(getOnMainSync(Process::myTid));
+        }
+    }
+
+    private <T> T getOnMainSync(@NonNull Supplier<T> supplier) throws Throwable {
+        final AtomicReference<T> result = new AtomicReference<>();
+        mActivityRule.runOnUiThread(() -> result.set(supplier.get()));
+        return result.get();
+    }
+
+    private static class TestOffThreadEditor extends View {
+        private static final int TEST_VIEW_HEIGHT = 10;
+
+        public CountDownLatch mOnCreateInputConnectionCalled = new CountDownLatch(1);
+        public CountDownLatch mInputConnectionClosedCalled = new CountDownLatch(1);
+        public CountDownLatch mInputConnectionClosedBlocker = new CountDownLatch(1);
+        public AtomicBoolean mInputConnectionClosedBlockerTimedOut = new AtomicBoolean();
+        public AtomicReference<Integer> mInputConnectionClosedCallingThreadId =
+                new AtomicReference<>();
+
+        public CountDownLatch mOnInputConnectionClosedCalled = new CountDownLatch(1);
+        public AtomicReference<Integer> mOnInputConnectionClosedCallingThreadId =
+                new AtomicReference<>();
+
+        private final Handler mInputConnectionHandler;
+
+        TestOffThreadEditor(Context context, @NonNull Handler inputConnectionHandler) {
+            super(context);
+            setBackgroundColor(Color.YELLOW);
+            setLayoutParams(new ViewGroup.LayoutParams(
+                    ViewGroup.LayoutParams.MATCH_PARENT, TEST_VIEW_HEIGHT));
+            setFocusableInTouchMode(true);
+            setFocusable(true);
+            mInputConnectionHandler = inputConnectionHandler;
+        }
+
+        @Override
+        public InputConnection onCreateInputConnection(EditorInfo outAttrs) {
+            mOnCreateInputConnectionCalled.countDown();
+            outAttrs.inputType = InputType.TYPE_CLASS_TEXT;
+            return new NoOpInputConnection() {
+                @Override
+                public Handler getHandler() {
+                    return mInputConnectionHandler;
+                }
+
+                @Override
+                public void closeConnection() {
+                    mInputConnectionClosedCallingThreadId.compareAndSet(null, Process.myTid());
+                    mInputConnectionClosedCalled.countDown();
+                    try {
+                        if (mInputConnectionClosedBlocker.await(TIMEOUT, TimeUnit.MILLISECONDS)) {
+                            return;
+                        }
+                    } catch (InterruptedException e) {
+                    }
+                    mInputConnectionClosedBlockerTimedOut.set(true);
+                }
+            };
+        }
+
+        @Override
+        public boolean onCheckIsTextEditor() {
+            return true;
+        }
+
+        @Override
+        public void onInputConnectionClosedInternal() {
+            mOnInputConnectionClosedCallingThreadId.compareAndSet(null, Process.myTid());
+            mOnInputConnectionClosedCalled.countDown();
+            super.onInputConnectionClosedInternal();
+        }
+    }
+
+    static class NoOpInputConnection implements InputConnection {
+
+        @Override
+        public CharSequence getTextBeforeCursor(int n, int flags) {
+            return null;
+        }
+
+        @Override
+        public CharSequence getTextAfterCursor(int n, int flags) {
+            return null;
+        }
+
+        @Override
+        public CharSequence getSelectedText(int flags) {
+            return null;
+        }
+
+        @Override
+        public int getCursorCapsMode(int reqModes) {
+            return 0;
+        }
+
+        @Override
+        public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) {
+            return null;
+        }
+
+        @Override
+        public boolean deleteSurroundingText(int beforeLength, int afterLength) {
+            return false;
+        }
+
+        @Override
+        public boolean deleteSurroundingTextInCodePoints(int beforeLength, int afterLength) {
+            return false;
+        }
+
+        @Override
+        public boolean setComposingText(CharSequence text, int newCursorPosition) {
+            return false;
+        }
+
+        @Override
+        public boolean setComposingRegion(int start, int end) {
+            return false;
+        }
+
+        @Override
+        public boolean finishComposingText() {
+            return false;
+        }
+
+        @Override
+        public boolean commitText(CharSequence text, int newCursorPosition) {
+            return false;
+        }
+
+        @Override
+        public boolean commitCompletion(CompletionInfo text) {
+            return false;
+        }
+
+        @Override
+        public boolean commitCorrection(CorrectionInfo correctionInfo) {
+            return false;
+        }
+
+        @Override
+        public boolean setSelection(int start, int end) {
+            return false;
+        }
+
+        @Override
+        public boolean performEditorAction(int editorAction) {
+            return false;
+        }
+
+        @Override
+        public boolean performContextMenuAction(int id) {
+            return false;
+        }
+
+        @Override
+        public boolean beginBatchEdit() {
+            return false;
+        }
+
+        @Override
+        public boolean endBatchEdit() {
+            return false;
+        }
+
+        @Override
+        public boolean sendKeyEvent(KeyEvent event) {
+            return false;
+        }
+
+        @Override
+        public boolean clearMetaKeyStates(int states) {
+            return false;
+        }
+
+        @Override
+        public boolean reportFullscreenMode(boolean enabled) {
+            return false;
+        }
+
+        @Override
+        public boolean performPrivateCommand(String action, Bundle data) {
+            return false;
+        }
+
+        @Override
+        public boolean requestCursorUpdates(int cursorUpdateMode) {
+            return false;
+        }
+
+        @Override
+        public Handler getHandler() {
+            return null;
+        }
+
+        @Override
+        public void closeConnection() {
+
+        }
+
+        @Override
+        public boolean commitContent(InputContentInfo inputContentInfo, int flags, Bundle opts) {
+            return false;
+        }
+    }
+
+    private static final class InputConnectionHandlingThread extends HandlerThread
+            implements AutoCloseable {
+
+        private final Handler mHandler;
+
+        InputConnectionHandlingThread() {
+            super("IC-callback");
+            start();
+            mHandler = Handler.createAsync(getLooper());
+        }
+
+        @NonNull
+        Handler getHandler() {
+            return mHandler;
+        }
+
+        @Override
+        public void close() {
+            quitSafely();
+            try {
+                join(TIMEOUT);
+            } catch (InterruptedException e) {
+                fail("Failed to stop the thread: " + e);
+            }
+        }
+    }
 }