Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ensure View#onInputConnectionClosedInternal() timing

Browse Source 
This is a follow up CL to our previous CLs [1][2], which introduced
an @hide callback

  View#onInputConnectionClosedInternal()

to notify View when an is closed.

What this CL aims to do is to fix a potential problem in a code path
that has not been yet used.  Thus there should be, in theory, no
observable app compat impact.

The problem is that

  RemoteInputConnectionImpl#deactivate()

can dispatch

  A: View#onInputConnectionClosedInternal()

before

  B: InputConnection#closeConnection()

is completed when A and B need to be dispatched to two different
threads.  This can, in theory, happen when

  A: View#onInputConnectionClosedInternal()
  C: InputConnection#getHandler()

are both explicitly overridden.  That said, A is still @hide and only
by TextView, which basically does not support InputConnection with a
custom InputConnection#getHandler().

Anyway, with this CL A is guaranteed to happen after B under any
circumstances.

 [1]: Iaafb0a03126c9292c24415f866dbdd72cadfa239
      7b384751ea
 [2]: I9280604e7ec7e8d08c1179e6bbf0068647a41040
      7b384751ea

Bug: 163400105
Test: atest FrameworksCoreTests:ViewInputConnectionTest
Change-Id: I8a0e321ecf6e0b3be4b6ab1a35e6ac7259826c2e
This commit is contained in:
Yohei Yukawa
2021-08-05 17:02:02 -07:00
parent a4816fae9a
commit d55ddf27c6
2 changed files with 346 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

94
core/java/com/android/internal/inputmethod/RemoteInputConnectionImpl.java
View File

@@ -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) {

301
core/tests/coretests/src/android/view/ViewInputConnectionTest.java
View File

@@ -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);
}
}
}
}