summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author wilsonshih <wilsonshih@google.com> 2024-03-29 08:10:43 +0000
committer wilsonshih <wilsonshih@google.com> 2024-04-01 05:32:19 +0000
commit1b8878fa870b4986314595838b653a6151a8924a (patch)
tree5d05ad79221b28d6c9ed7bf5883783fbfadac79d
parenta9b3adea5ceee2090a625aa8cfc8f37cf765ea2a (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
-rw-r--r--libs/WindowManager/Shell/src/com/android/wm/shell/back/BackAnimationController.java23
-rw-r--r--libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/back/BackAnimationControllerTest.java5
-rw-r--r--services/core/java/com/android/server/wm/BackNavigationController.java46
-rw-r--r--services/tests/wmtests/src/com/android/server/wm/BackNavigationControllerTests.java4
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,