Do not dispatch leashes if they are not ready

The previous solution used a different transaction from the one used in
SeamlessRotator#finish. So the transaction might be applied in an
unexpected order between here and there when the target frame number is
reached.

This CL reverted the previous solution. Instead, we don't dispatch the
leash if it is not ready. For the client, it won't play the animation
until obtaining the leash.

Fix: 154195854
Test: Rotate device to change the orientation in Camera, and see if
      navigation bar stays visible.
Test: Check if transient bar can be shown/hidden/aborted as expected.
Test: Make sure b/153104643 stay fixed.
Test: atest InsetsPolicyTest
Change-Id: I29f80f1c77615b0a3cde38df265220f48d66f117

7 files changed, 97 insertions, 12 deletions

diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java
index a72383913420..2dcfd899adf4 100644
--- a/core/java/android/view/InsetsSourceConsumer.java
+++ b/core/java/android/view/InsetsSourceConsumer.java
@@ -69,6 +69,13 @@ public class InsetsSourceConsumer {
     private Rect mPendingFrame;
     private Rect mPendingVisibleFrame;
 
+    /**
+     * Indicates if we have the pending animation. When we have the control, we need to play the
+     * animation if the requested visibility is different from the current state. But if we haven't
+     * had a leash yet, we will set this flag, and play the animation once we get the leash.
+     */
+    private boolean mIsAnimationPending;
+
     public InsetsSourceConsumer(@InternalInsetsType int type, InsetsState state,
             Supplier<Transaction> transactionSupplier, InsetsController controller) {
         mType = type;
@@ -107,13 +114,21 @@ public class InsetsSourceConsumer {
         } else {
             // We are gaining control, and need to run an animation since previous state
             // didn't match
-            if (isRequestedVisibleAwaitingControl() != mState.getSource(mType).isVisible()) {
-                if (isRequestedVisibleAwaitingControl()) {
+            final boolean requestedVisible = isRequestedVisibleAwaitingControl();
+            final boolean needAnimation = requestedVisible != mState.getSource(mType).isVisible();
+            if (control.getLeash() != null && (needAnimation || mIsAnimationPending)) {
+                if (requestedVisible) {
                     showTypes[0] |= toPublicType(getType());
                 } else {
                     hideTypes[0] |= toPublicType(getType());
                 }
+                mIsAnimationPending = false;
             } else {
+                if (needAnimation) {
+                    // We need animation but we haven't had a leash yet. Set this flag that when we
+                    // get the leash we can play the deferred animation.
+                    mIsAnimationPending = true;
+                }
                 // We are gaining control, but don't need to run an animation.
                 // However make sure that the leash visibility is still up to date.
                 if (applyLocalVisibilityOverride()) {
@@ -274,7 +289,10 @@ public class InsetsSourceConsumer {
      * the moment.
      */
     protected void setRequestedVisible(boolean requestedVisible) {
-        mRequestedVisible = requestedVisible;
+        if (mRequestedVisible != requestedVisible) {
+            mRequestedVisible = requestedVisible;
+            mIsAnimationPending = false;
+        }
         if (applyLocalVisibilityOverride()) {
             mController.notifyVisibilityChanged();
         }
diff --git a/services/core/java/com/android/server/wm/InsetsPolicy.java b/services/core/java/com/android/server/wm/InsetsPolicy.java
index 61a199a816df..5a9bf809fa4a 100644
--- a/services/core/java/com/android/server/wm/InsetsPolicy.java
+++ b/services/core/java/com/android/server/wm/InsetsPolicy.java
@@ -126,13 +126,22 @@ class InsetsPolicy {
             mPolicy.getStatusBarManagerInternal().showTransient(mDisplayContent.getDisplayId(),
                     mShowingTransientTypes.toArray());
             updateBarControlTarget(mFocusedWin);
-            InsetsState state = new InsetsState(mStateController.getRawInsetsState());
-            startAnimation(true /* show */, () -> {
+
+            // The leashes can be created while updating bar control target. The surface transaction
+            // of the new leashes might not be applied yet. The callback posted here ensures we can
+            // get the valid leashes because the surface transaction will be applied in the next
+            // animation frame which will be triggered if a new leash is created.
+            mDisplayContent.mWmService.mAnimator.getChoreographer().postFrameCallback(time -> {
                 synchronized (mDisplayContent.mWmService.mGlobalLock) {
-                    mStateController.notifyInsetsChanged();
+                    final InsetsState state = new InsetsState(mStateController.getRawInsetsState());
+                    startAnimation(true /* show */, () -> {
+                        synchronized (mDisplayContent.mWmService.mGlobalLock) {
+                            mStateController.notifyInsetsChanged();
+                        }
+                    }, state);
+                    mStateController.onInsetsModified(mDummyControlTarget, state);
                 }
-            }, state);
-            mStateController.onInsetsModified(mDummyControlTarget, state);
+            });
         }
     }
 
diff --git a/services/core/java/com/android/server/wm/InsetsSourceProvider.java b/services/core/java/com/android/server/wm/InsetsSourceProvider.java
index 1ca82ceeb570..351743f962b9 100644
--- a/services/core/java/com/android/server/wm/InsetsSourceProvider.java
+++ b/services/core/java/com/android/server/wm/InsetsSourceProvider.java
@@ -66,6 +66,7 @@ class InsetsSourceProvider {
     private TriConsumer<DisplayFrames, WindowState, Rect> mFrameProvider;
     private TriConsumer<DisplayFrames, WindowState, Rect> mImeFrameProvider;
     private final Rect mImeOverrideFrame = new Rect();
+    private boolean mIsLeashReadyForDispatching;
 
     /** The visibility override from the current controlling window. */
     private boolean mClientVisible;
@@ -266,9 +267,14 @@ class InsetsSourceProvider {
         if (getSource().getType() == ITYPE_IME) {
             setClientVisible(InsetsState.getDefaultVisibility(mSource.getType()));
         }
-        final Transaction t = mDisplayContent.mWmService.mTransactionFactory.get();
+        final Transaction t = mDisplayContent.getPendingTransaction();
         mWin.startAnimation(t, mAdapter, !mClientVisible /* hidden */,
                 ANIMATION_TYPE_INSETS_CONTROL, null /* animationFinishedCallback */);
+
+        // The leash was just created. We cannot dispatch it until its surface transaction is
+        // applied. Otherwise, the client's operation to the leash might be overwritten by us.
+        mIsLeashReadyForDispatching = false;
+
         final SurfaceControl leash = mAdapter.mCapturedLeash;
         final long frameNumber = mFinishSeamlessRotateFrameNumber;
         mFinishSeamlessRotateFrameNumber = -1;
@@ -281,9 +287,6 @@ class InsetsSourceProvider {
             t.deferTransactionUntil(mWin.getSurfaceControl(), barrier, frameNumber);
             t.deferTransactionUntil(leash, barrier, frameNumber);
         }
-        // Applying the transaction here can prevent the client from applying its transaction sooner
-        // than us which makes us overwrite the client's operation to the leash.
-        t.apply();
         mControlTarget = target;
         mControl = new InsetsSourceControl(mSource.getType(), leash,
                 new Point(mWin.getWindowFrames().mFrame.left, mWin.getWindowFrames().mFrame.top));
@@ -313,6 +316,10 @@ class InsetsSourceProvider {
         return true;
     }
 
+    void onSurfaceTransactionApplied() {
+        mIsLeashReadyForDispatching = true;
+    }
+
     private void setClientVisible(boolean clientVisible) {
         if (mClientVisible == clientVisible) {
             return;
@@ -334,6 +341,13 @@ class InsetsSourceProvider {
 
     InsetsSourceControl getControl(InsetsControlTarget target) {
         if (target == mControlTarget) {
+            if (!mIsLeashReadyForDispatching && mControl != null) {
+                // The surface transaction of preparing leash is not applied yet. We don't send it
+                // to the client in case that the client applies its transaction sooner than ours
+                // that we could unexpectedly overwrite the surface state.
+                return new InsetsSourceControl(mControl.getType(), null /* leash */,
+                        mControl.getSurfacePosition());
+            }
             return mControl;
         }
         if (target == mFakeControlTarget) {
diff --git a/services/core/java/com/android/server/wm/InsetsStateController.java b/services/core/java/com/android/server/wm/InsetsStateController.java
index 4ac319ddf6ce..765f98065dd0 100644
--- a/services/core/java/com/android/server/wm/InsetsStateController.java
+++ b/services/core/java/com/android/server/wm/InsetsStateController.java
@@ -407,6 +407,10 @@ class InsetsStateController {
             return;
         }
         mDisplayContent.mWmService.mAnimator.addAfterPrepareSurfacesRunnable(() -> {
+            for (int i = mProviders.size() - 1; i >= 0; i--) {
+                final InsetsSourceProvider provider = mProviders.valueAt(i);
+                provider.onSurfaceTransactionApplied();
+            }
             for (int i = mPendingControlChanged.size() - 1; i >= 0; i--) {
                 final InsetsControlTarget controlTarget = mPendingControlChanged.valueAt(i);
                 controlTarget.notifyInsetsControlChanged();
diff --git a/services/tests/wmtests/src/com/android/server/wm/InsetsPolicyTest.java b/services/tests/wmtests/src/com/android/server/wm/InsetsPolicyTest.java
index b6eb9010ce90..f83128705a2a 100644
--- a/services/tests/wmtests/src/com/android/server/wm/InsetsPolicyTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/InsetsPolicyTest.java
@@ -50,6 +50,7 @@ import android.view.test.InsetsModeSession;
 import androidx.test.filters.SmallTest;
 
 import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -72,6 +73,11 @@ public class InsetsPolicyTest extends WindowTestsBase {
         sInsetsModeSession.close();
     }
 
+    @Before
+    public void setup() {
+        mWm.mAnimator.ready();
+    }
+
     @Test
     public void testControlsForDispatch_regular() {
         addWindow(TYPE_STATUS_BAR, "statusBar");
@@ -194,6 +200,7 @@ public class InsetsPolicyTest extends WindowTestsBase {
         policy.updateBarControlTarget(mAppWindow);
         policy.showTransient(
                 IntArray.wrap(new int[]{ITYPE_STATUS_BAR, ITYPE_NAVIGATION_BAR}));
+        waitUntilWindowAnimatorIdle();
         final InsetsSourceControl[] controls =
                 mDisplayContent.getInsetsStateController().getControlsForDispatch(mAppWindow);
 
@@ -221,6 +228,7 @@ public class InsetsPolicyTest extends WindowTestsBase {
         policy.updateBarControlTarget(mAppWindow);
         policy.showTransient(
                 IntArray.wrap(new int[]{ITYPE_STATUS_BAR, ITYPE_NAVIGATION_BAR}));
+        waitUntilWindowAnimatorIdle();
         final InsetsSourceControl[] controls =
                 mDisplayContent.getInsetsStateController().getControlsForDispatch(mAppWindow);
 
@@ -249,6 +257,7 @@ public class InsetsPolicyTest extends WindowTestsBase {
         policy.updateBarControlTarget(mAppWindow);
         policy.showTransient(
                 IntArray.wrap(new int[]{ITYPE_STATUS_BAR, ITYPE_NAVIGATION_BAR}));
+        waitUntilWindowAnimatorIdle();
         InsetsSourceControl[] controls =
                 mDisplayContent.getInsetsStateController().getControlsForDispatch(mAppWindow);
 
@@ -262,6 +271,7 @@ public class InsetsPolicyTest extends WindowTestsBase {
         state.setSourceVisible(ITYPE_STATUS_BAR, true);
         state.setSourceVisible(ITYPE_NAVIGATION_BAR, true);
         policy.onInsetsModified(mAppWindow, state);
+        waitUntilWindowAnimatorIdle();
 
         controls = mDisplayContent.getInsetsStateController().getControlsForDispatch(mAppWindow);
 
diff --git a/services/tests/wmtests/src/com/android/server/wm/SystemServiceTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/SystemServiceTestsBase.java
index f6ed31455f18..d7462f810bb7 100644
--- a/services/tests/wmtests/src/com/android/server/wm/SystemServiceTestsBase.java
+++ b/services/tests/wmtests/src/com/android/server/wm/SystemServiceTestsBase.java
@@ -40,6 +40,11 @@ class SystemServiceTestsBase {
         mLockRule.waitForLocked(mSystemServicesTestRule::waitUntilWindowManagerHandlersIdle);
     }
 
+    /** Waits until the choreographer of WindowAnimator has processed all callbacks. */
+    void waitUntilWindowAnimatorIdle() {
+        mLockRule.waitForLocked(mSystemServicesTestRule::waitUntilWindowAnimatorIdle);
+    }
+
     void cleanupWindowManagerHandlers() {
         mLockRule.waitForLocked(mSystemServicesTestRule::cleanupWindowManagerHandlers);
     }
diff --git a/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java b/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java
index af3ec38631ae..dea9294ff75b 100644
--- a/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java
+++ b/services/tests/wmtests/src/com/android/server/wm/SystemServicesTestRule.java
@@ -429,6 +429,31 @@ public class SystemServicesTestRule implements TestRule {
         }
     }
 
+    void waitUntilWindowAnimatorIdle() {
+        final WindowManagerService wm = getWindowManagerService();
+        if (wm == null) {
+            return;
+        }
+        synchronized (mCurrentMessagesProcessed) {
+            // Add a message to the handler queue and make sure it is fully processed before we move
+            // on. This makes sure all previous messages in the handler are fully processed vs. just
+            // popping them from the message queue.
+            mCurrentMessagesProcessed.set(false);
+            wm.mAnimator.getChoreographer().postFrameCallback(time -> {
+                synchronized (mCurrentMessagesProcessed) {
+                    mCurrentMessagesProcessed.set(true);
+                    mCurrentMessagesProcessed.notifyAll();
+                }
+            });
+            while (!mCurrentMessagesProcessed.get()) {
+                try {
+                    mCurrentMessagesProcessed.wait();
+                } catch (InterruptedException e) {
+                }
+            }
+        }
+    }
+
     /**
      * Throws if caller doesn't hold the given lock.
      * @param lock the lock