From 618dbe022ff715cadff46e1d4102b2f0715e85ea Mon Sep 17 00:00:00 2001 From: Tiger Huang Date: Fri, 19 Jun 2020 00:12:55 +0800 Subject: [PATCH] Disable user animations on insets whose visible frame is empty (refined) Floating IME or fullscreen IME won't cause insets (except the area overlapped with navigation bar). It doesn't make much sense to let apps move the IME at these cases. Fix: 157777145 Test: atest InsetsSourceConsumerTest GlobalActionsImeTest ImeInsetsControllerTest Change-Id: Id70f59be7653beedc02d6c8bc3b1bd50a357f4fe --- .../InputMethodService.java | 4 + core/java/android/view/InsetsController.java | 75 ++++++++++++++++--- core/java/android/view/InsetsSource.java | 5 ++ .../android/view/InsetsSourceConsumer.java | 8 +- .../view/InsetsSourceConsumerTest.java | 18 +---- .../globalactions/GlobalActionsImeTest.java | 16 +++- 6 files changed, 95 insertions(+), 31 deletions(-) diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java index d8b1f41c86d5d..5647bf90d2fbb 100644 --- a/core/java/android/inputmethodservice/InputMethodService.java +++ b/core/java/android/inputmethodservice/InputMethodService.java @@ -471,6 +471,10 @@ public class InputMethodService extends AbstractInputMethodService { final ViewTreeObserver.OnComputeInternalInsetsListener mInsetsComputer = info -> { onComputeInsets(mTmpInsets); + if (!mViewsCreated) { + // The IME views are not ready, keep visible insets untouched. + mTmpInsets.visibleTopInsets = 0; + } if (isExtractViewShown()) { // In true fullscreen mode, we just say the window isn't covering // any content so we don't impact whatever is behind. diff --git a/core/java/android/view/InsetsController.java b/core/java/android/view/InsetsController.java index dd48d554f296b..0e250c4849b57 100644 --- a/core/java/android/view/InsetsController.java +++ b/core/java/android/view/InsetsController.java @@ -492,6 +492,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation /** Set of inset types for which an animation was started since last resetting this field */ private @InsetsType int mLastStartedAnimTypes; + /** Set of inset types which cannot be controlled by the user animation */ + private @InsetsType int mDisabledUserAnimationInsetsTypes; + + private Runnable mInvokeControllableInsetsChangedListeners = + this::invokeControllableInsetsChangedListeners; + public InsetsController(Host host) { this(host, (controller, type) -> { if (type == ITYPE_IME) { @@ -606,22 +612,57 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation private void updateState(InsetsState newState) { mState.setDisplayFrame(newState.getDisplayFrame()); - for (int i = 0; i < InsetsState.SIZE; i++) { - InsetsSource source = newState.peekSource(i); - if (source == null) continue;; - getSourceConsumer(source.getType()).updateSource(source); - } - for (int i = 0; i < InsetsState.SIZE; i++) { - InsetsSource source = mState.peekSource(i); + @InsetsType int disabledUserAnimationTypes = 0; + @InsetsType int[] cancelledUserAnimationTypes = {0}; + for (@InternalInsetsType int type = 0; type < InsetsState.SIZE; type++) { + InsetsSource source = newState.peekSource(type); if (source == null) continue; - if (newState.peekSource(source.getType()) == null) { - mState.removeSource(source.getType()); + @AnimationType int animationType = getAnimationType(type); + if (!source.isUserControllable()) { + @InsetsType int insetsType = toPublicType(type); + // The user animation is not allowed when visible frame is empty. + disabledUserAnimationTypes |= insetsType; + if (animationType == ANIMATION_TYPE_USER) { + // Existing user animation needs to be cancelled. + animationType = ANIMATION_TYPE_NONE; + cancelledUserAnimationTypes[0] |= insetsType; + } + } + getSourceConsumer(type).updateSource(source, animationType); + } + for (@InternalInsetsType int type = 0; type < InsetsState.SIZE; type++) { + InsetsSource source = mState.peekSource(type); + if (source == null) continue; + if (newState.peekSource(type) == null) { + mState.removeSource(type); } } if (mCaptionInsetsHeight != 0) { mState.getSource(ITYPE_CAPTION_BAR).setFrame(new Rect(mFrame.left, mFrame.top, mFrame.right, mFrame.top + mCaptionInsetsHeight)); } + + updateDisabledUserAnimationTypes(disabledUserAnimationTypes); + + if (cancelledUserAnimationTypes[0] != 0) { + mHandler.post(() -> show(cancelledUserAnimationTypes[0])); + } + } + + private void updateDisabledUserAnimationTypes(@InsetsType int disabledUserAnimationTypes) { + @InsetsType int diff = mDisabledUserAnimationInsetsTypes ^ disabledUserAnimationTypes; + if (diff != 0) { + for (int i = mSourceConsumers.size() - 1; i >= 0; i--) { + InsetsSourceConsumer consumer = mSourceConsumers.valueAt(i); + if (consumer.getControl() != null + && (toPublicType(consumer.getType()) & diff) != 0) { + mHandler.removeCallbacks(mInvokeControllableInsetsChangedListeners); + mHandler.post(mInvokeControllableInsetsChangedListeners); + break; + } + } + mDisabledUserAnimationInsetsTypes = disabledUserAnimationTypes; + } } private boolean captionInsetsUnchanged() { @@ -825,6 +866,18 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation + " while an existing " + Type.toString(mTypesBeingCancelled) + " is being cancelled."); } + if (animationType == ANIMATION_TYPE_USER) { + final @InsetsType int disabledTypes = types & mDisabledUserAnimationInsetsTypes; + if (DEBUG) Log.d(TAG, "user animation disabled types: " + disabledTypes); + types &= ~mDisabledUserAnimationInsetsTypes; + + if (fromIme && (disabledTypes & ime()) != 0 + && !mState.getSource(ITYPE_IME).isVisible()) { + // We've requested IMM to show IME, but the IME is not controllable. We need to + // cancel the request. + getSourceConsumer(ITYPE_IME).hide(true, animationType); + } + } if (types == 0) { // nothing to animate. listener.onCancelled(null); @@ -1297,7 +1350,8 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation @InsetsType int result = 0; for (int i = mSourceConsumers.size() - 1; i >= 0; i--) { InsetsSourceConsumer consumer = mSourceConsumers.valueAt(i); - if (consumer.getControl() != null) { + InsetsSource source = mState.peekSource(consumer.mType); + if (consumer.getControl() != null && source != null && source.isUserControllable()) { result |= toPublicType(consumer.mType); } } @@ -1308,6 +1362,7 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation * @return The types that are now animating due to a listener invoking control/show/hide */ private @InsetsType int invokeControllableInsetsChangedListeners() { + mHandler.removeCallbacks(mInvokeControllableInsetsChangedListeners); mLastStartedAnimTypes = 0; @InsetsType int types = calculateControllableTypes(); int size = mControllableInsetsChangedListeners.size(); diff --git a/core/java/android/view/InsetsSource.java b/core/java/android/view/InsetsSource.java index 15b9a93303924..dbf75705c073f 100644 --- a/core/java/android/view/InsetsSource.java +++ b/core/java/android/view/InsetsSource.java @@ -92,6 +92,11 @@ public class InsetsSource implements Parcelable { return mVisible; } + boolean isUserControllable() { + // If mVisibleFrame is null, it will be the same area as mFrame. + return mVisibleFrame == null || !mVisibleFrame.isEmpty(); + } + /** * Calculates the insets this source will cause to a client window. * diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java index 565846638acc0..8d92d7b5ab20e 100644 --- a/core/java/android/view/InsetsSourceConsumer.java +++ b/core/java/android/view/InsetsSourceConsumer.java @@ -18,8 +18,8 @@ package android.view; import static android.view.InsetsController.ANIMATION_TYPE_NONE; import static android.view.InsetsController.AnimationType; -import static android.view.InsetsState.getDefaultVisibility; import static android.view.InsetsController.DEBUG; +import static android.view.InsetsState.getDefaultVisibility; import static android.view.InsetsState.toPublicType; import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE; @@ -275,9 +275,9 @@ public class InsetsSourceConsumer { } @VisibleForTesting(visibility = PACKAGE) - public void updateSource(InsetsSource newSource) { + public void updateSource(InsetsSource newSource, @AnimationType int animationType) { InsetsSource source = mState.peekSource(mType); - if (source == null || mController.getAnimationType(mType) == ANIMATION_TYPE_NONE + if (source == null || animationType == ANIMATION_TYPE_NONE || source.getFrame().equals(newSource.getFrame())) { mPendingFrame = null; mPendingVisibleFrame = null; @@ -286,7 +286,7 @@ public class InsetsSourceConsumer { } // Frame is changing while animating. Keep note of the new frame but keep existing frame - // until animaition is finished. + // until animation is finished. newSource = new InsetsSource(newSource); mPendingFrame = new Rect(newSource.getFrame()); mPendingVisibleFrame = newSource.getVisibleFrame() != null diff --git a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java index bf7f339a8484e..1b3272572db04 100644 --- a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java @@ -26,13 +26,11 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.TestCase.assertFalse; import static junit.framework.TestCase.assertTrue; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; import android.app.Instrumentation; import android.content.Context; @@ -135,37 +133,29 @@ public class InsetsSourceConsumerTest { InsetsSourceConsumer consumer = new InsetsSourceConsumer( ITYPE_IME, state, null, controller); - when(controller.getAnimationType(anyInt())).thenReturn(ANIMATION_TYPE_NONE); - InsetsSource source = new InsetsSource(ITYPE_IME); source.setFrame(0, 1, 2, 3); - consumer.updateSource(new InsetsSource(source)); - - when(controller.getAnimationType(anyInt())).thenReturn(ANIMATION_TYPE_USER); + consumer.updateSource(new InsetsSource(source), ANIMATION_TYPE_NONE); // While we're animating, updates are delayed source.setFrame(4, 5, 6, 7); - consumer.updateSource(new InsetsSource(source)); + consumer.updateSource(new InsetsSource(source), ANIMATION_TYPE_USER); assertEquals(new Rect(0, 1, 2, 3), state.peekSource(ITYPE_IME).getFrame()); // Finish the animation, now the pending frame should be applied - when(controller.getAnimationType(anyInt())).thenReturn(ANIMATION_TYPE_NONE); assertTrue(consumer.notifyAnimationFinished()); assertEquals(new Rect(4, 5, 6, 7), state.peekSource(ITYPE_IME).getFrame()); - when(controller.getAnimationType(anyInt())).thenReturn(ANIMATION_TYPE_USER); - // Animating again, updates are delayed source.setFrame(8, 9, 10, 11); - consumer.updateSource(new InsetsSource(source)); + consumer.updateSource(new InsetsSource(source), ANIMATION_TYPE_USER); assertEquals(new Rect(4, 5, 6, 7), state.peekSource(ITYPE_IME).getFrame()); // Updating with the current frame triggers a different code path, verify this clears // the pending 8, 9, 10, 11 frame: source.setFrame(4, 5, 6, 7); - consumer.updateSource(new InsetsSource(source)); + consumer.updateSource(new InsetsSource(source), ANIMATION_TYPE_USER); - when(controller.getAnimationType(anyInt())).thenReturn(ANIMATION_TYPE_NONE); assertFalse(consumer.notifyAnimationFinished()); assertEquals(new Rect(4, 5, 6, 7), state.peekSource(ITYPE_IME).getFrame()); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsImeTest.java b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsImeTest.java index f38c722ad2db8..c646f33a226c3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsImeTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/globalactions/GlobalActionsImeTest.java @@ -18,6 +18,7 @@ package com.android.systemui.globalactions; import static android.view.WindowInsets.Type.ime; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -39,6 +40,7 @@ import androidx.test.rule.ActivityTestRule; import com.android.systemui.SysuiTestCase; +import org.junit.After; import org.junit.Rule; import org.junit.Test; @@ -52,6 +54,11 @@ public class GlobalActionsImeTest extends SysuiTestCase { public ActivityTestRule mActivityTestRule = new ActivityTestRule<>( TestActivity.class, false, false); + @After + public void tearDown() { + executeShellCommand("input keyevent HOME"); + } + /** * This test verifies that GlobalActions, which is frequently used to capture bugreports, * doesn't interfere with the IME, i.e. soft-keyboard state. @@ -63,6 +70,9 @@ public class GlobalActionsImeTest extends SysuiTestCase { waitUntil("Ime is visible", activity::isImeVisible); + // In some cases, IME is not controllable. e.g., floating IME or fullscreen IME. + final boolean activityControlledIme = activity.mControlsIme; + executeShellCommand("input keyevent --longpress POWER"); waitUntil("activity loses focus", () -> !activity.mHasFocus); @@ -72,9 +82,9 @@ public class GlobalActionsImeTest extends SysuiTestCase { runAssertionOnMainThread(() -> { assertTrue("IME should remain visible behind GlobalActions, but didn't", - activity.mControlsIme); - assertTrue("App behind GlobalActions should remain in control of IME, but didn't", activity.mImeVisible); + assertEquals("App behind GlobalActions should remain in control of IME, but didn't", + activityControlledIme, activity.mControlsIme); }); } @@ -167,7 +177,7 @@ public class GlobalActionsImeTest extends SysuiTestCase { } boolean isImeVisible() { - return mHasFocus && mControlsIme && mImeVisible; + return mHasFocus && mImeVisible; } @Override