diff options
| author | 2024-03-29 08:10:43 +0000 | |
|---|---|---|
| committer | 2024-04-01 05:32:19 +0000 | |
| commit | 1b8878fa870b4986314595838b653a6151a8924a (patch) | |
| tree | 5d05ad79221b28d6c9ed7bf5883783fbfadac79d | |
| parent | a9b3adea5ceee2090a625aa8cfc8f37cf765ea2a (diff) | |
More stabilize predictive back animation.
The one way binder call may not invoke immediately, so it's also
possible that shell side received onAnimationStart after core has
canceled that animation. So in shell, validate the leashes of animation
targets before send it to runner, normally NavigationObserver should
receive cancel callback after that.
Also add the same check before send animation targets to shell, if
invalide, cancel the animation and notify onAnimationCancelled.
Bug: 327312316
Bug: 318294405
Test: simulate send invalidate animation targets to shell, verify it
won't crash systemui.
Change-Id: I4ffa8b6654806181763d6a58d2a748537675e030
4 files changed, 66 insertions, 12 deletions
diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java index ad3be3d0b022..ce7859e4752d 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java @@ -173,6 +173,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont ProtoLog.i(WM_SHELL_BACK_PREVIEW, "Navigation window gone."); setTriggerBack(false); resetTouchTracker(); + // Don't wait for animation start + mShellExecutor.removeCallbacks(mAnimationTimeoutRunnable); }); } }); @@ -950,7 +952,7 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont ProtoLog.e(WM_SHELL_BACK_PREVIEW, "Lack of navigation info to start animation."); return; } - if (mApps == null) { + if (!validateAnimationTargets(mApps)) { ProtoLog.w(WM_SHELL_BACK_PREVIEW, "Not starting animation due to mApps being null."); return; } @@ -981,6 +983,21 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont } } + /** + * Validate animation targets. + */ + static boolean validateAnimationTargets(RemoteAnimationTarget[] apps) { + if (apps == null || apps.length == 0) { + return false; + } + for (int i = apps.length - 1; i >= 0; --i) { + if (!apps[i].leash.isValid()) { + return false; + } + } + return true; + } + private void createAdapter() { IBackAnimationRunner runner = new IBackAnimationRunner.Stub() { @@ -993,6 +1010,10 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont mShellExecutor.execute( () -> { endLatencyTracking(); + if (!validateAnimationTargets(apps)) { + Log.e(TAG, "Invalid animation targets!"); + return; + } mBackAnimationFinishedCallback = finishedCallback; mApps = apps; startSystemAnimation(); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java index 2919782a758a..5570d5a2d873 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java @@ -69,7 +69,6 @@ import com.android.wm.shell.sysui.ShellController; import com.android.wm.shell.sysui.ShellInit; import com.android.wm.shell.sysui.ShellSharedConstants; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -182,7 +181,9 @@ public class BackAnimationControllerTest extends ShellTestCase { } RemoteAnimationTarget createAnimationTarget() { - SurfaceControl topWindowLeash = new SurfaceControl(); + SurfaceControl topWindowLeash = new SurfaceControl.Builder() + .setName("FakeLeash") + .build(); return new RemoteAnimationTarget(-1, RemoteAnimationTarget.MODE_CLOSING, topWindowLeash, false, new Rect(), new Rect(), -1, new Point(0, 0), new Rect(), new Rect(), new WindowConfiguration(), diff --git a/services/core/java/com/android/server/wm/BackNavigationController.java b/services/core/java/com/android/server/wm/BackNavigationController.java index 7144445f86d8..f6681c571090 100644 --- a/services/core/java/com/android/server/wm/BackNavigationController.java +++ b/services/core/java/com/android/server/wm/BackNavigationController.java @@ -359,6 +359,7 @@ class BackNavigationController { mAnimationHandler.prepareAnimation( backType, adapter, + mNavigationMonitor, currentTask, prevTask, currentActivity, @@ -667,7 +668,8 @@ class BackNavigationController { mAnimationHandler.markWindowHasDrawn(openActivity); } - private class NavigationMonitor { + @VisibleForTesting + class NavigationMonitor { // The window which triggering the back navigation. private WindowState mNavigatingWindow; private RemoteCallback mObserver; @@ -1492,28 +1494,31 @@ class BackNavigationController { ScheduleAnimationBuilder prepareAnimation( int backType, BackAnimationAdapter adapter, + NavigationMonitor monitor, Task currentTask, Task previousTask, ActivityRecord currentActivity, ArrayList<ActivityRecord> previousActivity, WindowContainer removedWindowContainer) { + final ScheduleAnimationBuilder builder = + new ScheduleAnimationBuilder(backType, adapter, monitor); switch (backType) { case BackNavigationInfo.TYPE_RETURN_TO_HOME: - return new ScheduleAnimationBuilder(backType, adapter) + return builder .setIsLaunchBehind(true) .setComposeTarget(currentTask, previousTask); case BackNavigationInfo.TYPE_CROSS_ACTIVITY: ActivityRecord[] prevActs = new ActivityRecord[previousActivity.size()]; prevActs = previousActivity.toArray(prevActs); - return new ScheduleAnimationBuilder(backType, adapter) + return builder .setComposeTarget(currentActivity, prevActs) .setIsLaunchBehind(false); case BackNavigationInfo.TYPE_CROSS_TASK: - return new ScheduleAnimationBuilder(backType, adapter) + return builder .setComposeTarget(currentTask, previousTask) .setIsLaunchBehind(false); case BackNavigationInfo.TYPE_DIALOG_CLOSE: - return new ScheduleAnimationBuilder(backType, adapter) + return builder .setComposeTarget(removedWindowContainer, currentActivity) .setIsLaunchBehind(false); } @@ -1523,13 +1528,16 @@ class BackNavigationController { class ScheduleAnimationBuilder { final int mType; final BackAnimationAdapter mBackAnimationAdapter; + final NavigationMonitor mNavigationMonitor; WindowContainer mCloseTarget; WindowContainer[] mOpenTargets; boolean mIsLaunchBehind; - ScheduleAnimationBuilder(int type, BackAnimationAdapter backAnimationAdapter) { + ScheduleAnimationBuilder(int type, BackAnimationAdapter adapter, + NavigationMonitor monitor) { mType = type; - mBackAnimationAdapter = backAnimationAdapter; + mBackAnimationAdapter = adapter; + mNavigationMonitor = monitor; } ScheduleAnimationBuilder setComposeTarget(@NonNull WindowContainer close, @@ -1610,8 +1618,13 @@ class BackNavigationController { return () -> { try { - mBackAnimationAdapter.getRunner().onAnimationStart( - targets, null, null, callback); + if (hasTargetDetached() || !validateAnimationTargets(targets)) { + mNavigationMonitor.cancelBackNavigating("cancelAnimation"); + mBackAnimationAdapter.getRunner().onAnimationCancelled(); + } else { + mBackAnimationAdapter.getRunner().onAnimationStart( + targets, null, null, callback); + } } catch (RemoteException e) { e.printStackTrace(); } @@ -1641,6 +1654,21 @@ class BackNavigationController { } /** + * Validate animation targets. + */ + private static boolean validateAnimationTargets(RemoteAnimationTarget[] apps) { + if (apps == null || apps.length == 0) { + return false; + } + for (int i = apps.length - 1; i >= 0; --i) { + if (!apps[i].leash.isValid()) { + return false; + } + } + return true; + } + + /** * Finds next opening activity(ies) based on open targets, which could be: * 1. If the open window is Task, then the open activity can either be an activity, or * two activities inside two TaskFragments diff --git a/services/tests/wmtests/src/com/android/server/wm/BackNavigationControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/BackNavigationControllerTests.java index 363ae141e512..6426d230fde4 100644 --- a/services/tests/wmtests/src/com/android/server/wm/BackNavigationControllerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/BackNavigationControllerTests.java @@ -94,6 +94,7 @@ public class BackNavigationControllerTests extends WindowTestsBase { private BackNavigationController mBackNavigationController; private WindowManagerInternal mWindowManagerInternal; private BackAnimationAdapter mBackAnimationAdapter; + private BackNavigationController.NavigationMonitor mNavigationMonitor; private Task mRootHomeTask; @Before @@ -105,6 +106,7 @@ public class BackNavigationControllerTests extends WindowTestsBase { mWindowManagerInternal = mock(WindowManagerInternal.class); LocalServices.addService(WindowManagerInternal.class, mWindowManagerInternal); mBackAnimationAdapter = mock(BackAnimationAdapter.class); + mNavigationMonitor = mock(BackNavigationController.NavigationMonitor.class); mRootHomeTask = initHomeActivity(); } @@ -813,6 +815,7 @@ public class BackNavigationControllerTests extends WindowTestsBase { animationHandler.prepareAnimation( BackNavigationInfo.TYPE_RETURN_TO_HOME, mBackAnimationAdapter, + mNavigationMonitor, task, mRootHomeTask, bottomActivity, @@ -832,6 +835,7 @@ public class BackNavigationControllerTests extends WindowTestsBase { animationHandler.prepareAnimation( BackNavigationInfo.TYPE_CROSS_ACTIVITY, mBackAnimationAdapter, + mNavigationMonitor, task, task, topActivity, |