Ensure dim changes are in same transaction; do not wait for token to change visibility

- Use the pending transaction to avoid collecting changes to the same
dim in separate transactions. This was causing problems since
multiple WindowStates can request a dim change, and they were doing so
by placing the changes in different transactions. This was an issue since there was
no guarantee that they were then merged in the correct order.

- Check for the requested visibility of the token to avoid waiting for it to change its visibility at the end of a transition. This allows to
shield the dim from changes to its windows while it waits to be
committed.

Fix: 288863491
Test: DimmerTests
Change-Id: I96f8d1b091dd0f897c50833be7540b00fa2ead70

3 files changed, 28 insertions, 29 deletions

diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java
index 89f044bdd163..d7667d8ce7a8 100644
--- a/services/core/java/com/android/server/wm/Dimmer.java
+++ b/services/core/java/com/android/server/wm/Dimmer.java
@@ -215,8 +215,7 @@ class Dimmer {
         return mDimState;
     }
 
-    private void dim(SurfaceControl.Transaction t, WindowContainer container, int relativeLayer,
-            float alpha, int blurRadius) {
+    private void dim(WindowContainer container, int relativeLayer, float alpha, int blurRadius) {
         final DimState d = getDimState(container);
 
         if (d == null) {
@@ -226,6 +225,7 @@ class Dimmer {
         // The dim method is called from WindowState.prepareSurfaces(), which is always called
         // in the correct Z from lowest Z to highest. This ensures that the dim layer is always
         // relative to the highest Z layer with a dim.
+        SurfaceControl.Transaction t = mHost.getPendingTransaction();
         t.setRelativeLayer(d.mDimLayer, container.getSurfaceControl(), relativeLayer);
         t.setAlpha(d.mDimLayer, alpha);
         t.setBackgroundBlurRadius(d.mDimLayer, blurRadius);
@@ -238,26 +238,23 @@ class Dimmer {
      * for each call to {@link WindowContainer#prepareSurfaces} the Dim state will be reset
      * and the child should call dimAbove again to request the Dim to continue.
      *
-     * @param t         A transaction in which to apply the Dim.
      * @param container The container which to dim above. Should be a child of our host.
      * @param alpha     The alpha at which to Dim.
      */
-    void dimAbove(SurfaceControl.Transaction t, WindowContainer container, float alpha) {
-        dim(t, container, 1, alpha, 0);
+    void dimAbove(WindowContainer container, float alpha) {
+        dim(container, 1, alpha, 0);
     }
 
     /**
      * Like {@link #dimAbove} but places the dim below the given container.
      *
-     * @param t          A transaction in which to apply the Dim.
      * @param container  The container which to dim below. Should be a child of our host.
      * @param alpha      The alpha at which to Dim.
      * @param blurRadius The amount of blur added to the Dim.
      */
 
-    void dimBelow(SurfaceControl.Transaction t, WindowContainer container, float alpha,
-                  int blurRadius) {
-        dim(t, container, -1, alpha, blurRadius);
+    void dimBelow(WindowContainer container, float alpha, int blurRadius) {
+        dim(container, -1, alpha, blurRadius);
     }
 
     /**
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index cf1e51fb4e94..cae3dbe769f9 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -5132,7 +5132,8 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
 
     private void applyDims() {
         if (((mAttrs.flags & FLAG_DIM_BEHIND) != 0 || shouldDrawBlurBehind())
-                   && isVisibleNow() && !mHidden && mTransitionController.canApplyDim(getTask())) {
+                && mToken.isVisibleRequested() && isVisibleNow() && !mHidden
+                && mTransitionController.canApplyDim(getTask())) {
             // Only show the Dimmer when the following is satisfied:
             // 1. The window has the flag FLAG_DIM_BEHIND or blur behind is requested
             // 2. The WindowToken is not hidden so dims aren't shown when the window is exiting.
@@ -5142,7 +5143,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
             mIsDimming = true;
             final float dimAmount = (mAttrs.flags & FLAG_DIM_BEHIND) != 0 ? mAttrs.dimAmount : 0;
             final int blurRadius = shouldDrawBlurBehind() ? mAttrs.getBlurBehindRadius() : 0;
-            getDimmer().dimBelow(getSyncTransaction(), this, dimAmount, blurRadius);
+            getDimmer().dimBelow(this, dimAmount, blurRadius);
         }
     }
 
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 f235d153c658..233a2076a867 100644
--- a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java
@@ -52,7 +52,8 @@ public class DimmerTests extends WindowTestsBase {
 
     private static class TestWindowContainer extends WindowContainer<TestWindowContainer> {
         final SurfaceControl mControl = mock(SurfaceControl.class);
-        final SurfaceControl.Transaction mTransaction = spy(StubTransaction.class);
+        final SurfaceControl.Transaction mPendingTransaction = spy(StubTransaction.class);
+        final SurfaceControl.Transaction mSyncTransaction = spy(StubTransaction.class);
 
         TestWindowContainer(WindowManagerService wm) {
             super(wm);
@@ -65,12 +66,12 @@ public class DimmerTests extends WindowTestsBase {
 
         @Override
         public SurfaceControl.Transaction getSyncTransaction() {
-            return mTransaction;
+            return mSyncTransaction;
         }
 
         @Override
         public SurfaceControl.Transaction getPendingTransaction() {
-            return mTransaction;
+            return mPendingTransaction;
         }
     }
 
@@ -144,7 +145,7 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
 
         int width = 100;
         int height = 300;
@@ -161,13 +162,13 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
         SurfaceControl dimLayer = getDimLayer();
 
         assertNotNull("Dimmer should have created a surface", dimLayer);
 
-        verify(mTransaction).setAlpha(dimLayer, alpha);
-        verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, 1);
+        verify(mHost.getPendingTransaction()).setAlpha(dimLayer, alpha);
+        verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, 1);
     }
 
     @Test
@@ -176,13 +177,13 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimBelow(mTransaction, child, alpha, 0);
+        mDimmer.dimBelow(child, alpha, 0);
         SurfaceControl dimLayer = getDimLayer();
 
         assertNotNull("Dimmer should have created a surface", dimLayer);
 
-        verify(mTransaction).setAlpha(dimLayer, alpha);
-        verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, -1);
+        verify(mHost.getPendingTransaction()).setAlpha(dimLayer, alpha);
+        verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, -1);
     }
 
     @Test
@@ -191,7 +192,7 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
         SurfaceControl dimLayer = getDimLayer();
         mDimmer.resetDimStates();
 
@@ -208,10 +209,10 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
         SurfaceControl dimLayer = getDimLayer();
         mDimmer.resetDimStates();
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
 
         mDimmer.updateDims(mTransaction);
         verify(mTransaction).show(dimLayer);
@@ -224,7 +225,7 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final float alpha = 0.8f;
-        mDimmer.dimAbove(mTransaction, child, alpha);
+        mDimmer.dimAbove(child, alpha);
         final Rect bounds = mDimmer.mDimState.mDimBounds;
 
         SurfaceControl dimLayer = getDimLayer();
@@ -245,7 +246,7 @@ public class DimmerTests extends WindowTestsBase {
         TestWindowContainer child = new TestWindowContainer(mWm);
         mHost.addChild(child, 0);
 
-        mDimmer.dimAbove(mTransaction, child, 1);
+        mDimmer.dimAbove(child, 1);
         SurfaceControl dimLayer = getDimLayer();
         mDimmer.updateDims(mTransaction);
         verify(mTransaction, times(1)).show(dimLayer);
@@ -266,13 +267,13 @@ public class DimmerTests extends WindowTestsBase {
         mHost.addChild(child, 0);
 
         final int blurRadius = 50;
-        mDimmer.dimBelow(mTransaction, child, 0, blurRadius);
+        mDimmer.dimBelow(child, 0, blurRadius);
         SurfaceControl dimLayer = getDimLayer();
 
         assertNotNull("Dimmer should have created a surface", dimLayer);
 
-        verify(mTransaction).setBackgroundBlurRadius(dimLayer, blurRadius);
-        verify(mTransaction).setRelativeLayer(dimLayer, child.mControl, -1);
+        verify(mHost.getPendingTransaction()).setBackgroundBlurRadius(dimLayer, blurRadius);
+        verify(mHost.getPendingTransaction()).setRelativeLayer(dimLayer, child.mControl, -1);
     }
 
     private SurfaceControl getDimLayer() {