From 5a810c8403eeafc6e86084c6b0ce11c96908a30d Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Wed, 17 Mar 2021 12:33:15 +0800 Subject: [PATCH] Don't skip IME animation when backed to Task has no editor focus When the dialer app previously shows IME, if the user swipe out the dialer task to launcher or switching task and then navigating back to dialer task, will see IME surface will fade-out because the editor view has been cleared the focus. The issue happens when user taping editText to show keyboard and the IME show animaion will be skipped. It could be related the CL[1] that we skip the animation once when the task snapshot has the IME surface, but we should not skip IME show animation if the launching task has been cleared the editor focus. [1]: I83ffc03119e01da71ad12f8ad8043cf0730dfd50 Fix: 183347520 Bug: 166736352 Test: manual as steps in commit message Test: atest ImeInsetsSourceConsumerTest#\ testImeGetAndClearSkipAnimationOnce_expectNoSkip Change-Id: Ib140801f1ce03b5566e756914f96dacba3ad8892 --- .../android/view/ImeInsetsSourceConsumer.java | 4 +- core/java/android/view/InsetsController.java | 9 +-- .../android/view/InsetsSourceConsumer.java | 12 +++- core/java/android/view/ViewRootImpl.java | 6 +- .../view/ImeInsetsSourceConsumerTest.java | 58 +++++++++++++++---- .../android/view/InsetsControllerTest.java | 4 +- 6 files changed, 69 insertions(+), 24 deletions(-) diff --git a/core/java/android/view/ImeInsetsSourceConsumer.java b/core/java/android/view/ImeInsetsSourceConsumer.java index 98b7dbfa670fc..f401ad9346c6c 100644 --- a/core/java/android/view/ImeInsetsSourceConsumer.java +++ b/core/java/android/view/ImeInsetsSourceConsumer.java @@ -51,8 +51,8 @@ public final class ImeInsetsSourceConsumer extends InsetsSourceConsumer { } @Override - public void onWindowFocusGained() { - super.onWindowFocusGained(); + public void onWindowFocusGained(boolean hasViewFocus) { + super.onWindowFocusGained(hasViewFocus); getImm().registerImeConsumer(this); } diff --git a/core/java/android/view/InsetsController.java b/core/java/android/view/InsetsController.java index a68f528837c08..c001ec9da3a46 100644 --- a/core/java/android/view/InsetsController.java +++ b/core/java/android/view/InsetsController.java @@ -1302,8 +1302,8 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation /** * Called when current window gains focus. */ - public void onWindowFocusGained() { - getSourceConsumer(ITYPE_IME).onWindowFocusGained(); + public void onWindowFocusGained(boolean hasViewFocused) { + getSourceConsumer(ITYPE_IME).onWindowFocusGained(hasViewFocused); } /** @@ -1366,8 +1366,9 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation final InsetsSourceControl imeControl = consumer != null ? consumer.getControl() : null; // Skip showing animation once that made by system for some reason. // (e.g. starting window with IME snapshot) - if (imeControl != null && show) { - skipAnim = imeControl.getAndClearSkipAnimationOnce(); + if (imeControl != null) { + skipAnim = imeControl.getAndClearSkipAnimationOnce() && show + && consumer.hasViewFocusWhenWindowFocusGain(); } } applyAnimation(types, show, fromIme, skipAnim); diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java index fd1c3b82ca2ce..bc50dbe311b91 100644 --- a/core/java/android/view/InsetsSourceConsumer.java +++ b/core/java/android/view/InsetsSourceConsumer.java @@ -81,6 +81,11 @@ public class InsetsSourceConsumer { private final Supplier mTransactionSupplier; private @Nullable InsetsSourceControl mSourceControl; private boolean mHasWindowFocus; + + /** + * Whether the view has focus returned by {@link #onWindowFocusGained(boolean)}. + */ + private boolean mHasViewFocusWhenWindowFocusGain; private Rect mPendingFrame; private Rect mPendingVisibleFrame; @@ -223,8 +228,9 @@ public class InsetsSourceConsumer { /** * Called when current window gains focus */ - public void onWindowFocusGained() { + public void onWindowFocusGained(boolean hasViewFocus) { mHasWindowFocus = true; + mHasViewFocusWhenWindowFocusGain = hasViewFocus; } /** @@ -238,6 +244,10 @@ public class InsetsSourceConsumer { return mHasWindowFocus; } + boolean hasViewFocusWhenWindowFocusGain() { + return mHasViewFocusWhenWindowFocusGain; + } + boolean applyLocalVisibilityOverride() { final InsetsSource source = mState.peekSource(mType); final boolean isVisible = source != null ? source.isVisible() : getDefaultVisibility(mType); diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index e4fb61107c4ac..a56f6b7e0f596 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -3354,8 +3354,9 @@ public final class ViewRootImpl implements ViewParent, } // TODO (b/131181940): Make sure this doesn't leak Activity with mActivityConfigCallback // config changes. + final View focusedView = mView != null ? mView.findFocus() : null; if (hasWindowFocus) { - mInsetsController.onWindowFocusGained(); + mInsetsController.onWindowFocusGained(focusedView != null /* hasViewFocused */); } else { mInsetsController.onWindowFocusLost(); } @@ -3404,8 +3405,7 @@ public final class ViewRootImpl implements ViewParent, // Note: must be done after the focus change callbacks, // so all of the view state is set up correctly. - mImeFocusController.onPostWindowFocus(mView != null ? mView.findFocus() : null, - hasWindowFocus, mWindowAttributes); + mImeFocusController.onPostWindowFocus(focusedView, hasWindowFocus, mWindowAttributes); if (hasWindowFocus) { // Clear the forward bit. We can just do this directly, since diff --git a/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java index 47556c370f220..34a1fd89e4f88 100644 --- a/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/ImeInsetsSourceConsumerTest.java @@ -97,7 +97,7 @@ public class ImeInsetsSourceConsumerTest { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { // test if setVisibility can show IME - mImeConsumer.onWindowFocusGained(); + mImeConsumer.onWindowFocusGained(true); mController.show(WindowInsets.Type.ime(), true /* fromIme */); mController.cancelExistingAnimations(); assertTrue(mController.getSourceConsumer(ime.getType()).isRequestedVisible()); @@ -116,7 +116,7 @@ public class ImeInsetsSourceConsumerTest { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { // Request IME visible before control is available. - mImeConsumer.onWindowFocusGained(); + mImeConsumer.onWindowFocusGained(true); mController.show(WindowInsets.Type.ime(), true /* fromIme */); // set control and verify visibility is applied. @@ -132,24 +132,58 @@ public class ImeInsetsSourceConsumerTest { } @Test - public void testImeGetAndClearSkipAnimationOnce() { + public void testImeGetAndClearSkipAnimationOnce_expectSkip() { + // Expect IME animation will skipped when the IME is visible at first place. + verifyImeGetAndClearSkipAnimationOnce(true /* hasWindowFocus */, true /* hasViewFocus */, + true /* expectSkipAnim */); + } + + @Test + public void testImeGetAndClearSkipAnimationOnce_expectNoSkip() { + // Expect IME animation will not skipped if previously no view focused when gained the + // window focus and requesting the IME visible next time. + verifyImeGetAndClearSkipAnimationOnce(true /* hasWindowFocus */, false /* hasViewFocus */, + false /* expectSkipAnim */); + } + + private void verifyImeGetAndClearSkipAnimationOnce(boolean hasWindowFocus, boolean hasViewFocus, + boolean expectSkipAnim) { InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { // Request IME visible before control is available. - mImeConsumer.onWindowFocusGained(); - mController.show(WindowInsets.Type.ime(), true /* fromIme */); + mImeConsumer.onWindowFocusGained(hasWindowFocus); + final boolean imeVisible = hasWindowFocus && hasViewFocus; + if (imeVisible) { + mController.show(WindowInsets.Type.ime(), true /* fromIme */); + } // set control and verify visibility is applied. InsetsSourceControl control = Mockito.spy( new InsetsSourceControl(ITYPE_IME, mLeash, new Point(), Insets.NONE)); // Simulate IME source control set this flag when the target has starting window. control.setSkipAnimationOnce(true); - mController.onControlsChanged(new InsetsSourceControl[] { control }); - // Verify IME show animation should be triggered when control becomes available and - // the animation will be skipped by getAndClearSkipAnimationOnce invoked. - verify(control).getAndClearSkipAnimationOnce(); - verify(mController).applyAnimation( - eq(WindowInsets.Type.ime()), eq(true) /* show */, eq(false) /* fromIme */, - eq(true) /* skipAnim */); + + if (imeVisible) { + // Verify IME applyAnimation should be triggered when control becomes available, + // and expect skip animation state after getAndClearSkipAnimationOnce invoked. + mController.onControlsChanged(new InsetsSourceControl[]{ control }); + verify(control).getAndClearSkipAnimationOnce(); + verify(mController).applyAnimation(eq(WindowInsets.Type.ime()), + eq(true) /* show */, eq(false) /* fromIme */, + eq(expectSkipAnim) /* skipAnim */); + } + + // If previously hasViewFocus is false, verify when requesting the IME visible next + // time will not skip animation. + if (!hasViewFocus) { + mController.show(WindowInsets.Type.ime(), true); + mController.onControlsChanged(new InsetsSourceControl[]{ control }); + // Verify IME show animation should be triggered when control becomes available and + // the animation will be skipped by getAndClearSkipAnimationOnce invoked. + verify(control).getAndClearSkipAnimationOnce(); + verify(mController).applyAnimation(eq(WindowInsets.Type.ime()), + eq(true) /* show */, eq(true) /* fromIme */, + eq(false) /* skipAnim */); + } }); } } diff --git a/core/tests/coretests/src/android/view/InsetsControllerTest.java b/core/tests/coretests/src/android/view/InsetsControllerTest.java index 4390546559b42..6301f32169f71 100644 --- a/core/tests/coretests/src/android/view/InsetsControllerTest.java +++ b/core/tests/coretests/src/android/view/InsetsControllerTest.java @@ -234,7 +234,7 @@ public class InsetsControllerTest { InsetsSourceControl ime = controls[2]; InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { - mController.getSourceConsumer(ITYPE_IME).onWindowFocusGained(); + mController.getSourceConsumer(ITYPE_IME).onWindowFocusGained(true); // since there is no focused view, forcefully make IME visible. mController.show(Type.ime(), true /* fromIme */); mController.show(Type.all()); @@ -260,7 +260,7 @@ public class InsetsControllerTest { InsetsSourceControl ime = createControl(ITYPE_IME); mController.onControlsChanged(new InsetsSourceControl[] { ime }); InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { - mController.getSourceConsumer(ITYPE_IME).onWindowFocusGained(); + mController.getSourceConsumer(ITYPE_IME).onWindowFocusGained(true); mController.show(Type.ime(), true /* fromIme */); mController.cancelExistingAnimations(); assertTrue(mController.getSourceConsumer(ime.getType()).isRequestedVisible());