Merge "Revisit how launch animations handle View visibility" into tm-qpr-dev am: 9758d71bd9

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20946885

Change-Id: I52027fa486dd97ec02eef48798b0b56c7e873fff
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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)
     }
 
     /**

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,15 +299,20 @@ 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
+        if (ghostedView is LaunchableView) {
-        // INVISIBLE then VISIBLE (see b/204944038#comment17 for more info).
+            // 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 {
         private const val CORNER_RADIUS_TOP_INDEX = 0

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
+     * Set whether this view should block/postpone all calls to [View.setVisibility]. This ensures
-     * view:
+     * 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
+     * When `setShouldBlockVisibilityChanges(false)` is called, then visibility of the View should
-     * [transition][android.view.View.setTransitionVisibility] visibility changes must be blocked.
+     * 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,29 +50,33 @@ class LaunchableViewDelegate(
      * super.setVisibility(visibility).
      */
     private val superSetVisibility: (Int) -> Unit,
-
+) : LaunchableView {
-    /**
-     * The lambda that should set the actual transition visibility of [view], usually by calling
-     * super.setTransitionVisibility(visibility).
-     */
-    private val superSetTransitionVisibility: (Int) -> Unit,
-) {
     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 {
+            // 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)
             }
         }
+    }
 
     /** Call this when [View.setVisibility] is called. */
     fun setVisibility(visibility: Int) {
@@ -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)
-    }
 }

packages/SystemUI/animation/src/com/android/systemui/animation/ViewDialogLaunchAnimatorController.kt

@@ -34,24 +34,30 @@ 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.
+        // already done by the launch controller created below.
 
-        // Make sure we allow the source to change its visibility again.
+        if (source is LaunchableView) {
-        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
+            // 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 {
         val delegate = GhostedViewLaunchAnimatorController(source)
@@ -67,13 +73,17 @@ internal constructor(
             override fun onLaunchAnimationEnd(isExpandingFullyAbove: Boolean) {
                 delegate.onLaunchAnimationEnd(isExpandingFullyAbove)
 
-                // We hide the source when the dialog is showing. We will make this view
+                // At this point the view visibility is restored by the delegate, so we delay the
-                // visible again when dismissing the dialog. This does nothing if the source
+                // visibility changes again and make it invisible while the dialog is shown.
-                // implements [LaunchableView], as it's already INVISIBLE in that case.
+                if (source is LaunchableView) {
+                    source.setShouldBlockVisibilityChanges(true)
+                    source.setTransitionVisibility(View.INVISIBLE)
+                } else {
                     source.visibility = View.INVISIBLE
                 }
             }
         }
+    }
 
     override fun createExitController(): LaunchAnimator.Controller {
         return GhostedViewLaunchAnimatorController(source)
@@ -90,15 +100,17 @@ internal constructor(
     }
 
     override fun onExitAnimationCancelled() {
+        if (source is LaunchableView) {
             // Make sure we allow the source to change its visibility again.
-        (source as? LaunchableView)?.setShouldBlockVisibilityChanges(false)
+            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
             }
         }
+    }
 
     override fun jankConfigurationBuilder(): InteractionJankMonitor.Configuration.Builder? {
         val type = cuj?.cujType ?: return null

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)
-    }
 }

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)
-    }
 }

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) {

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);
-    }
 }

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 {
+        return showDialogFromView(touchSurface, animator)
-            val dialog = TestDialog(context)
-            animator.showFromView(dialog, touchSurface)
-            dialog
-        }
     }
 
     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