Fix performance regressions

* Have the statusIcons always added to StatusBarIconController
  (attach/dettach) instead of when visibility changes. That prevents
  readding views and remeasuring.
* Introduce a specific variable for edit mode to control visibility.
  This makes it so that we don't have to rely on alpha (and it doesn't
  change with QS expansion).

Fixes: 266444213
Fixes: 265104777
Test: perfetto trace
Test: atest sysui-jank-custom
Test: atest LargeScreenShadeHeaderControllerTest
Change-Id: Ib8c3d1bcb1fb59f55c1c97a5b3966a2892b3f4b1

2 files changed, 60 insertions, 81 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt
index 197232ecb547..b3e8d6e441af 100644
--- a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt
@@ -17,6 +17,7 @@
 package com.android.systemui.shade
 
 import android.animation.Animator
+import android.animation.AnimatorListenerAdapter
 import android.annotation.IdRes
 import android.app.StatusBarManager
 import android.content.res.Configuration
@@ -144,6 +145,14 @@ class LargeScreenShadeHeaderController @Inject constructor(
             updateListeners()
         }
 
+    private var customizing = false
+        set(value) {
+            if (field != value) {
+                field = value
+                updateVisibility()
+            }
+        }
+
     /**
      * Whether the QQS/QS part of the shade is visible. This is particularly important in
      * Lockscreen, as the shade is visible but QS is not.
@@ -175,14 +184,9 @@ class LargeScreenShadeHeaderController @Inject constructor(
      */
     var shadeExpandedFraction = -1f
         set(value) {
-            if (field != value) {
-                val oldAlpha = header.alpha
+            if (qsVisible && field != value) {
                 header.alpha = ShadeInterpolation.getContentAlpha(value)
                 field = value
-                if ((oldAlpha == 0f && header.alpha > 0f) ||
-                        (oldAlpha > 0f && header.alpha == 0f)) {
-                    updateVisibility()
-                }
             }
         }
 
@@ -317,6 +321,7 @@ class LargeScreenShadeHeaderController @Inject constructor(
         dumpManager.registerDumpable(this)
         configurationController.addCallback(configurationControllerListener)
         demoModeController.addCallback(demoModeReceiver)
+        statusBarIconController.addIconGroup(iconManager)
     }
 
     override fun onViewDetached() {
@@ -324,6 +329,7 @@ class LargeScreenShadeHeaderController @Inject constructor(
         dumpManager.unregisterDumpable(this::class.java.simpleName)
         configurationController.removeCallback(configurationControllerListener)
         demoModeController.removeCallback(demoModeReceiver)
+        statusBarIconController.removeIconGroup(iconManager)
     }
 
     fun disable(state1: Int, state2: Int, animate: Boolean) {
@@ -338,31 +344,10 @@ class LargeScreenShadeHeaderController @Inject constructor(
                 .setDuration(duration)
                 .alpha(if (show) 0f else 1f)
                 .setInterpolator(if (show) Interpolators.ALPHA_OUT else Interpolators.ALPHA_IN)
-                .setUpdateListener {
-                    updateVisibility()
-                }
-                .setListener(endAnimationListener)
+                .setListener(CustomizerAnimationListener(show))
                 .start()
     }
 
-    private val endAnimationListener = object : Animator.AnimatorListener {
-        override fun onAnimationCancel(animation: Animator?) {
-            clearListeners()
-        }
-
-        override fun onAnimationEnd(animation: Animator?) {
-            clearListeners()
-        }
-
-        override fun onAnimationRepeat(animation: Animator?) {}
-
-        override fun onAnimationStart(animation: Animator?) {}
-
-        private fun clearListeners() {
-            header.animate().setListener(null).setUpdateListener(null)
-        }
-    }
-
     private fun loadConstraints() {
         if (header is MotionLayout) {
             // Use resources.getXml instead of passing the resource id due to bug b/205018300
@@ -443,7 +428,7 @@ class LargeScreenShadeHeaderController @Inject constructor(
     private fun updateVisibility() {
         val visibility = if (!largeScreenActive && !combinedHeaders || qsDisabled) {
             View.GONE
-        } else if (qsVisible && header.alpha > 0f) {
+        } else if (qsVisible && !customizing) {
             View.VISIBLE
         } else {
             View.INVISIBLE
@@ -491,10 +476,8 @@ class LargeScreenShadeHeaderController @Inject constructor(
         if (visible) {
             updateSingleCarrier(qsCarrierGroupController.isSingleCarrier)
             qsCarrierGroupController.setOnSingleCarrierChangedListener { updateSingleCarrier(it) }
-            statusBarIconController.addIconGroup(iconManager)
         } else {
             qsCarrierGroupController.setOnSingleCarrierChangedListener(null)
-            statusBarIconController.removeIconGroup(iconManager)
         }
     }
 
@@ -566,4 +549,23 @@ class LargeScreenShadeHeaderController @Inject constructor(
 
     @VisibleForTesting
     internal fun simulateViewDetached() = this.onViewDetached()
+
+    inner class CustomizerAnimationListener(
+            private val enteringCustomizing: Boolean,
+    ) : AnimatorListenerAdapter() {
+        override fun onAnimationEnd(animation: Animator?) {
+            super.onAnimationEnd(animation)
+            header.animate().setListener(null)
+            if (enteringCustomizing) {
+                customizing = true
+            }
+        }
+
+        override fun onAnimationStart(animation: Animator?) {
+            super.onAnimationStart(animation)
+            if (!enteringCustomizing) {
+                customizing = false
+            }
+        }
+    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt
index 2bf2a81fe13e..36812807c54e 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt
@@ -1,7 +1,6 @@
 package com.android.systemui.shade
 
 import android.animation.Animator
-import android.animation.ValueAnimator
 import android.app.StatusBarManager
 import android.content.Context
 import android.testing.AndroidTestingRunner
@@ -44,6 +43,7 @@ import org.mockito.ArgumentMatchers.anyFloat
 import org.mockito.ArgumentMatchers.anyInt
 import org.mockito.Mock
 import org.mockito.Mockito.mock
+import org.mockito.Mockito.reset
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyZeroInteractions
 import org.mockito.junit.MockitoJUnit
@@ -156,24 +156,24 @@ class LargeScreenShadeHeaderControllerTest : SysuiTestCase() {
     fun updateListeners_registersWhenVisible() {
         makeShadeVisible()
         verify(qsCarrierGroupController).setListening(true)
+    }
+
+    @Test
+    fun statusIconsAddedWhenAttached() {
         verify(statusBarIconController).addIconGroup(any())
     }
 
     @Test
-    fun shadeExpandedFraction_updatesAlpha() {
-        makeShadeVisible()
-        mLargeScreenShadeHeaderController.shadeExpandedFraction = 0.5f
-        verify(view).setAlpha(ShadeInterpolation.getContentAlpha(0.5f))
+    fun statusIconsRemovedWhenDettached() {
+        mLargeScreenShadeHeaderController.simulateViewDetached()
+        verify(statusBarIconController).removeIconGroup(any())
     }
 
     @Test
-    fun alphaChangesUpdateVisibility() {
+    fun shadeExpandedFraction_updatesAlpha() {
         makeShadeVisible()
-        mLargeScreenShadeHeaderController.shadeExpandedFraction = 0f
-        assertThat(viewVisibility).isEqualTo(View.INVISIBLE)
-
-        mLargeScreenShadeHeaderController.shadeExpandedFraction = 1f
-        assertThat(viewVisibility).isEqualTo(View.VISIBLE)
+        mLargeScreenShadeHeaderController.shadeExpandedFraction = 0.5f
+        verify(view).setAlpha(ShadeInterpolation.getContentAlpha(0.5f))
     }
 
     @Test
@@ -261,54 +261,32 @@ class LargeScreenShadeHeaderControllerTest : SysuiTestCase() {
     }
 
     @Test
-    fun testShadeExpanded_true_alpha_zero_invisible() {
-        view.alpha = 0f
-        mLargeScreenShadeHeaderController.largeScreenActive = true
-        mLargeScreenShadeHeaderController.qsVisible = true
-
-        assertThat(viewVisibility).isEqualTo(View.INVISIBLE)
-    }
+    fun customizerAnimatorChangesViewVisibility() {
+        makeShadeVisible()
 
-    @Test
-    fun animatorCallsUpdateVisibilityOnUpdate() {
         val animator = mock(ViewPropertyAnimator::class.java, Answers.RETURNS_SELF)
+        val duration = 1000L
         whenever(view.animate()).thenReturn(animator)
+        val listenerCaptor = argumentCaptor<Animator.AnimatorListener>()
 
-        mLargeScreenShadeHeaderController.startCustomizingAnimation(show = false, 0L)
-
-        val updateCaptor = argumentCaptor<ValueAnimator.AnimatorUpdateListener>()
-        verify(animator).setUpdateListener(capture(updateCaptor))
-
-        mLargeScreenShadeHeaderController.largeScreenActive = true
-        mLargeScreenShadeHeaderController.qsVisible = true
-
-        view.alpha = 1f
-        updateCaptor.value.onAnimationUpdate(mock())
-
-        assertThat(viewVisibility).isEqualTo(View.VISIBLE)
-
-        view.alpha = 0f
-        updateCaptor.value.onAnimationUpdate(mock())
-
+        mLargeScreenShadeHeaderController.startCustomizingAnimation(show = true, duration)
+        verify(animator).setListener(capture(listenerCaptor))
+        // Start and end the animation
+        listenerCaptor.value.onAnimationStart(mock())
+        listenerCaptor.value.onAnimationEnd(mock())
         assertThat(viewVisibility).isEqualTo(View.INVISIBLE)
-    }
 
-    @Test
-    fun animatorListenersClearedAtEnd() {
-        val animator = mock(ViewPropertyAnimator::class.java, Answers.RETURNS_SELF)
-        whenever(view.animate()).thenReturn(animator)
-
-        mLargeScreenShadeHeaderController.startCustomizingAnimation(show = true, 0L)
-        val listenerCaptor = argumentCaptor<Animator.AnimatorListener>()
+        reset(animator)
+        mLargeScreenShadeHeaderController.startCustomizingAnimation(show = false, duration)
         verify(animator).setListener(capture(listenerCaptor))
-
+        // Start and end the animation
+        listenerCaptor.value.onAnimationStart(mock())
         listenerCaptor.value.onAnimationEnd(mock())
-        verify(animator).setListener(null)
-        verify(animator).setUpdateListener(null)
+        assertThat(viewVisibility).isEqualTo(View.VISIBLE)
     }
 
     @Test
-    fun animatorListenersClearedOnCancel() {
+    fun animatorListenerClearedAtEnd() {
         val animator = mock(ViewPropertyAnimator::class.java, Answers.RETURNS_SELF)
         whenever(view.animate()).thenReturn(animator)
 
@@ -316,9 +294,8 @@ class LargeScreenShadeHeaderControllerTest : SysuiTestCase() {
         val listenerCaptor = argumentCaptor<Animator.AnimatorListener>()
         verify(animator).setListener(capture(listenerCaptor))
 
-        listenerCaptor.value.onAnimationCancel(mock())
+        listenerCaptor.value.onAnimationEnd(mock())
         verify(animator).setListener(null)
-        verify(animator).setUpdateListener(null)
     }
 
     @Test