Revisit how launch animations handle View visibility

This CL refactors the way the ActivityLaunchAnimator and
DialogLaunchAnimator handle the visibility of the animated View when
that View implements LaunchableView.

Before this CL, a View that is animated would always end up being
VISIBLE, even if something made that View INVISIBLE or GONE before or
during the animation. That is because our animation code would sometimes
change the visibility of the View, using View.setVisibility.

After this CL, when animating a LaunchableView, all the visibility
changes made to the animated View are done using
View.setTransitionVisibility instead. That way, a LaunchableView can
track all calls to View.setVisibility only and restore the last
visibility that was set on the View at the end of the animation. Doing
so ensures that all calls made to View.setVisibility have the expected
result once the animation is done.

Bug: 264728445
Test: DialogLaunchAnimatorTest
Change-Id: I5848d42ef94952ecf058d4e895164389e37e0311
(cherry picked from commit 9ecb3db788b9f6be586c0f4f90c19d6337a7cd1b)
Merged-In: I5848d42ef94952ecf058d4e895164389e37e0311

9 files changed, 157 insertions, 88 deletions

diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt
index 54aa3516d867..0f81b0b8d6e0 100644
--- a/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt
+++ b/packages/SystemUI/animation/src/com/android/systemui/animation/DialogLaunchAnimator.kt
@@ -791,13 +791,13 @@ private class AnimatedDialog(
         // Move the drawing of the source in the overlay of this dialog, then animate. We trigger a
         // one-off synchronization to make sure that this is done in sync between the two different
         // windows.
+        controller.startDrawingInOverlayOf(decorView)
         synchronizeNextDraw(
             then = {
                 isSourceDrawnInDialog = true
                 maybeStartLaunchAnimation()
             }
         )
-        controller.startDrawingInOverlayOf(decorView)
     }
 
     /**
diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/GhostedViewLaunchAnimatorController.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/GhostedViewLaunchAnimatorController.kt
index 0028d13ffd5e..dfac02d99c4d 100644
--- a/packages/SystemUI/animation/src/com/android/systemui/animation/GhostedViewLaunchAnimatorController.kt
+++ b/packages/SystemUI/animation/src/com/android/systemui/animation/GhostedViewLaunchAnimatorController.kt
@@ -195,14 +195,16 @@ open class GhostedViewLaunchAnimatorController @JvmOverloads constructor(
         backgroundDrawable = WrappedDrawable(background)
         backgroundView?.background = backgroundDrawable
 
+        // Delay the calls to `ghostedView.setVisibility()` during the animation. This must be
+        // called before `GhostView.addGhost()` is called because the latter will change the
+        // *transition* visibility, which won't be blocked and will affect the normal View
+        // visibility that is saved by `setShouldBlockVisibilityChanges()` for a later restoration.
+        (ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
+
         // Create a ghost of the view that will be moving and fading out. This allows to fade out
         // the content before fading out the background.
         ghostView = GhostView.addGhost(ghostedView, launchContainer)
 
-        // The ghost was just created, so ghostedView is currently invisible. We need to make sure
-        // that it stays invisible as long as we are animating.
-        (ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
-
         val matrix = ghostView?.animationMatrix ?: Matrix.IDENTITY_MATRIX
         matrix.getValues(initialGhostViewMatrixValues)
 
@@ -297,14 +299,19 @@ open class GhostedViewLaunchAnimatorController @JvmOverloads constructor(
         backgroundDrawable?.wrapped?.alpha = startBackgroundAlpha
 
         GhostView.removeGhost(ghostedView)
-        (ghostedView as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
         launchContainerOverlay.remove(backgroundView)
 
-        // Make sure that the view is considered VISIBLE by accessibility by first making it
-        // INVISIBLE then VISIBLE (see b/204944038#comment17 for more info).
-        ghostedView.visibility = View.INVISIBLE
-        ghostedView.visibility = View.VISIBLE
-        ghostedView.invalidate()
+        if (ghostedView is LaunchableView) {
+            // Restore the ghosted view visibility.
+            ghostedView.setShouldBlockVisibilityChanges(false)
+        } else {
+            // Make the ghosted view visible. We ensure that the view is considered VISIBLE by
+            // accessibility by first making it INVISIBLE then VISIBLE (see b/204944038#comment17
+            // for more info).
+            ghostedView.visibility = View.INVISIBLE
+            ghostedView.visibility = View.VISIBLE
+            ghostedView.invalidate()
+        }
     }
 
     companion object {
diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/LaunchableView.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/LaunchableView.kt
index 67b59e0e9928..774255be4007 100644
--- a/packages/SystemUI/animation/src/com/android/systemui/animation/LaunchableView.kt
+++ b/packages/SystemUI/animation/src/com/android/systemui/animation/LaunchableView.kt
@@ -21,15 +21,19 @@ import android.view.View
 /** A view that can expand/launch into an app or a dialog. */
 interface LaunchableView {
     /**
-     * Set whether this view should block/postpone all visibility changes. This ensures that this
-     * view:
+     * Set whether this view should block/postpone all calls to [View.setVisibility]. This ensures
+     * that this view:
      * - remains invisible during the launch animation given that it is ghosted and already drawn
      * somewhere else.
      * - remains invisible as long as a dialog expanded from it is shown.
      * - restores its expected visibility once the dialog expanded from it is dismissed.
      *
-     * Note that when this is set to true, both the [normal][android.view.View.setVisibility] and
-     * [transition][android.view.View.setTransitionVisibility] visibility changes must be blocked.
+     * When `setShouldBlockVisibilityChanges(false)` is called, then visibility of the View should
+     * be restored to its expected value, i.e. it should have the visibility of the last call to
+     * `View.setVisibility()` that was made after `setShouldBlockVisibilityChanges(true)`, if any,
+     * or the original view visibility otherwise.
+     *
+     * Note that calls to [View.setTransitionVisibility] shouldn't be blocked.
      *
      * @param block whether we should block/postpone all calls to `setVisibility` and
      * `setTransitionVisibility`.
@@ -46,27 +50,31 @@ class LaunchableViewDelegate(
      * super.setVisibility(visibility).
      */
     private val superSetVisibility: (Int) -> Unit,
-
-    /**
-     * The lambda that should set the actual transition visibility of [view], usually by calling
-     * super.setTransitionVisibility(visibility).
-     */
-    private val superSetTransitionVisibility: (Int) -> Unit,
-) {
+) : LaunchableView {
     private var blockVisibilityChanges = false
     private var lastVisibility = view.visibility
 
     /** Call this when [LaunchableView.setShouldBlockVisibilityChanges] is called. */
-    fun setShouldBlockVisibilityChanges(block: Boolean) {
+    override fun setShouldBlockVisibilityChanges(block: Boolean) {
         if (block == blockVisibilityChanges) {
             return
         }
 
         blockVisibilityChanges = block
         if (block) {
+            // Save the current visibility for later.
             lastVisibility = view.visibility
         } else {
-            superSetVisibility(lastVisibility)
+            // Restore the visibility. To avoid accessibility issues, we change the visibility twice
+            // which makes sure that we trigger a visibility flag change (see b/204944038#comment17
+            // for more info).
+            if (lastVisibility == View.VISIBLE) {
+                superSetVisibility(View.INVISIBLE)
+                superSetVisibility(View.VISIBLE)
+            } else {
+                superSetVisibility(View.VISIBLE)
+                superSetVisibility(lastVisibility)
+            }
         }
     }
 
@@ -79,16 +87,4 @@ class LaunchableViewDelegate(
 
         superSetVisibility(visibility)
     }
-
-    /** Call this when [View.setTransitionVisibility] is called. */
-    fun setTransitionVisibility(visibility: Int) {
-        if (blockVisibilityChanges) {
-            // View.setTransitionVisibility just sets the visibility flag, so we don't have to save
-            // the transition visibility separately from the normal visibility.
-            lastVisibility = visibility
-            return
-        }
-
-        superSetTransitionVisibility(visibility)
-    }
 }
diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewDialogLaunchAnimatorController.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewDialogLaunchAnimatorController.kt
index 964ef8c88098..46d5a5c0af8c 100644
--- a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewDialogLaunchAnimatorController.kt
+++ b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewDialogLaunchAnimatorController.kt
@@ -34,23 +34,29 @@ internal constructor(
     override val sourceIdentity: Any = source
 
     override fun startDrawingInOverlayOf(viewGroup: ViewGroup) {
+        // Delay the calls to `source.setVisibility()` during the animation. This must be called
+        // before `GhostView.addGhost()` is called because the latter will change the *transition*
+        // visibility, which won't be blocked and will affect the normal View visibility that is
+        // saved by `setShouldBlockVisibilityChanges()` for a later restoration.
+        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
+
         // Create a temporary ghost of the source (which will make it invisible) and add it
         // to the host dialog.
         GhostView.addGhost(source, viewGroup)
-
-        // The ghost of the source was just created, so the source is currently invisible.
-        // We need to make sure that it stays invisible as long as the dialog is shown or
-        // animating.
-        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(true)
     }
 
     override fun stopDrawingInOverlay() {
         // Note: here we should remove the ghost from the overlay, but in practice this is
-        // already done by the launch controllers created below.
-
-        // Make sure we allow the source to change its visibility again.
-        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
-        source.visibility = View.VISIBLE
+        // already done by the launch controller created below.
+
+        if (source is LaunchableView) {
+            // Make sure we allow the source to change its visibility again and restore its previous
+            // value.
+            source.setShouldBlockVisibilityChanges(false)
+        } else {
+            // We made the source invisible earlier, so let's make it visible again.
+            source.visibility = View.VISIBLE
+        }
     }
 
     override fun createLaunchController(): LaunchAnimator.Controller {
@@ -67,10 +73,14 @@ internal constructor(
             override fun onLaunchAnimationEnd(isExpandingFullyAbove: Boolean) {
                 delegate.onLaunchAnimationEnd(isExpandingFullyAbove)
 
-                // We hide the source when the dialog is showing. We will make this view
-                // visible again when dismissing the dialog. This does nothing if the source
-                // implements [LaunchableView], as it's already INVISIBLE in that case.
-                source.visibility = View.INVISIBLE
+                // At this point the view visibility is restored by the delegate, so we delay the
+                // visibility changes again and make it invisible while the dialog is shown.
+                if (source is LaunchableView) {
+                    source.setShouldBlockVisibilityChanges(true)
+                    source.setTransitionVisibility(View.INVISIBLE)
+                } else {
+                    source.visibility = View.INVISIBLE
+                }
             }
         }
     }
@@ -90,13 +100,15 @@ internal constructor(
     }
 
     override fun onExitAnimationCancelled() {
-        // Make sure we allow the source to change its visibility again.
-        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
-
-        // If the view is invisible it's probably because of us, so we make it visible
-        // again.
-        if (source.visibility == View.INVISIBLE) {
-            source.visibility = View.VISIBLE
+        if (source is LaunchableView) {
+            // Make sure we allow the source to change its visibility again.
+            source.setShouldBlockVisibilityChanges(false)
+        } else {
+            // If the view is invisible it's probably because of us, so we make it visible
+            // again.
+            if (source.visibility == View.INVISIBLE) {
+                source.visibility = View.VISIBLE
+            }
         }
     }
 
diff --git a/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableImageView.kt b/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableImageView.kt
index f95a8ee89a2c..7bbfec7df9d8 100644
--- a/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableImageView.kt
+++ b/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableImageView.kt
@@ -28,7 +28,6 @@ class LaunchableImageView : ImageView, LaunchableView {
         LaunchableViewDelegate(
             this,
             superSetVisibility = { super.setVisibility(it) },
-            superSetTransitionVisibility = { super.setTransitionVisibility(it) },
         )
 
     constructor(context: Context?) : super(context)
@@ -53,8 +52,4 @@ class LaunchableImageView : ImageView, LaunchableView {
     override fun setVisibility(visibility: Int) {
         delegate.setVisibility(visibility)
     }
-
-    override fun setTransitionVisibility(visibility: Int) {
-        delegate.setTransitionVisibility(visibility)
-    }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableLinearLayout.kt b/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableLinearLayout.kt
index c27b82aeeb47..ddde6280f3a2 100644
--- a/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableLinearLayout.kt
+++ b/packages/SystemUI/src/com/android/systemui/common/ui/view/LaunchableLinearLayout.kt
@@ -28,7 +28,6 @@ class LaunchableLinearLayout : LinearLayout, LaunchableView {
         LaunchableViewDelegate(
             this,
             superSetVisibility = { super.setVisibility(it) },
-            superSetTransitionVisibility = { super.setTransitionVisibility(it) },
         )
 
     constructor(context: Context?) : super(context)
@@ -53,8 +52,4 @@ class LaunchableLinearLayout : LinearLayout, LaunchableView {
     override fun setVisibility(visibility: Int) {
         delegate.setVisibility(visibility)
     }
-
-    override fun setTransitionVisibility(visibility: Int) {
-        delegate.setTransitionVisibility(visibility)
-    }
 }
diff --git a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
index b355d4bb67fe..29d7fb02e613 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
@@ -145,7 +145,6 @@ open class QSTileViewImpl @JvmOverloads constructor(
     private val launchableViewDelegate = LaunchableViewDelegate(
         this,
         superSetVisibility = { super.setVisibility(it) },
-        superSetTransitionVisibility = { super.setTransitionVisibility(it) },
     )
     private var lastDisabledByPolicy = false
 
@@ -362,10 +361,6 @@ open class QSTileViewImpl @JvmOverloads constructor(
         launchableViewDelegate.setVisibility(visibility)
     }
 
-    override fun setTransitionVisibility(visibility: Int) {
-        launchableViewDelegate.setTransitionVisibility(visibility)
-    }
-
     // Accessibility
 
     override fun onInitializeAccessibilityEvent(event: AccessibilityEvent) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/AlphaOptimizedFrameLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/AlphaOptimizedFrameLayout.java
index 662f70ef269e..438b0f625fc5 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/AlphaOptimizedFrameLayout.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/AlphaOptimizedFrameLayout.java
@@ -36,10 +36,6 @@ public class AlphaOptimizedFrameLayout extends FrameLayout implements Launchable
             visibility -> {
                 super.setVisibility(visibility);
                 return Unit.INSTANCE;
-            },
-            visibility -> {
-                super.setTransitionVisibility(visibility);
-                return Unit.INSTANCE;
             });
 
     public AlphaOptimizedFrameLayout(Context context) {
@@ -73,9 +69,4 @@ public class AlphaOptimizedFrameLayout extends FrameLayout implements Launchable
     public void setVisibility(int visibility) {
         mLaunchableViewDelegate.setVisibility(visibility);
     }
-
-    @Override
-    public void setTransitionVisibility(int visibility) {
-        mLaunchableViewDelegate.setTransitionVisibility(visibility);
-    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt
index 7c1e384f8c30..cac4a0e5432c 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/animation/DialogLaunchAnimatorTest.kt
@@ -12,11 +12,13 @@ import android.view.View
 import android.view.ViewGroup
 import android.view.ViewGroup.LayoutParams.MATCH_PARENT
 import android.view.WindowManager
+import android.widget.FrameLayout
 import android.widget.LinearLayout
 import androidx.test.filters.SmallTest
 import com.android.internal.jank.InteractionJankMonitor
 import com.android.internal.policy.DecorView
 import com.android.systemui.SysuiTestCase
+import com.google.common.truth.Truth.assertThat
 import junit.framework.Assert.assertEquals
 import junit.framework.Assert.assertFalse
 import junit.framework.Assert.assertNotNull
@@ -205,25 +207,74 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
         verify(interactionJankMonitor).end(InteractionJankMonitor.CUJ_USER_DIALOG_OPEN)
     }
 
+    @Test
+    fun testAnimationDoesNotChangeLaunchableViewVisibility_viewVisible() {
+        val touchSurface = createTouchSurface()
+
+        // View is VISIBLE when starting the animation.
+        runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.VISIBLE }
+
+        // View is invisible while the dialog is shown.
+        val dialog = showDialogFromView(touchSurface)
+        assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
+
+        // View is visible again when the dialog is dismissed.
+        runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
+        assertThat(touchSurface.visibility).isEqualTo(View.VISIBLE)
+    }
+
+    @Test
+    fun testAnimationDoesNotChangeLaunchableViewVisibility_viewInvisible() {
+        val touchSurface = createTouchSurface()
+
+        // View is INVISIBLE when starting the animation.
+        runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.INVISIBLE }
+
+        // View is INVISIBLE while the dialog is shown.
+        val dialog = showDialogFromView(touchSurface)
+        assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
+
+        // View is invisible like it was before showing the dialog.
+        runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
+        assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
+    }
+
+    @Test
+    fun testAnimationDoesNotChangeLaunchableViewVisibility_viewVisibleThenGone() {
+        val touchSurface = createTouchSurface()
+
+        // View is VISIBLE when starting the animation.
+        runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.VISIBLE }
+
+        // View is INVISIBLE while the dialog is shown.
+        val dialog = showDialogFromView(touchSurface)
+        assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
+
+        // Some external call makes the View GONE. It remains INVISIBLE while the dialog is shown,
+        // as all visibility changes should be blocked.
+        runOnMainThreadAndWaitForIdleSync { touchSurface.visibility = View.GONE }
+        assertThat(touchSurface.visibility).isEqualTo(View.INVISIBLE)
+
+        // View is restored to GONE once the dialog is dismissed.
+        runOnMainThreadAndWaitForIdleSync { dialog.dismiss() }
+        assertThat(touchSurface.visibility).isEqualTo(View.GONE)
+    }
+
     private fun createAndShowDialog(
         animator: DialogLaunchAnimator = dialogLaunchAnimator,
     ): TestDialog {
         val touchSurface = createTouchSurface()
-        return runOnMainThreadAndWaitForIdleSync {
-            val dialog = TestDialog(context)
-            animator.showFromView(dialog, touchSurface)
-            dialog
-        }
+        return showDialogFromView(touchSurface, animator)
     }
 
     private fun createTouchSurface(): View {
         return runOnMainThreadAndWaitForIdleSync {
             val touchSurfaceRoot = LinearLayout(context)
-            val touchSurface = View(context)
+            val touchSurface = TouchSurfaceView(context)
             touchSurfaceRoot.addView(touchSurface)
 
             // We need to attach the root to the window manager otherwise the exit animation will
-            // be skipped
+            // be skipped.
             ViewUtils.attachView(touchSurfaceRoot)
             attachedViews.add(touchSurfaceRoot)
 
@@ -231,6 +282,17 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
         }
     }
 
+    private fun showDialogFromView(
+        touchSurface: View,
+        animator: DialogLaunchAnimator = dialogLaunchAnimator,
+    ): TestDialog {
+        return runOnMainThreadAndWaitForIdleSync {
+            val dialog = TestDialog(context)
+            animator.showFromView(dialog, touchSurface)
+            dialog
+        }
+    }
+
     private fun createDialogAndShowFromDialog(animateFrom: Dialog): TestDialog {
         return runOnMainThreadAndWaitForIdleSync {
             val dialog = TestDialog(context)
@@ -248,6 +310,22 @@ class DialogLaunchAnimatorTest : SysuiTestCase() {
         return result
     }
 
+    private class TouchSurfaceView(context: Context) : FrameLayout(context), LaunchableView {
+        private val delegate =
+            LaunchableViewDelegate(
+                this,
+                superSetVisibility = { super.setVisibility(it) },
+            )
+
+        override fun setShouldBlockVisibilityChanges(block: Boolean) {
+            delegate.setShouldBlockVisibilityChanges(block)
+        }
+
+        override fun setVisibility(visibility: Int) {
+            delegate.setVisibility(visibility)
+        }
+    }
+
     private class TestDialog(context: Context) : Dialog(context) {
         companion object {
             const val DIALOG_WIDTH = 100