From 28b9201e18777a0b3820b4b37160f1fa02b6fc66 Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Mon, 25 Apr 2022 10:25:00 +0800 Subject: [PATCH 1/2] Make IME visiblity stable without unexpected hidden when setControl 2 reasons that may unexpectly calls hideMySoftInput from IME: 1) When requesting show IME on the activity, before receiving the control from server side, the null control might received from the relayoutWindow, if initially in consumer side didn't get the control yet, we should not hide the IME immediately to broke the on-going show request. 2) When ImeInsetsSourceConsumer#onWindowFocusGained, if we don't set mIsRequestedVisibleAwaitingControl as true if it has requested IME visible but not yet get control, it also could possble mistakenly hide IME when ImeInsetsSourceConsumer#setControl. As the result, we should prevent both case with 1) add returned value for InsetsSourceConsumer#setControl to see whether the control has changed from the server, if the control didn't change like receiving a duplicated null control callback, then in ImeInsetsConsumer didn't have to do anything. 2) set mIsRequestedVisibleAwaitingControl as true when receiving onWindowFocusGained and the host is waiting the control to make IME visibility reliable. Bug: 227142436 Bug: 204524304 Test: atest FlickerTests:LaunchAppShowImeAndDialogThemeAppTest --rerun-until-failure 10 Change-Id: If4b09f5b52bc96cf3429aaa912961f3ac4e326f2 --- .../android/view/ImeInsetsSourceConsumer.java | 15 ++++++++------- core/java/android/view/InsetsSourceConsumer.java | 6 ++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/ImeInsetsSourceConsumer.java b/core/java/android/view/ImeInsetsSourceConsumer.java index 4fdea3b006dc0..332e97c8bcf59 100644 --- a/core/java/android/view/ImeInsetsSourceConsumer.java +++ b/core/java/android/view/ImeInsetsSourceConsumer.java @@ -65,6 +65,9 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { public void onWindowFocusGained(boolean hasViewFocus) { super.onWindowFocusGained(hasViewFocus); getImm().registerImeConsumer(this); + if (isRequestedVisible() && getControl() == null) { + mIsRequestedVisibleAwaitingControl = true; + } } @Override @@ -149,14 +152,11 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { } @Override - public void setControl(@Nullable InsetsSourceControl control, int[] showTypes, + public boolean setControl(@Nullable InsetsSourceControl control, int[] showTypes, int[] hideTypes) { - super.setControl(control, showTypes, hideTypes); - // TODO(b/204524304): clean-up how to deal with the timing issues of hiding IME: - // 1) Already requested show IME, in the meantime of WM callback the control but got null - // control when relayout comes first - // 2) Make sure no regression on some implicit request IME visibility calls (e.g. - // toggleSoftInput) + if (!super.setControl(control, showTypes, hideTypes)) { + return false; + } if (control == null && !mIsRequestedVisibleAwaitingControl) { hide(); removeSurface(); @@ -164,6 +164,7 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { if (control != null) { mIsRequestedVisibleAwaitingControl = false; } + return true; } @Override diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java index 6aab6359d23ed..a82e19b108124 100644 --- a/core/java/android/view/InsetsSourceConsumer.java +++ b/core/java/android/view/InsetsSourceConsumer.java @@ -121,8 +121,9 @@ public class InsetsSourceConsumer { * animation should be run after setting the control. * @param hideTypes An integer array with a single entry that determines which types a hide * animation should be run after setting the control. + * @return Whether the control has changed from the server */ - public void setControl(@Nullable InsetsSourceControl control, + public boolean setControl(@Nullable InsetsSourceControl control, @InsetsType int[] showTypes, @InsetsType int[] hideTypes) { if (mType == ITYPE_IME) { ImeTracing.getInstance().triggerClientDump("InsetsSourceConsumer#setControl", @@ -133,7 +134,7 @@ public class InsetsSourceConsumer { mSourceControl.release(SurfaceControl::release); mSourceControl = control; } - return; + return false; } SurfaceControl oldLeash = mSourceControl != null ? mSourceControl.getLeash() : null; @@ -198,6 +199,7 @@ public class InsetsSourceConsumer { if (lastControl != null) { lastControl.release(SurfaceControl::release); } + return true; } @VisibleForTesting From c3fcf5dbc25b3f08d78cb7df744c290279f1c2a3 Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Tue, 26 Apr 2022 11:56:39 +0800 Subject: [PATCH 2/2] Make DialogThemedActivity main window is not focusable In case the window may become to IME input target when dismissing the dialog window to mistakly hide the IME when getting the IME insets control. Also, add the SystemBars' insets visibility string on the dialog for the test to easier track and verify the insets visiblity before thetest. Bug: 227142436 Test: atest LaunchAppShowImeAndDialogThemeAppTest --rerun-until-failure 10 Change-Id: I96b17c78d4bd211a930bc0e4a083168e83f8aae9 --- .../flicker/helpers/ImeAppAutoFocusHelper.kt | 30 +++++++++++++++++++ .../LaunchAppShowImeAndDialogThemeAppTest.kt | 10 +++++++ .../flicker/testapp/DialogThemedActivity.java | 20 ++++++++++++- 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/tests/FlickerTests/src/com/android/server/wm/flicker/helpers/ImeAppAutoFocusHelper.kt b/tests/FlickerTests/src/com/android/server/wm/flicker/helpers/ImeAppAutoFocusHelper.kt index 93b987ea8787a..aacc17a49a242 100644 --- a/tests/FlickerTests/src/com/android/server/wm/flicker/helpers/ImeAppAutoFocusHelper.kt +++ b/tests/FlickerTests/src/com/android/server/wm/flicker/helpers/ImeAppAutoFocusHelper.kt @@ -16,6 +16,10 @@ package com.android.server.wm.flicker.helpers +import android.view.WindowInsets.Type.ime +import android.view.WindowInsets.Type.navigationBars +import android.view.WindowInsets.Type.statusBars + import android.app.Instrumentation import androidx.test.uiautomator.By import androidx.test.uiautomator.UiDevice @@ -25,6 +29,8 @@ import com.android.server.wm.traces.common.FlickerComponentName import com.android.server.wm.traces.parser.toFlickerComponent import com.android.server.wm.traces.parser.windowmanager.WindowManagerStateHelper +import java.util.regex.Pattern + class ImeAppAutoFocusHelper @JvmOverloads constructor( instr: Instrumentation, private val rotation: Int, @@ -72,6 +78,7 @@ class ImeAppAutoFocusHelper @JvmOverloads constructor( wmHelper.waitForAppTransitionIdle() wmHelper.waitForFullScreenApp( ActivityOptions.DIALOG_THEMED_ACTIVITY_COMPONENT_NAME.toFlickerComponent()) + mInstrumentation.waitForIdleSync() } fun dismissDialog(wmHelper: WindowManagerStateHelper) { val dialog = uiDevice.wait( @@ -83,4 +90,27 @@ class ImeAppAutoFocusHelper @JvmOverloads constructor( wmHelper.waitForAppTransitionIdle() } } + fun getInsetsVisibleFromDialog(type: Int): Boolean { + var insetsVisibilityTextView = uiDevice.wait( + Until.findObject(By.res("android:id/text1")), FIND_TIMEOUT) + if (insetsVisibilityTextView != null) { + var visibility = insetsVisibilityTextView.text.toString() + val matcher = when (type) { + ime() -> { + Pattern.compile("IME\\: (VISIBLE|INVISIBLE)").matcher(visibility) + } + statusBars() -> { + Pattern.compile("StatusBar\\: (VISIBLE|INVISIBLE)").matcher(visibility) + } + navigationBars() -> { + Pattern.compile("NavBar\\: (VISIBLE|INVISIBLE)").matcher(visibility) + } + else -> null + } + if (matcher != null && matcher.find()) { + return matcher.group(1).equals("VISIBLE") + } + } + return false + } } diff --git a/tests/FlickerTests/src/com/android/server/wm/flicker/ime/LaunchAppShowImeAndDialogThemeAppTest.kt b/tests/FlickerTests/src/com/android/server/wm/flicker/ime/LaunchAppShowImeAndDialogThemeAppTest.kt index 1b60403ac354e..2f8f9441a7b9c 100644 --- a/tests/FlickerTests/src/com/android/server/wm/flicker/ime/LaunchAppShowImeAndDialogThemeAppTest.kt +++ b/tests/FlickerTests/src/com/android/server/wm/flicker/ime/LaunchAppShowImeAndDialogThemeAppTest.kt @@ -16,6 +16,10 @@ package com.android.server.wm.flicker.ime +import android.view.WindowInsets.Type.ime +import android.view.WindowInsets.Type.navigationBars +import android.view.WindowInsets.Type.statusBars + import android.app.Instrumentation import android.platform.test.annotations.Presubmit import android.view.Surface @@ -35,6 +39,8 @@ import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.MethodSorters import org.junit.runners.Parameterized +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue /** * Test IME snapshot mechanism won't apply when transitioning from non-IME focused dialog activity. @@ -56,6 +62,10 @@ class LaunchAppShowImeAndDialogThemeAppTest(private val testSpec: FlickerTestPar testApp.launchViaIntent(wmHelper) wmHelper.waitImeShown() testApp.startDialogThemedActivity(wmHelper) + // Verify IME insets isn't visible on dialog since it's non-IME focusable window + assertFalse(testApp.getInsetsVisibleFromDialog(ime())) + assertTrue(testApp.getInsetsVisibleFromDialog(statusBars())) + assertTrue(testApp.getInsetsVisibleFromDialog(navigationBars())) } } teardown { diff --git a/tests/FlickerTests/test-apps/flickerapp/src/com/android/server/wm/flicker/testapp/DialogThemedActivity.java b/tests/FlickerTests/test-apps/flickerapp/src/com/android/server/wm/flicker/testapp/DialogThemedActivity.java index 27606d81f9d38..20eb295d3e6b3 100644 --- a/tests/FlickerTests/test-apps/flickerapp/src/com/android/server/wm/flicker/testapp/DialogThemedActivity.java +++ b/tests/FlickerTests/test-apps/flickerapp/src/com/android/server/wm/flicker/testapp/DialogThemedActivity.java @@ -17,11 +17,16 @@ package com.android.server.wm.flicker.testapp; import static android.view.ViewGroup.LayoutParams.MATCH_PARENT; +import static android.view.WindowInsets.Type.ime; +import static android.view.WindowInsets.Type.navigationBars; +import static android.view.WindowInsets.Type.statusBars; import static android.view.WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM; import static android.view.WindowManager.LayoutParams.FLAG_DIM_BEHIND; +import static android.view.WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE; import android.app.Activity; import android.app.AlertDialog; +import android.app.Dialog; import android.graphics.Color; import android.os.Bundle; import android.view.WindowManager; @@ -33,9 +38,12 @@ public class DialogThemedActivity extends Activity { public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setContentView(R.layout.activity_simple); + getWindow().addFlags(FLAG_NOT_FOCUSABLE); getWindow().getDecorView().setBackgroundColor(Color.TRANSPARENT); TextView textView = new TextView(this); - textView.setText("This is a test dialog"); + // Print SystemBars' insets visibility on this window for demonstrating during the test. + textView.setId(android.R.id.text1); + textView.setText("Insets visibility\n\n"); textView.setTextColor(Color.BLACK); LinearLayout layout = new LinearLayout(this); layout.setBackgroundColor(Color.GREEN); @@ -51,7 +59,17 @@ public class DialogThemedActivity extends Activity { attrs.flags = FLAG_DIM_BEHIND | FLAG_ALT_FOCUSABLE_IM; dialog.getWindow().getDecorView().setLayoutParams(attrs); dialog.setCanceledOnTouchOutside(true); + dialog.setOnShowListener(d -> textView.setText(textView.getText() + + "IME: " + isInsetsVisible(dialog, ime()) + "\n" + + "StatusBar: " + isInsetsVisible(dialog, statusBars()) + "\n" + + "NavBar: " + isInsetsVisible(dialog, navigationBars()) + "\n") + ); dialog.show(); dialog.setOnDismissListener((d) -> finish()); } + + private String isInsetsVisible(Dialog d, int type) { + return d.getWindow().getDecorView().getRootWindowInsets().isVisible(type) ? "VISIBLE" + : "INVISIBLE"; + } }