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

Do not cleanup IME window if show is requested.

Browse Source 
This CL fixes a race condition for IMM#showSoftInput, which surfaces
when it's called during an IME hide animation.
IMM#showSoftInput ends up calling WIC#show asynchronously, but at that
time the running IME hide animation may have already been finished
successfully, and WIC#show may fail to cancel the hide animation
(then the cleanup IMM#notifyImeHidden hides the IME again disruptively).

I think a clean fix is to have IMM#showSoftInput call WIC#show
synchronously. However, this requires a significant refactoring.

As a short term fix, this CL adds a boolean field indicating whether or
not IMM#showSoftInput has been called. If it's called, we skip calling
IMM#notifyImeHidden.

Bug: 221483132
Bug: 225674038
Test: atest InputMethodStressTest
Test: atest CtsInputMethodTestCases
Test: atest WindowInsetsAnimationControllerTests
Change-Id: I36d570630085d0bc34097a2433208601dc9cb0fd
(cherry picked from commit 4c607982ed)
Merged-In: I36d570630085d0bc34097a2433208601dc9cb0fd
This commit is contained in:
Kohsuke Yatoh
2022-04-14 15:19:29 -07:00
parent 4844170377
commit eae7ca8165
4 changed files with 168 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
core/java/android/view/ImeInsetsSourceConsumer.java
View File

@@ -18,7 +18,9 @@ package android.view;
import static android.os.Trace.TRACE_TAG_VIEW;
import static android.view.ImeInsetsSourceConsumerProto.INSETS_SOURCE_CONSUMER;
import static android.view.ImeInsetsSourceConsumerProto.IS_HIDE_ANIMATION_RUNNING;
import static android.view.ImeInsetsSourceConsumerProto.IS_REQUESTED_VISIBLE_AWAITING_CONTROL;
import static android.view.ImeInsetsSourceConsumerProto.IS_SHOW_REQUESTED_DURING_HIDE_ANIMATION;
import static android.view.InsetsController.AnimationType;
import static android.view.InsetsState.ITYPE_IME;
@@ -43,6 +45,16 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer {
*/
private boolean mIsRequestedVisibleAwaitingControl;
private boolean mIsHideAnimationRunning;
/**
* Tracks whether {@link WindowInsetsController#show(int)} or
* {@link InputMethodManager#showSoftInput(View, int)} is called during IME hide animation.
* If it was called, we should not call {@link InputMethodManager#notifyImeHidden(IBinder)},
* because the IME is being shown.
*/
private boolean mIsShowRequestedDuringHideAnimation;
public ImeInsetsSourceConsumer(
InsetsState state, Supplier<Transaction> transactionSupplier,
InsetsController controller) {
@@ -62,6 +74,12 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer {
mIsRequestedVisibleAwaitingControl = false;
}
@Override
public void show(boolean fromIme) {
super.show(fromIme);
onShowRequested();
}
@Override
public void hide() {
super.hide();
@@ -73,10 +91,20 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer {
hide();
if (animationFinished) {
// remove IME surface as IME has finished hide animation.
notifyHidden();
removeSurface();
// Remove IME surface as IME has finished hide animation, if there is no pending
// show request.
if (!mIsShowRequestedDuringHideAnimation) {
notifyHidden();
removeSurface();
}
}
// This method is called
// (1) before the hide animation starts.
// (2) after the hide animation ends.
// (3) if the IME is not controllable (animationFinished == true in this case).
// We should reset mIsShowRequestedDuringHideAnimation in all cases.
mIsHideAnimationRunning = !animationFinished;
mIsShowRequestedDuringHideAnimation = false;
}
/**
@@ -157,9 +185,20 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer {
final long token = proto.start(fieldId);
super.dumpDebug(proto, INSETS_SOURCE_CONSUMER);
proto.write(IS_REQUESTED_VISIBLE_AWAITING_CONTROL, mIsRequestedVisibleAwaitingControl);
proto.write(IS_HIDE_ANIMATION_RUNNING, mIsHideAnimationRunning);
proto.write(IS_SHOW_REQUESTED_DURING_HIDE_ANIMATION, mIsShowRequestedDuringHideAnimation);
proto.end(token);
}
/**
* Called when {@link #show} or {@link InputMethodManager#showSoftInput(View, int)} is called.
*/
public void onShowRequested() {
if (mIsHideAnimationRunning) {
mIsShowRequestedDuringHideAnimation = true;
}
}
private InputMethodManager getImm() {
return mController.getHost().getInputMethodManager();
}

15
core/java/android/view/inputmethod/InputMethodManager.java
View File

@@ -543,6 +543,7 @@ public final class InputMethodManager {
static final int MSG_BIND_ACCESSIBILITY_SERVICE = 11;
static final int MSG_UNBIND_ACCESSIBILITY_SERVICE = 12;
static final int MSG_UPDATE_VIRTUAL_DISPLAY_TO_SCREEN_MATRIX = 30;
static final int MSG_ON_SHOW_REQUESTED = 31;
private static boolean isAutofillUIShowing(View servedView) {
AutofillManager afm = servedView.getContext().getSystemService(AutofillManager.class);
@@ -1117,6 +1118,14 @@ public final class InputMethodManager {
}
return;
}
case MSG_ON_SHOW_REQUESTED: {
synchronized (mH) {
if (mImeInsetsConsumer != null) {
mImeInsetsConsumer.onShowRequested();
}
}
return;
}
}
}
}
@@ -1834,6 +1843,9 @@ public final class InputMethodManager {
return false;
}
// Makes sure to call ImeInsetsSourceConsumer#onShowRequested on the UI thread.
// TODO(b/229426865): call WindowInsetsController#show instead.
mH.executeOrSendMessage(Message.obtain(mH, MSG_ON_SHOW_REQUESTED));
try {
Log.d(TAG, "showSoftInput() view=" + view + " flags=" + flags + " reason="
+ InputMethodDebug.softInputDisplayReasonToString(reason));
@@ -1869,6 +1881,9 @@ public final class InputMethodManager {
Log.w(TAG, "No current root view, ignoring showSoftInputUnchecked()");
return;
}
// Makes sure to call ImeInsetsSourceConsumer#onShowRequested on the UI thread.
// TODO(b/229426865): call WindowInsetsController#show instead.
mH.executeOrSendMessage(Message.obtain(mH, MSG_ON_SHOW_REQUESTED));
mService.showSoftInput(
mClient,
mCurRootView.getView().getWindowToken(),

2
core/proto/android/view/imeinsetssourceconsumer.proto
View File

@@ -29,4 +29,6 @@ message ImeInsetsSourceConsumerProto {
optional InsetsSourceConsumerProto insets_source_consumer = 1;
reserved 2; // focused_editor = 2
optional bool is_requested_visible_awaiting_control = 3;
optional bool is_hide_animation_running = 4;
optional bool is_show_requested_during_hide_animation = 5;
}

122
tests/InputMethodStressTest/src/com/android/inputmethod/stresstest/ImeOpenCloseStressTest.java
View File

@@ -27,8 +27,10 @@ import android.app.Activity;
import android.app.Instrumentation;
import android.content.Intent;
import android.os.Bundle;
import android.os.SystemClock;
import android.platform.test.annotations.RootPermissionTest;
import android.platform.test.rule.UnlockScreenRule;
import android.util.Log;
import android.view.WindowInsets;
import android.view.WindowInsetsAnimation;
import android.view.inputmethod.InputMethodManager;
@@ -39,16 +41,19 @@ import androidx.annotation.Nullable;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.platform.app.InstrumentationRegistry;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import java.util.ArrayList;
import java.util.List;
@RootPermissionTest
@RunWith(AndroidJUnit4.class)
public final class ImeOpenCloseStressTest {
private static final String TAG = "ImeOpenCloseStressTest";
private static final int NUM_TEST_ITERATIONS = 10;
@Rule
@@ -57,29 +62,103 @@ public final class ImeOpenCloseStressTest {
@Rule
public ScreenCaptureRule mScreenCaptureRule =
new ScreenCaptureRule("/sdcard/InputMethodStressTest");
private Instrumentation mInstrumentation;
@Before
public void setUp() {
mInstrumentation = InstrumentationRegistry.getInstrumentation();
}
@Test
public void test() {
Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
Intent intent = new Intent()
.setAction(Intent.ACTION_MAIN)
.setClass(instrumentation.getContext(), TestActivity.class)
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP);
TestActivity activity = (TestActivity) instrumentation.startActivitySync(intent);
public void testShowHide_waitingVisibilityChange() {
TestActivity activity = TestActivity.start();
EditText editText = activity.getEditText();
waitOnMainUntil("activity should gain focus", editText::hasWindowFocus);
for (int i = 0; i < NUM_TEST_ITERATIONS; i++) {
String msgPrefix = "Iteration #" + i + " ";
instrumentation.runOnMainSync(activity::showIme);
Log.i(TAG, msgPrefix + "start");
mInstrumentation.runOnMainSync(activity::showIme);
waitOnMainUntil(msgPrefix + "IME should be visible", () -> isImeShown(editText));
mInstrumentation.runOnMainSync(activity::hideIme);
waitOnMainUntil(msgPrefix + "IME should be hidden", () -> !isImeShown(editText));
}
}
@Test
public void testShowHide_waitingAnimationEnd() {
TestActivity activity = TestActivity.start();
activity.enableAnimationMonitoring();
EditText editText = activity.getEditText();
waitOnMainUntil("activity should gain focus", editText::hasWindowFocus);
for (int i = 0; i < NUM_TEST_ITERATIONS; i++) {
String msgPrefix = "Iteration #" + i + " ";
Log.i(TAG, msgPrefix + "start");
mInstrumentation.runOnMainSync(activity::showIme);
waitOnMainUntil(msgPrefix + "IME should be visible",
() -> !activity.isAnimating() && isImeShown(editText));
instrumentation.runOnMainSync(activity::hideIme);
mInstrumentation.runOnMainSync(activity::hideIme);
waitOnMainUntil(msgPrefix + "IME should be hidden",
() -> !activity.isAnimating() && !isImeShown(editText));
}
}
@Test
public void testShowHide_intervalAfterHide() {
// Regression test for b/221483132
TestActivity activity = TestActivity.start();
EditText editText = activity.getEditText();
// Intervals = 10, 20, 30, ..., 100, 150, 200, ...
List<Integer> intervals = new ArrayList<>();
for (int i = 10; i < 100; i += 10) intervals.add(i);
for (int i = 100; i < 1000; i += 50) intervals.add(i);
waitOnMainUntil("activity should gain focus", editText::hasWindowFocus);
for (int intervalMillis : intervals) {
String msgPrefix = "Interval = " + intervalMillis + " ";
Log.i(TAG, msgPrefix + " start");
mInstrumentation.runOnMainSync(activity::hideIme);
SystemClock.sleep(intervalMillis);
mInstrumentation.runOnMainSync(activity::showIme);
waitOnMainUntil(msgPrefix + "IME should be visible",
() -> isImeShown(editText));
}
}
@Test
public void testShowHideInSameFrame() {
TestActivity activity = TestActivity.start();
activity.enableAnimationMonitoring();
EditText editText = activity.getEditText();
waitOnMainUntil("activity should gain focus", editText::hasWindowFocus);
// hidden -> show -> hide
mInstrumentation.runOnMainSync(() -> {
Log.i(TAG, "Calling showIme() and hideIme()");
activity.showIme();
activity.hideIme();
});
// Wait until IMMS / IMS handles messages.
SystemClock.sleep(1000);
mInstrumentation.waitForIdleSync();
waitOnMainUntil("IME should be invisible after show/hide", () -> !isImeShown(editText));
mInstrumentation.runOnMainSync(activity::showIme);
waitOnMainUntil("IME should be visible",
() -> !activity.isAnimating() && isImeShown(editText));
mInstrumentation.waitForIdleSync();
// shown -> hide -> show
mInstrumentation.runOnMainSync(() -> {
Log.i(TAG, "Calling hideIme() and showIme()");
activity.hideIme();
activity.showIme();
});
// Wait until IMMS / IMS handles messages.
SystemClock.sleep(1000);
mInstrumentation.waitForIdleSync();
waitOnMainUntil("IME should be visible after hide/show",
() -> !activity.isAnimating() && isImeShown(editText));
}
public static class TestActivity extends Activity {
private EditText mEditText;
@@ -107,6 +186,15 @@ public final class ImeOpenCloseStressTest {
}
};
public static TestActivity start() {
Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
Intent intent = new Intent()
.setAction(Intent.ACTION_MAIN)
.setClass(instrumentation.getContext(), TestActivity.class)
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP);
return (TestActivity) instrumentation.startActivitySync(intent);
}
@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
@@ -116,9 +204,6 @@ public final class ImeOpenCloseStressTest {
mEditText = new EditText(this);
rootView.addView(mEditText, new LinearLayout.LayoutParams(MATCH_PARENT, WRAP_CONTENT));
setContentView(rootView);
// Enable WindowInsetsAnimation.
getWindow().setDecorFitsSystemWindows(false);
mEditText.setWindowInsetsAnimationCallback(mWindowInsetsAnimationCallback);
}
public EditText getEditText() {
@@ -126,16 +211,27 @@ public final class ImeOpenCloseStressTest {
}
public void showIme() {
Log.i(TAG, "TestActivity.showIme");
mEditText.requestFocus();
InputMethodManager imm = getSystemService(InputMethodManager.class);
imm.showSoftInput(mEditText, 0);
}
public void hideIme() {
Log.i(TAG, "TestActivity.hideIme");
InputMethodManager imm = getSystemService(InputMethodManager.class);
imm.hideSoftInputFromWindow(mEditText.getWindowToken(), 0);
}
public void enableAnimationMonitoring() {
// Enable WindowInsetsAnimation.
// Note that this has a side effect of disabling InsetsAnimationThreadControlRunner.
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
getWindow().setDecorFitsSystemWindows(false);
mEditText.setWindowInsetsAnimationCallback(mWindowInsetsAnimationCallback);
});
}
public boolean isAnimating() {
return mIsAnimating;
}