Fix recent task screenshot may not cleanup in some cases
In previous implementation, TaskScreenshotAnimatable's surface
relies on SurfaceAnimator#onAnimationLeashDestroyed callback.
As the definition of this callback method is called when leash is being
destroyed, and the surface was reparented back to the original parent,
which may not be reliable for recent task screenshot use case because task
screenhot surface may fail to reparent back if the parent is null, and
the onAnimationLeashDestroyed callback won't be called.
To fix this, we modify the reparent check to exclude parent surface check.
This is because the screenshot animatable was returning null in
Animatable.getParentSurface(), but we still need to invoke
onAnimationLeashDestroyed.
We also rename the callback to onAnimationLeashLost as it will also be called
when the animation is transferred away to another animation, even though the
leash didn't get destroyed.
Also, modified TaskScreenshotAnimatable#getParentSurfaceControl to
align Task's surface for surface hierachy correctness.
Fix: 130606600
Test: atest RecentsAnimationControllerTest
Test: atest SurfaceAnimatorTest
Change-Id: I918554befe3982a3dc0a9949c6973042697bb24b
diff --git a/services/core/java/com/android/server/wm/AppWindowThumbnail.java b/services/core/java/com/android/server/wm/AppWindowThumbnail.java
index 4d972dc..4ceae72 100644
--- a/services/core/java/com/android/server/wm/AppWindowThumbnail.java
+++ b/services/core/java/com/android/server/wm/AppWindowThumbnail.java
@@ -195,7 +195,7 @@
}
@Override
- public void onAnimationLeashDestroyed(Transaction t) {
+ public void onAnimationLeashLost(Transaction t) {
// TODO: Once attached to app token, we don't need to hide it immediately if thumbnail
// became visible.
diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java
index 5be8e14..6499497 100644
--- a/services/core/java/com/android/server/wm/AppWindowToken.java
+++ b/services/core/java/com/android/server/wm/AppWindowToken.java
@@ -2668,8 +2668,8 @@
}
@Override
- public void onAnimationLeashDestroyed(Transaction t) {
- super.onAnimationLeashDestroyed(t);
+ public void onAnimationLeashLost(Transaction t) {
+ super.onAnimationLeashLost(t);
if (mAnimationBoundsLayer != null) {
t.remove(mAnimationBoundsLayer);
mAnimationBoundsLayer = null;
@@ -2855,7 +2855,7 @@
t.reparent(mTransitChangeLeash, null);
mTransitChangeLeash = null;
if (cancel) {
- onAnimationLeashDestroyed(t);
+ onAnimationLeashLost(t);
}
}
diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java
index 767327a..e6150cb 100644
--- a/services/core/java/com/android/server/wm/Dimmer.java
+++ b/services/core/java/com/android/server/wm/Dimmer.java
@@ -62,7 +62,7 @@
}
@Override
- public void onAnimationLeashDestroyed(SurfaceControl.Transaction t) {
+ public void onAnimationLeashLost(SurfaceControl.Transaction t) {
}
@Override
diff --git a/services/core/java/com/android/server/wm/RecentsAnimation.java b/services/core/java/com/android/server/wm/RecentsAnimation.java
index 07d3fb9..b50bfbb 100644
--- a/services/core/java/com/android/server/wm/RecentsAnimation.java
+++ b/services/core/java/com/android/server/wm/RecentsAnimation.java
@@ -331,6 +331,10 @@
}
final RecentsAnimationController controller =
mWindowManager.getRecentsAnimationController();
+ if (controller == null) {
+ return;
+ }
+
final DisplayContent dc =
mService.mRootActivityContainer.getDefaultDisplay().mDisplayContent;
dc.mBoundsAnimationController.setAnimationType(
diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java
index b3b41b7..3d9dfeb 100644
--- a/services/core/java/com/android/server/wm/SurfaceAnimator.java
+++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java
@@ -50,7 +50,8 @@
@VisibleForTesting
SurfaceControl mLeash;
- private final Animatable mAnimatable;
+ @VisibleForTesting
+ final Animatable mAnimatable;
private final OnAnimationFinishedCallback mInnerAnimationFinishedCallback;
@VisibleForTesting
final Runnable mAnimationFinishedCallback;
@@ -282,13 +283,15 @@
boolean scheduleAnim = false;
- // If the surface was destroyed, we don't care to reparent it back.
- final boolean destroy = mLeash != null && surface != null && parent != null;
- if (destroy) {
- if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent");
+ // If the surface was destroyed or the leash is invalid, we don't care to reparent it back.
+ // Note that we also set this variable to true even if the parent isn't valid anymore, in
+ // order to ensure onAnimationLeashLost still gets called in this case.
+ final boolean reparent = mLeash != null && surface != null;
+ if (reparent) {
+ if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent: " + parent);
// We shouldn't really need these isValid checks but we do
// b/130364451
- if (surface.isValid() && parent.isValid()) {
+ if (surface.isValid() && parent != null && parent.isValid()) {
t.reparent(surface, parent);
scheduleAnim = true;
}
@@ -301,9 +304,10 @@
mLeash = null;
mAnimation = null;
- // Make sure to inform the animatable after the leash was destroyed.
- if (destroy) {
- mAnimatable.onAnimationLeashDestroyed(t);
+ if (reparent) {
+ // Make sure to inform the animatable after the surface was reparented (or reparent
+ // wasn't possible, but we still need to invoke the callback)
+ mAnimatable.onAnimationLeashLost(t);
scheduleAnim = true;
}
@@ -394,12 +398,12 @@
void onAnimationLeashCreated(Transaction t, SurfaceControl leash);
/**
- * Called when the leash is being destroyed, and the surface was reparented back to the
- * original parent.
+ * Called when the leash is being destroyed, or when the leash is being transferred to
+ * another SurfaceAnimator.
*
* @param t The transaction to use to apply any necessary changes.
*/
- void onAnimationLeashDestroyed(Transaction t);
+ void onAnimationLeashLost(Transaction t);
/**
* @return A new surface to be used for the animation leash, inserted at the correct
diff --git a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
index f47e794..17e4b89 100644
--- a/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
+++ b/services/core/java/com/android/server/wm/TaskScreenshotAnimatable.java
@@ -91,9 +91,11 @@
}
@Override
- public void onAnimationLeashDestroyed(SurfaceControl.Transaction t) {
- t.remove(mSurfaceControl);
- mSurfaceControl = null;
+ public void onAnimationLeashLost(SurfaceControl.Transaction t) {
+ if (mSurfaceControl != null) {
+ t.remove(mSurfaceControl);
+ mSurfaceControl = null;
+ }
}
@Override
@@ -113,7 +115,7 @@
@Override
public SurfaceControl getParentSurfaceControl() {
- return mTask.mSurfaceAnimator.mLeash;
+ return mTask.mSurfaceControl;
}
@Override
diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index f3f6397..d5c3e4f 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -1290,7 +1290,7 @@
}
@Override
- public void onAnimationLeashDestroyed(Transaction t) {
+ public void onAnimationLeashLost(Transaction t) {
mLastLayer = -1;
reassignLayer(t);
}
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index 497c91a..3834d57 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -4809,8 +4809,8 @@
}
@Override
- public void onAnimationLeashDestroyed(Transaction t) {
- super.onAnimationLeashDestroyed(t);
+ public void onAnimationLeashLost(Transaction t) {
+ super.onAnimationLeashLost(t);
updateSurfacePosition(t);
}