From 410b36e547a5a2356a3cee3067d4ec4632a13dee Mon Sep 17 00:00:00 2001 From: Chris Li Date: Tue, 10 May 2022 21:22:15 +0800 Subject: Use getSyncTransaction in SurfaceAnimator and SurfaceFreezer Otherwise, when we ever use SurfaceFreezer for change transition, it may have race condition with WM Shell. Bug: 227533765 Test: verify with ActivityEmbedding app in split screen. Change-Id: I7027e624303f3fa76dafde0e2372ecc93d15f516 --- .../core/java/com/android/server/wm/Dimmer.java | 9 +++++++-- .../android/server/wm/ScreenRotationAnimation.java | 1 + .../android/server/wm/SimpleSurfaceAnimatable.java | 22 ++++++++++++++++++++++ .../com/android/server/wm/SurfaceAnimator.java | 14 ++++++++++---- .../com/android/server/wm/WindowContainer.java | 5 +++-- .../server/wm/WindowContainerThumbnail.java | 5 +++++ .../src/com/android/server/wm/DimmerTests.java | 10 ++++++++++ .../com/android/server/wm/SurfaceAnimatorTest.java | 5 +++++ 8 files changed, 63 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java index 029056ac26fb..e7ab63eab202 100644 --- a/services/core/java/com/android/server/wm/Dimmer.java +++ b/services/core/java/com/android/server/wm/Dimmer.java @@ -50,10 +50,15 @@ class Dimmer { } @Override - public SurfaceControl.Transaction getPendingTransaction() { + public SurfaceControl.Transaction getSyncTransaction() { return mHost.getSyncTransaction(); } + @Override + public SurfaceControl.Transaction getPendingTransaction() { + return mHost.getPendingTransaction(); + } + @Override public void commitPendingTransaction() { mHost.commitPendingTransaction(); @@ -105,7 +110,7 @@ class Dimmer { void removeSurface() { if (mDimLayer != null && mDimLayer.isValid()) { - getPendingTransaction().remove(mDimLayer); + getSyncTransaction().remove(mDimLayer); } mDimLayer = null; } diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java index b4029d185b9f..518bfd4c90df 100644 --- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java +++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java @@ -565,6 +565,7 @@ class ScreenRotationAnimation { private SimpleSurfaceAnimatable.Builder initializeBuilder() { return new SimpleSurfaceAnimatable.Builder() + .setSyncTransactionSupplier(mDisplayContent::getSyncTransaction) .setPendingTransactionSupplier(mDisplayContent::getPendingTransaction) .setCommitTransactionRunnable(mDisplayContent::commitPendingTransaction) .setAnimationLeashSupplier(mDisplayContent::makeOverlay); diff --git a/services/core/java/com/android/server/wm/SimpleSurfaceAnimatable.java b/services/core/java/com/android/server/wm/SimpleSurfaceAnimatable.java index bf5d5e2653a8..3b3db890f67e 100644 --- a/services/core/java/com/android/server/wm/SimpleSurfaceAnimatable.java +++ b/services/core/java/com/android/server/wm/SimpleSurfaceAnimatable.java @@ -41,6 +41,7 @@ public class SimpleSurfaceAnimatable implements SurfaceAnimator.Animatable { private final SurfaceControl mParentSurfaceControl; private final Runnable mCommitTransactionRunnable; private final Supplier mAnimationLeashFactory; + private final Supplier mSyncTransaction; private final Supplier mPendingTransaction; private final BiConsumer mOnAnimationLeashCreated; private final Consumer mOnAnimationLeashLost; @@ -60,10 +61,16 @@ public class SimpleSurfaceAnimatable implements SurfaceAnimator.Animatable { mAnimationLeashFactory = builder.mAnimationLeashFactory; mOnAnimationLeashCreated = builder.mOnAnimationLeashCreated; mOnAnimationLeashLost = builder.mOnAnimationLeashLost; + mSyncTransaction = builder.mSyncTransactionSupplier; mPendingTransaction = builder.mPendingTransactionSupplier; mOnAnimationFinished = builder.mOnAnimationFinished; } + @Override + public SurfaceControl.Transaction getSyncTransaction() { + return mSyncTransaction.get(); + } + @NonNull @Override public SurfaceControl.Transaction getPendingTransaction() { @@ -159,6 +166,9 @@ public class SimpleSurfaceAnimatable implements SurfaceAnimator.Animatable { @Nullable private Consumer mOnAnimationFinished = null; + @NonNull + private Supplier mSyncTransactionSupplier; + @NonNull private Supplier mPendingTransactionSupplier; @@ -206,6 +216,15 @@ public class SimpleSurfaceAnimatable implements SurfaceAnimator.Animatable { return this; } + /** + * @see SurfaceAnimator.Animatable#getSyncTransaction() + */ + public Builder setSyncTransactionSupplier( + @NonNull Supplier syncTransactionSupplier) { + mSyncTransactionSupplier = syncTransactionSupplier; + return this; + } + /** * @see SurfaceAnimator.Animatable#getPendingTransaction() */ @@ -290,6 +309,9 @@ public class SimpleSurfaceAnimatable implements SurfaceAnimator.Animatable { } public SurfaceAnimator.Animatable build() { + if (mSyncTransactionSupplier == null) { + throw new IllegalArgumentException("mSyncTransactionSupplier cannot be null"); + } if (mPendingTransactionSupplier == null) { throw new IllegalArgumentException("mPendingTransactionSupplier cannot be null"); } diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java index fbf04262cc37..3dde2f1aa01f 100644 --- a/services/core/java/com/android/server/wm/SurfaceAnimator.java +++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java @@ -128,7 +128,7 @@ class SurfaceAnimator { } final OnAnimationFinishedCallback animationFinishCallback = mSurfaceAnimationFinishedCallback; - reset(mAnimatable.getPendingTransaction(), true /* destroyLeash */); + reset(mAnimatable.getSyncTransaction(), true /* destroyLeash */); if (staticAnimationFinishedCallback != null) { staticAnimationFinishedCallback.onAnimationFinished(type, anim); } @@ -234,7 +234,7 @@ class SurfaceAnimator { final boolean delayed = mAnimationStartDelayed; mAnimationStartDelayed = false; if (delayed && mAnimation != null) { - mAnimation.startAnimation(mLeash, mAnimatable.getPendingTransaction(), + mAnimation.startAnimation(mLeash, mAnimatable.getSyncTransaction(), mAnimationType, mInnerAnimationFinishedCallback); mAnimatable.commitPendingTransaction(); } @@ -264,7 +264,7 @@ class SurfaceAnimator { * Cancels any currently running animation. */ void cancelAnimation() { - cancelAnimation(mAnimatable.getPendingTransaction(), false /* restarting */, + cancelAnimation(mAnimatable.getSyncTransaction(), false /* restarting */, true /* forwardCancel */); mAnimatable.commitPendingTransaction(); } @@ -319,7 +319,7 @@ class SurfaceAnimator { return; } endDelayingAnimationStart(); - final Transaction t = mAnimatable.getPendingTransaction(); + final Transaction t = mAnimatable.getSyncTransaction(); cancelAnimation(t, true /* restarting */, true /* forwardCancel */); mLeash = from.mLeash; mAnimation = from.mAnimation; @@ -619,6 +619,12 @@ class SurfaceAnimator { */ interface Animatable { + /** + * Use this method instead of {@link #getPendingTransaction()} if the transaction should be + * synchronized with the client. + */ + @NonNull Transaction getSyncTransaction(); + /** * @return The pending transaction that will be committed in the next frame. */ diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 2f00bc821678..19f21fabad17 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -1004,7 +1004,7 @@ class WindowContainer extends ConfigurationContainer< void onDisplayChanged(DisplayContent dc) { if (mDisplayContent != null && mDisplayContent.mChangingContainers.remove(this)) { // Cancel any change transition queued-up for this container on the old display. - mSurfaceFreezer.unfreeze(getPendingTransaction()); + mSurfaceFreezer.unfreeze(getSyncTransaction()); } mDisplayContent = dc; if (dc != null && dc != this) { @@ -2689,6 +2689,7 @@ class WindowContainer extends ConfigurationContainer< * @return {@link #mBLASTSyncTransaction} if available. Otherwise, returns * {@link #getPendingTransaction()} */ + @Override public Transaction getSyncTransaction() { if (mSyncTransactionCommitCallbackDepth > 0) { return mSyncTransaction; @@ -2759,7 +2760,7 @@ class WindowContainer extends ConfigurationContainer< void cancelAnimation() { doAnimationFinished(mSurfaceAnimator.getAnimationType(), mSurfaceAnimator.getAnimation()); mSurfaceAnimator.cancelAnimation(); - mSurfaceFreezer.unfreeze(getPendingTransaction()); + mSurfaceFreezer.unfreeze(getSyncTransaction()); } /** Whether we can start change transition with this window and current display status. */ diff --git a/services/core/java/com/android/server/wm/WindowContainerThumbnail.java b/services/core/java/com/android/server/wm/WindowContainerThumbnail.java index 7f21eeb43d59..9b6f4d947694 100644 --- a/services/core/java/com/android/server/wm/WindowContainerThumbnail.java +++ b/services/core/java/com/android/server/wm/WindowContainerThumbnail.java @@ -166,6 +166,11 @@ class WindowContainerThumbnail implements Animatable { proto.end(token); } + @Override + public Transaction getSyncTransaction() { + return mWindowContainer.getSyncTransaction(); + } + @Override public Transaction getPendingTransaction() { return mWindowContainer.getPendingTransaction(); diff --git a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java index 3beb7f2049df..55a7c1ba0bca 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java @@ -63,6 +63,11 @@ public class DimmerTests extends WindowTestsBase { return mControl; } + @Override + public SurfaceControl.Transaction getSyncTransaction() { + return mTransaction; + } + @Override public SurfaceControl.Transaction getPendingTransaction() { return mTransaction; @@ -101,6 +106,11 @@ public class DimmerTests extends WindowTestsBase { return mHostControl; } + @Override + public SurfaceControl.Transaction getSyncTransaction() { + return mHostTransaction; + } + @Override public SurfaceControl.Transaction getPendingTransaction() { return mHostTransaction; diff --git a/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java b/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java index 79ba1759f4c4..ff0063c4ffa0 100644 --- a/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java @@ -337,6 +337,11 @@ public class SurfaceAnimatorTest extends WindowTestsBase { mSurfaceAnimator = new SurfaceAnimator(this, mFinishedCallback, wm); } + @Override + public SurfaceControl.Transaction getSyncTransaction() { + return mTransaction; + } + @Override public Transaction getPendingTransaction() { return mTransaction; -- cgit v1.2.3-59-g8ed1b