Merge "YouTube and YTMusic trigger a race which causes the AnimatorSet to hang" into tm-dev

packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java

@@ -50,7 +50,6 @@ import android.util.Log;
 import android.util.Pair;
 import android.view.View;
 import android.view.ViewGroup;
-import android.view.ViewTreeObserver;
 import android.view.animation.Interpolator;
 import android.widget.ImageButton;
 import android.widget.ImageView;
@@ -553,30 +552,6 @@ public class MediaControlPanel {
 
                 // refreshState is required here to resize the text views (and prevent ellipsis)
                 mMediaViewController.refreshState();
-
-                // Use OnPreDrawListeners to enforce zero alpha on these views for a frame.
-                // TransitionLayout insists on resetting the alpha of these views to 1 when onLayout
-                // is called which causes the animation to look bad. These suppress that behavior.
-                titleText.getViewTreeObserver().addOnPreDrawListener(
-                        new ViewTreeObserver.OnPreDrawListener() {
-                            @Override
-                            public boolean onPreDraw() {
-                                titleText.setAlpha(0);
-                                titleText.getViewTreeObserver().removeOnPreDrawListener(this);
-                                return true;
-                            }
-                        });
-
-                artistText.getViewTreeObserver().addOnPreDrawListener(
-                        new ViewTreeObserver.OnPreDrawListener() {
-                            @Override
-                            public boolean onPreDraw() {
-                                artistText.setAlpha(0);
-                                artistText.getViewTreeObserver().removeOnPreDrawListener(this);
-                                return true;
-                            }
-                        });
-
                 return Unit.INSTANCE;
             },
             () -> {

packages/SystemUI/src/com/android/systemui/media/MetadataAnimationHandler.kt

@@ -18,8 +18,6 @@ package com.android.systemui.media
 
 import android.animation.Animator
 import android.animation.AnimatorListenerAdapter
-import android.animation.AnimatorSet
-import com.android.internal.annotations.VisibleForTesting
 
 /**
  * MetadataAnimationHandler controls the current state of the MediaControlPanel's transition motion.
@@ -33,37 +31,37 @@ internal open class MetadataAnimationHandler(
     private val enterAnimator: Animator
 ) : AnimatorListenerAdapter() {
 
-    private val animator: AnimatorSet
     private var postExitUpdate: (() -> Unit)? = null
     private var postEnterUpdate: (() -> Unit)? = null
     private var targetData: Any? = null
 
     val isRunning: Boolean
-        get() = animator.isRunning
+        get() = enterAnimator.isRunning || exitAnimator.isRunning
 
     fun setNext(targetData: Any, postExit: () -> Unit, postEnter: () -> Unit): Boolean {
         if (targetData != this.targetData) {
             this.targetData = targetData
             postExitUpdate = postExit
             postEnterUpdate = postEnter
-            if (!animator.isRunning) {
+            if (!isRunning) {
-                animator.start()
+                exitAnimator.start()
             }
             return true
         }
         return false
     }
 
-    override fun onAnimationEnd(animator: Animator) {
+    override fun onAnimationEnd(anim: Animator) {
-        if (animator === exitAnimator) {
+        if (anim === exitAnimator) {
             postExitUpdate?.let { it() }
             postExitUpdate = null
+            enterAnimator.start()
         }
 
-        if (animator === enterAnimator) {
+        if (anim === enterAnimator) {
             // Another new update appeared while entering
             if (postExitUpdate != null) {
-                this.animator.start()
+                exitAnimator.start()
             } else {
                 postEnterUpdate?.let { it() }
                 postEnterUpdate = null
@@ -74,13 +72,5 @@ internal open class MetadataAnimationHandler(
     init {
         exitAnimator.addListener(this)
         enterAnimator.addListener(this)
-        animator = buildAnimatorSet(exitAnimator, enterAnimator)
-    }
-
-    @VisibleForTesting
-    protected open fun buildAnimatorSet(exit: Animator, enter: Animator): AnimatorSet {
-        val result = AnimatorSet()
-        result.playSequentially(exitAnimator, enterAnimator)
-        return result
     }
 }

packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt

@@ -947,7 +947,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
 
         // Rebinding should not trigger animation
         player.bindPlayer(mediaData, PACKAGE)
-        verify(mockAnimator, times(1)).start()
+        verify(mockAnimator, times(2)).start()
     }
 
     @Test
@@ -969,7 +969,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
 
         // Bind trigges new animation
         player.bindPlayer(data1, PACKAGE)
-        verify(mockAnimator, times(2)).start()
+        verify(mockAnimator, times(3)).start()
         whenever(mockAnimator.isRunning()).thenReturn(true)
 
         // Rebind before animation end binds corrct data

packages/SystemUI/tests/src/com/android/systemui/media/MetadataAnimationHandlerTest.kt

@@ -18,7 +18,6 @@ package com.android.systemui.media
 
 import org.mockito.Mockito.`when` as whenever
 import android.animation.Animator
-import android.animation.AnimatorSet
 import android.test.suitebuilder.annotation.SmallTest
 import android.testing.AndroidTestingRunner
 import android.testing.TestableLooper
@@ -44,7 +43,6 @@ class MetadataAnimationHandlerTest : SysuiTestCase() {
     private interface Callback : () -> Unit
     private lateinit var handler: MetadataAnimationHandler
 
-    @Mock private lateinit var animatorSet: AnimatorSet
     @Mock private lateinit var enterAnimator: Animator
     @Mock private lateinit var exitAnimator: Animator
     @Mock private lateinit var postExitCB: Callback
@@ -54,11 +52,7 @@ class MetadataAnimationHandlerTest : SysuiTestCase() {
 
     @Before
     fun setUp() {
-        handler = object : MetadataAnimationHandler(exitAnimator, enterAnimator) {
+        handler = MetadataAnimationHandler(exitAnimator, enterAnimator)
-            override fun buildAnimatorSet(exit: Animator, enter: Animator): AnimatorSet {
-                return animatorSet
-            }
-        }
     }
 
     @After
@@ -69,22 +63,31 @@ class MetadataAnimationHandlerTest : SysuiTestCase() {
         val cb = { fail("Unexpected callback") }
         handler.setNext("data-1", cb, cb)
 
-        verify(animatorSet).start()
+        verify(exitAnimator).start()
     }
 
     @Test
     fun executeAnimationEnd_runsCallacks() {
+        // We expect this first call to only start the exit animator
         handler.setNext("data-1", postExitCB, postEnterCB)
-        verify(animatorSet, times(1)).start()
+        verify(exitAnimator, times(1)).start()
+        verify(enterAnimator, never()).start()
         verify(postExitCB, never()).invoke()
+        verify(postEnterCB, never()).invoke()
 
+        // After the exit animator completes,
+        // the exit cb should run, and enter animation should start
         handler.onAnimationEnd(exitAnimator)
-        verify(animatorSet, times(1)).start()
+        verify(exitAnimator, times(1)).start()
+        verify(enterAnimator, times(1)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postEnterCB, never()).invoke()
 
+        // After the exit animator completes,
+        // the enter cb should run without other state changes
         handler.onAnimationEnd(enterAnimator)
-        verify(animatorSet, times(1)).start()
+        verify(exitAnimator, times(1)).start()
+        verify(enterAnimator, times(1)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postEnterCB, times(1)).invoke()
     }
@@ -120,38 +123,58 @@ class MetadataAnimationHandlerTest : SysuiTestCase() {
         val postExitCB2 = mock(Callback::class.java)
         val postEnterCB2 = mock(Callback::class.java)
 
+        // We expect this first call to only start the exit animator
         handler.setNext("data-1", postExitCB, postEnterCB)
-        verify(animatorSet, times(1)).start()
+        verify(exitAnimator, times(1)).start()
+        verify(enterAnimator, never()).start()
         verify(postExitCB, never()).invoke()
         verify(postExitCB2, never()).invoke()
         verify(postEnterCB, never()).invoke()
         verify(postEnterCB2, never()).invoke()
 
-        whenever(animatorSet.isRunning()).thenReturn(true)
+        // After the exit animator completes,
+        // the exit cb should run, and enter animation should start
+        whenever(exitAnimator.isRunning()).thenReturn(true)
+        whenever(enterAnimator.isRunning()).thenReturn(false)
         handler.onAnimationEnd(exitAnimator)
-        verify(animatorSet, times(1)).start()
+        verify(exitAnimator, times(1)).start()
+        verify(enterAnimator, times(1)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postExitCB2, never()).invoke()
         verify(postEnterCB, never()).invoke()
         verify(postEnterCB2, never()).invoke()
 
+        // Setting new data before the enter animator completes should not trigger
+        // the exit animator an additional time (since it's already running)
+        whenever(exitAnimator.isRunning()).thenReturn(false)
+        whenever(enterAnimator.isRunning()).thenReturn(true)
         handler.setNext("data-2", postExitCB2, postEnterCB2)
+        verify(exitAnimator, times(1)).start()
+
+        // Finishing the enterAnimator should cause the exitAnimator to fire again
+        // since the data change and additional time. No enterCB should be executed.
         handler.onAnimationEnd(enterAnimator)
-        verify(animatorSet, times(2)).start()
+        verify(exitAnimator, times(2)).start()
+        verify(enterAnimator, times(1)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postExitCB2, never()).invoke()
         verify(postEnterCB, never()).invoke()
         verify(postEnterCB2, never()).invoke()
 
+        // Continuing the sequence, this triggers the enter animator an additional time
         handler.onAnimationEnd(exitAnimator)
-        verify(animatorSet, times(2)).start()
+        verify(exitAnimator, times(2)).start()
+        verify(enterAnimator, times(2)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postExitCB2, times(1)).invoke()
         verify(postEnterCB, never()).invoke()
         verify(postEnterCB2, never()).invoke()
 
+        // And finally the enter animator completes,
+        // triggering the correct postEnterCallback to fire
         handler.onAnimationEnd(enterAnimator)
-        verify(animatorSet, times(2)).start()
+        verify(exitAnimator, times(2)).start()
+        verify(enterAnimator, times(2)).start()
         verify(postExitCB, times(1)).invoke()
         verify(postExitCB2, times(1)).invoke()
         verify(postEnterCB, never()).invoke()
@@ -172,6 +195,7 @@ class MetadataAnimationHandlerTest : SysuiTestCase() {
     fun enterAnimatorEndsWithoutCallback_noAnimatiorStart() {
         handler.onAnimationEnd(enterAnimator)
 
-        verify(animatorSet, never()).start()
+        verify(exitAnimator, never()).start()
+        verify(enterAnimator, never()).start()
     }
 }