From 1fd078151b0f1a0d69d6235f3da21505123bea26 Mon Sep 17 00:00:00 2001 From: Bill Lin Date: Tue, 3 Nov 2020 16:26:00 +0800 Subject: [PATCH] Re-map DisplayAreaAppearedInfo to token-leash map Clean-up DA callback APIs of OneHandedDisplayAreaOrganizer onDisplayAreaAppeared() onDisplayAreaVanished() Note: When super.unregisterOrganizer() is called, all DAs should be removed from the map in the callback of onDisplayAreaVanished(). It was not working correctly because the map is using DisplayAreaInfo as the key, which will be different because of the different configuration object. We should use the DA token as the key. Bug: b/172298357 Test: atest WMShellUnitTests:OneHandedDisplayAreaOrganizerTest Test: atest SystemUITests Test: Trigger OHM, rotate screen Test: Trigger OHM, disabled through settings Change-Id: I4560df1ab36ec33292fa4ec08cab28644b0fabe2 --- .../OneHandedAnimationController.java | 40 ++++++------ .../OneHandedDisplayAreaOrganizer.java | 61 ++++++++----------- .../OneHandedAnimationControllerTest.java | 5 +- .../OneHandedDisplayAreaOrganizerTest.java | 11 ++-- 4 files changed, 57 insertions(+), 60 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedAnimationController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedAnimationController.java index d22abe4dd19b0..125e322974bf1 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedAnimationController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedAnimationController.java @@ -24,6 +24,7 @@ import android.graphics.Rect; import android.view.SurfaceControl; import android.view.animation.Interpolator; import android.view.animation.OvershootInterpolator; +import android.window.WindowContainerToken; import androidx.annotation.VisibleForTesting; @@ -55,7 +56,7 @@ public class OneHandedAnimationController { private final Interpolator mOvershootInterpolator; private final OneHandedSurfaceTransactionHelper mSurfaceTransactionHelper; - private final HashMap mAnimatorMap = + private final HashMap mAnimatorMap = new HashMap<>(); /** @@ -67,23 +68,23 @@ public class OneHandedAnimationController { } @SuppressWarnings("unchecked") - OneHandedTransitionAnimator getAnimator(SurfaceControl leash, Rect startBounds, - Rect endBounds) { - final OneHandedTransitionAnimator animator = mAnimatorMap.get(leash); + OneHandedTransitionAnimator getAnimator(WindowContainerToken token, SurfaceControl leash, + Rect startBounds, Rect endBounds) { + final OneHandedTransitionAnimator animator = mAnimatorMap.get(token); if (animator == null) { - mAnimatorMap.put(leash, setupOneHandedTransitionAnimator( - OneHandedTransitionAnimator.ofBounds(leash, startBounds, endBounds))); + mAnimatorMap.put(token, setupOneHandedTransitionAnimator( + OneHandedTransitionAnimator.ofBounds(token, leash, startBounds, endBounds))); } else if (animator.isRunning()) { animator.updateEndValue(endBounds); } else { animator.cancel(); - mAnimatorMap.put(leash, setupOneHandedTransitionAnimator( - OneHandedTransitionAnimator.ofBounds(leash, startBounds, endBounds))); + mAnimatorMap.put(token, setupOneHandedTransitionAnimator( + OneHandedTransitionAnimator.ofBounds(token, leash, startBounds, endBounds))); } - return mAnimatorMap.get(leash); + return mAnimatorMap.get(token); } - HashMap getAnimatorMap() { + HashMap getAnimatorMap() { return mAnimatorMap; } @@ -91,8 +92,8 @@ public class OneHandedAnimationController { return mAnimatorMap.isEmpty(); } - void removeAnimator(SurfaceControl key) { - final OneHandedTransitionAnimator animator = mAnimatorMap.remove(key); + void removeAnimator(WindowContainerToken token) { + final OneHandedTransitionAnimator animator = mAnimatorMap.remove(token); if (animator != null && animator.isRunning()) { animator.cancel(); } @@ -116,6 +117,7 @@ public class OneHandedAnimationController { ValueAnimator.AnimatorListener { private final SurfaceControl mLeash; + private final WindowContainerToken mToken; private T mStartValue; private T mEndValue; private T mCurrentValue; @@ -128,8 +130,10 @@ public class OneHandedAnimationController { private @TransitionDirection int mTransitionDirection; - private OneHandedTransitionAnimator(SurfaceControl leash, T startValue, T endValue) { + private OneHandedTransitionAnimator(WindowContainerToken token, SurfaceControl leash, + T startValue, T endValue) { mLeash = leash; + mToken = token; mStartValue = startValue; mEndValue = endValue; addListener(this); @@ -208,8 +212,8 @@ public class OneHandedAnimationController { return this; } - SurfaceControl getLeash() { - return mLeash; + WindowContainerToken getToken() { + return mToken; } Rect getDestinationBounds() { @@ -254,10 +258,10 @@ public class OneHandedAnimationController { } @VisibleForTesting - static OneHandedTransitionAnimator ofBounds(SurfaceControl leash, - Rect startValue, Rect endValue) { + static OneHandedTransitionAnimator ofBounds(WindowContainerToken token, + SurfaceControl leash, Rect startValue, Rect endValue) { - return new OneHandedTransitionAnimator(leash, new Rect(startValue), + return new OneHandedTransitionAnimator(token, leash, new Rect(startValue), new Rect(endValue)) { private final Rect mTmpRect = new Rect(); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizer.java b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizer.java index d2d5591100d4c..1da72f8efbb8c 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizer.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizer.java @@ -26,11 +26,11 @@ import android.graphics.Point; import android.graphics.Rect; import android.os.SystemProperties; import android.util.ArrayMap; -import android.util.Log; import android.view.SurfaceControl; import android.window.DisplayAreaAppearedInfo; import android.window.DisplayAreaInfo; import android.window.DisplayAreaOrganizer; +import android.window.WindowContainerToken; import android.window.WindowContainerTransaction; import androidx.annotation.NonNull; @@ -44,8 +44,6 @@ import com.android.wm.shell.common.ShellExecutor; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; -import java.util.Objects; -import java.util.concurrent.Executor; /** * Manages OneHanded display areas such as offset. @@ -69,7 +67,7 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { private int mEnterExitAnimationDurationMs; @VisibleForTesting - ArrayMap mDisplayAreaMap = new ArrayMap(); + ArrayMap mDisplayAreaTokenMap = new ArrayMap(); private DisplayController mDisplayController; private OneHandedAnimationController mAnimationController; private OneHandedSurfaceTransactionHelper.SurfaceControlTransactionFactory @@ -89,7 +87,7 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { @Override public void onOneHandedAnimationEnd(SurfaceControl.Transaction tx, OneHandedAnimationController.OneHandedTransitionAnimator animator) { - mAnimationController.removeAnimator(animator.getLeash()); + mAnimationController.removeAnimator(animator.getToken()); if (mAnimationController.isAnimatorsConsumed()) { finishOffset(animator.getDestinationOffset(), animator.getTransitionDirection()); @@ -99,7 +97,7 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { @Override public void onOneHandedAnimationCancel( OneHandedAnimationController.OneHandedTransitionAnimator animator) { - mAnimationController.removeAnimator(animator.getLeash()); + mAnimationController.removeAnimator(animator.getToken()); if (mAnimationController.isAnimatorsConsumed()) { finishOffset(animator.getDestinationOffset(), animator.getTransitionDirection()); @@ -119,7 +117,6 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { super(mainExecutor); mAnimationController = animationController; mDisplayController = displayController; - mDefaultDisplayBounds.set(getDisplayBounds()); mLastVisualDisplayBounds.set(getDisplayBounds()); final int animationDurationConfig = context.getResources().getInteger( R.integer.config_one_handed_translate_animation_duration); @@ -134,24 +131,12 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { @Override public void onDisplayAreaAppeared(@NonNull DisplayAreaInfo displayAreaInfo, @NonNull SurfaceControl leash) { - Objects.requireNonNull(displayAreaInfo, "displayAreaInfo must not be null"); - Objects.requireNonNull(leash, "leash must not be null"); - if (mDisplayAreaMap.get(displayAreaInfo) == null) { - // mDefaultDisplayBounds may out of date after removeDisplayChangingController() - mDefaultDisplayBounds.set(getDisplayBounds()); - mDisplayAreaMap.put(displayAreaInfo, leash); - } + mDisplayAreaTokenMap.put(displayAreaInfo.token, leash); } @Override public void onDisplayAreaVanished(@NonNull DisplayAreaInfo displayAreaInfo) { - Objects.requireNonNull(displayAreaInfo, - "Requires valid displayArea, and displayArea must not be null"); - if (!mDisplayAreaMap.containsKey(displayAreaInfo)) { - Log.w(TAG, "Unrecognized token: " + displayAreaInfo.token); - return; - } - mDisplayAreaMap.remove(displayAreaInfo); + mDisplayAreaTokenMap.remove(displayAreaInfo.token); } @Override @@ -162,6 +147,7 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { final DisplayAreaAppearedInfo info = displayAreaInfos.get(i); onDisplayAreaAppeared(info.getDisplayAreaInfo(), info.getLeash()); } + mDefaultDisplayBounds.set(getDisplayBounds()); return displayAreaInfos; } @@ -176,9 +162,9 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { * handles 90 degree display rotation changes {@link Surface.Rotation}. * * @param fromRotation starting rotation of the display. - * @param toRotation target rotation of the display (after rotating). - * @param wct A task transaction {@link WindowContainerTransaction} from - * {@link DisplayChangeController} to populate. + * @param toRotation target rotation of the display (after rotating). + * @param wct A task transaction {@link WindowContainerTransaction} from + * {@link DisplayChangeController} to populate. */ public void onRotateDisplay(int fromRotation, int toRotation, WindowContainerTransaction wct) { // Stop one handed without animation and reset cropped size immediately @@ -210,11 +196,11 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { : TRANSITION_DIRECTION_EXIT; final WindowContainerTransaction wct = new WindowContainerTransaction(); - mDisplayAreaMap.forEach( - (key, leash) -> { - animateWindows(leash, fromBounds, toBounds, direction, + mDisplayAreaTokenMap.forEach( + (token, leash) -> { + animateWindows(token, leash, fromBounds, toBounds, direction, mEnterExitAnimationDurationMs); - wct.setBounds(key.token, toBounds); + wct.setBounds(token, toBounds); }); applyTransaction(wct); } @@ -222,10 +208,10 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { private void resetWindowsOffset(WindowContainerTransaction wct) { final SurfaceControl.Transaction tx = mSurfaceControlTransactionFactory.getTransaction(); - mDisplayAreaMap.forEach( - (key, leash) -> { + mDisplayAreaTokenMap.forEach( + (token, leash) -> { final OneHandedAnimationController.OneHandedTransitionAnimator animator = - mAnimationController.getAnimatorMap().remove(leash); + mAnimationController.getAnimatorMap().remove(token); if (animator != null && animator.isRunning()) { animator.cancel(); } @@ -233,16 +219,17 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { .setWindowCrop(leash, -1/* reset */, -1/* reset */); // DisplayRotationController will applyTransaction() after finish rotating if (wct != null) { - wct.setBounds(key.token, null/* reset */); + wct.setBounds(token, null/* reset */); } }); tx.apply(); } - private void animateWindows(SurfaceControl leash, Rect fromBounds, Rect toBounds, - @OneHandedAnimationController.TransitionDirection int direction, int durationMs) { + private void animateWindows(WindowContainerToken token, SurfaceControl leash, Rect fromBounds, + Rect toBounds, @OneHandedAnimationController.TransitionDirection int direction, + int durationMs) { final OneHandedAnimationController.OneHandedTransitionAnimator animator = - mAnimationController.getAnimator(leash, fromBounds, toBounds); + mAnimationController.getAnimator(token, leash, fromBounds, toBounds); if (animator != null) { animator.setTransitionDirection(direction) .addOneHandedAnimationCallback(mOneHandedAnimationCallback) @@ -311,8 +298,8 @@ public class OneHandedDisplayAreaOrganizer extends DisplayAreaOrganizer { pw.println(TAG + "states: "); pw.print(innerPrefix + "mIsInOneHanded="); pw.println(mIsInOneHanded); - pw.print(innerPrefix + "mDisplayAreaMap="); - pw.println(mDisplayAreaMap); + pw.print(innerPrefix + "mDisplayAreaTokenMap="); + pw.println(mDisplayAreaTokenMap); pw.print(innerPrefix + "mDefaultDisplayBounds="); pw.println(mDefaultDisplayBounds); pw.print(innerPrefix + "mLastVisualDisplayBounds="); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedAnimationControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedAnimationControllerTest.java index 17fc0578dd2b7..8d5139b182f09 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedAnimationControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedAnimationControllerTest.java @@ -22,6 +22,7 @@ import android.graphics.Rect; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.view.SurfaceControl; +import android.window.WindowContainerToken; import androidx.test.filters.SmallTest; @@ -50,6 +51,8 @@ public class OneHandedAnimationControllerTest extends OneHandedTestCase { @Mock private SurfaceControl mMockLeash; + @Mock + private WindowContainerToken mMockToken; @Mock private ShellExecutor mMainExecutor; @@ -69,7 +72,7 @@ public class OneHandedAnimationControllerTest extends OneHandedTestCase { destinationBounds.offset(0, 300); final OneHandedAnimationController.OneHandedTransitionAnimator animator = mOneHandedAnimationController - .getAnimator(mMockLeash, originalBounds, destinationBounds); + .getAnimator(mMockToken, mMockLeash, originalBounds, destinationBounds); assertNotNull(animator); } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizerTest.java index 6cfd0c43724c2..01162b5c0b838 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedDisplayAreaOrganizerTest.java @@ -24,13 +24,14 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.anyFloat; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.res.Configuration; -import android.os.Handler; +import android.os.Binder; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.view.Display; @@ -89,12 +90,14 @@ public class OneHandedDisplayAreaOrganizerTest extends OneHandedTestCase { public void setUp() throws Exception { MockitoAnnotations.initMocks(this); mTestableLooper = TestableLooper.get(this); + Binder binder = new Binder(); + doReturn(binder).when(mMockRealToken).asBinder(); mToken = new WindowContainerToken(mMockRealToken); mLeash = new SurfaceControl(); mDisplay = mContext.getDisplay(); mDisplayAreaInfo = new DisplayAreaInfo(mToken, DEFAULT_DISPLAY, FEATURE_ONE_HANDED); mDisplayAreaInfo.configuration.orientation = Configuration.ORIENTATION_PORTRAIT; - when(mMockAnimationController.getAnimator(any(), any(), any())).thenReturn(null); + when(mMockAnimationController.getAnimator(any(), any(), any(), any())).thenReturn(null); when(mMockDisplayController.getDisplay(anyInt())).thenReturn(mDisplay); when(mMockSurfaceTransactionHelper.translate(any(), any(), anyFloat())).thenReturn( mMockSurfaceTransactionHelper); @@ -121,7 +124,7 @@ public class OneHandedDisplayAreaOrganizerTest extends OneHandedTestCase { public void testOnDisplayAreaAppeared() { mDisplayAreaOrganizer.onDisplayAreaAppeared(mDisplayAreaInfo, mLeash); - verify(mMockAnimationController, never()).getAnimator(any(), any(), any()); + verify(mMockAnimationController, never()).getAnimator(any(), any(), any(), any()); } @Test @@ -129,7 +132,7 @@ public class OneHandedDisplayAreaOrganizerTest extends OneHandedTestCase { mDisplayAreaOrganizer.onDisplayAreaAppeared(mDisplayAreaInfo, mLeash); mDisplayAreaOrganizer.onDisplayAreaVanished(mDisplayAreaInfo); - assertThat(mDisplayAreaOrganizer.mDisplayAreaMap).isEmpty(); + assertThat(mDisplayAreaOrganizer.mDisplayAreaTokenMap).isEmpty(); } @Test