Optimizing the QSLongPressEffect

StateFlows in the QSLongPressEffect need to emit and be collected on the
main thread to avoid thread-switching and preventing jank. Therefore,
the need for a background scope has been removed. In terms of
testability, the concern of setting touch listeners has been delegated
to the QSLongPressEffectViewBinder

Test: SystemUITests:QSTileViewImplTest
Test: SystemUiRobotTests:QSLongPressEffectTest
Bug: 332903800
Flag: ACONFIG quick_settings_visual_haptics_longpress TEAMFOOD
Change-Id: I6efe612c3bfda4ac39525b97c3a7797c61f54b8b

6 files changed, 69 insertions, 90 deletions

diff --git a/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt b/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
index 3889703e74c4..e332656e0f15 100644
--- a/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
+++ b/packages/SystemUI/multivalentTests/src/com/android/systemui/haptics/qs/QSLongPressEffectTest.kt
@@ -18,9 +18,6 @@ package com.android.systemui.haptics.qs
 
 import android.os.VibrationEffect
 import android.testing.TestableLooper.RunWithLooper
-import android.view.MotionEvent
-import android.view.View
-import androidx.test.core.view.MotionEventBuilder
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
@@ -29,18 +26,15 @@ import com.android.systemui.coroutines.collectLastValue
 import com.android.systemui.haptics.vibratorHelper
 import com.android.systemui.keyguard.data.repository.fakeKeyguardRepository
 import com.android.systemui.keyguard.domain.interactor.keyguardInteractor
-import com.android.systemui.kosmos.backgroundCoroutineContext
 import com.android.systemui.kosmos.testScope
 import com.android.systemui.testKosmos
 import com.google.common.truth.Truth.assertThat
-import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mockito.Mock
 import org.mockito.junit.MockitoJUnit
 import org.mockito.junit.MockitoRule
 
@@ -50,7 +44,6 @@ import org.mockito.junit.MockitoRule
 class QSLongPressEffectTest : SysuiTestCase() {
 
     @Rule @JvmField val mMockitoRule: MockitoRule = MockitoJUnit.rule()
-    @Mock private lateinit var testView: View
     @get:Rule val animatorTestRule = AnimatorTestRule(this)
     private val kosmos = testKosmos()
     private val vibratorHelper = kosmos.vibratorHelper
@@ -73,7 +66,6 @@ class QSLongPressEffectTest : SysuiTestCase() {
             QSLongPressEffect(
                 vibratorHelper,
                 kosmos.keyguardInteractor,
-                CoroutineScope(kosmos.backgroundCoroutineContext),
             )
         longPressEffect.initializeEffect(effectDuration)
     }
@@ -133,8 +125,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
     @Test
     fun onActionDown_whileIdle_startsWait() = testWithScope {
         // GIVEN an action down event occurs
-        val downEvent = buildMotionEvent(MotionEvent.ACTION_DOWN)
-        longPressEffect.onTouch(testView, downEvent)
+        longPressEffect.handleActionDown()
 
         // THEN the effect moves to the TIMEOUT_WAIT state
         val state by collectLastValue(longPressEffect.state)
@@ -144,8 +135,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
     @Test
     fun onActionCancel_whileWaiting_goesIdle() = testWhileWaiting {
         // GIVEN an action cancel occurs
-        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
-        longPressEffect.onTouch(testView, cancelEvent)
+        longPressEffect.handleActionCancel()
 
         // THEN the effect goes back to idle and does not start
         val state by collectLastValue(longPressEffect.state)
@@ -159,8 +149,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
         val action by collectLastValue(longPressEffect.actionType)
 
         // GIVEN an action up occurs
-        val upEvent = buildMotionEvent(MotionEvent.ACTION_UP)
-        longPressEffect.onTouch(testView, upEvent)
+        longPressEffect.handleActionUp()
 
         // THEN the action to invoke is the click action and the effect does not start
         assertThat(action).isEqualTo(QSLongPressEffect.ActionType.CLICK)
@@ -182,8 +171,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
         animatorTestRule.advanceTimeBy(effectDuration / 2L)
 
         // WHEN an action up occurs
-        val upEvent = buildMotionEvent(MotionEvent.ACTION_UP)
-        longPressEffect.onTouch(testView, upEvent)
+        longPressEffect.handleActionUp()
 
         // THEN the effect gets reversed at 50% progress
         assertEffectReverses(0.5f)
@@ -195,8 +183,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
         animatorTestRule.advanceTimeBy(effectDuration / 2L)
 
         // WHEN an action cancel occurs
-        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
-        longPressEffect.onTouch(testView, cancelEvent)
+        longPressEffect.handleActionCancel()
 
         // THEN the effect gets reversed at 50% progress
         assertEffectReverses(0.5f)
@@ -230,12 +217,10 @@ class QSLongPressEffectTest : SysuiTestCase() {
         animatorTestRule.advanceTimeBy(effectDuration / 2L)
 
         // GIVEN an action cancel occurs and the effect gets reversed
-        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
-        longPressEffect.onTouch(testView, cancelEvent)
+        longPressEffect.handleActionCancel()
 
         // GIVEN an action down occurs
-        val downEvent = buildMotionEvent(MotionEvent.ACTION_DOWN)
-        longPressEffect.onTouch(testView, downEvent)
+        longPressEffect.handleActionDown()
 
         // THEN the effect resets
         assertEffectResets()
@@ -247,8 +232,7 @@ class QSLongPressEffectTest : SysuiTestCase() {
         animatorTestRule.advanceTimeBy(effectDuration / 2L)
 
         // GIVEN an action cancel occurs and the effect gets reversed
-        val cancelEvent = buildMotionEvent(MotionEvent.ACTION_CANCEL)
-        longPressEffect.onTouch(testView, cancelEvent)
+        longPressEffect.handleActionCancel()
 
         // GIVEN that the animation completes after a sufficient amount of time
         animatorTestRule.advanceTimeBy(effectDuration.toLong())
@@ -258,9 +242,6 @@ class QSLongPressEffectTest : SysuiTestCase() {
         assertThat(state).isEqualTo(QSLongPressEffect.State.IDLE)
     }
 
-    private fun buildMotionEvent(action: Int): MotionEvent =
-        MotionEventBuilder.newBuilder().setAction(action).build()
-
     private fun testWithScope(test: suspend TestScope.() -> Unit) =
         with(kosmos) { testScope.runTest { test() } }
 
diff --git a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
index 4327d18da97e..bccc3c5dc284 100644
--- a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
+++ b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffect.kt
@@ -17,9 +17,7 @@
 package com.android.systemui.haptics.qs
 
 import android.animation.ValueAnimator
-import android.annotation.SuppressLint
 import android.os.VibrationEffect
-import android.view.MotionEvent
 import android.view.View
 import android.view.ViewConfiguration
 import android.view.animation.AccelerateDecelerateInterpolator
@@ -27,18 +25,14 @@ import androidx.annotation.VisibleForTesting
 import androidx.core.animation.doOnCancel
 import androidx.core.animation.doOnEnd
 import androidx.core.animation.doOnStart
-import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.keyguard.domain.interactor.KeyguardInteractor
 import com.android.systemui.statusbar.VibratorHelper
 import javax.inject.Inject
-import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.SharingStarted
-import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.stateIn
 
 /**
  * A class that handles the long press visuo-haptic effect for a QS tile.
@@ -55,34 +49,32 @@ class QSLongPressEffect
 @Inject
 constructor(
     private val vibratorHelper: VibratorHelper?,
-    val keyguardInteractor: KeyguardInteractor,
-    @Background bgScope: CoroutineScope,
-) : View.OnTouchListener {
+    keyguardInteractor: KeyguardInteractor,
+) {
 
     private var effectDuration = 0
 
     /** Current state */
     private var _state = MutableStateFlow(State.IDLE)
-    val state = _state.stateIn(bgScope, SharingStarted.Lazily, State.IDLE)
+    val state = _state.asStateFlow()
 
     /** Flows for view control and action */
     private val _effectProgress = MutableStateFlow<Float?>(null)
-    val effectProgress = _effectProgress.stateIn(bgScope, SharingStarted.Lazily, null)
+    val effectProgress = _effectProgress.asStateFlow()
 
     // Actions to perform
     private val _postedActionType = MutableStateFlow<ActionType?>(null)
-    val actionType: StateFlow<ActionType?> =
+    val actionType: Flow<ActionType?> =
         combine(
-                _postedActionType,
-                keyguardInteractor.isKeyguardDismissible,
-            ) { action, isDismissible ->
-                if (!isDismissible && action == ActionType.LONG_PRESS) {
-                    ActionType.RESET_AND_LONG_PRESS
-                } else {
-                    action
-                }
+            _postedActionType,
+            keyguardInteractor.isKeyguardDismissible,
+        ) { action, isDismissible ->
+            if (!isDismissible && action == ActionType.LONG_PRESS) {
+                ActionType.RESET_AND_LONG_PRESS
+            } else {
+                action
             }
-            .stateIn(bgScope, SharingStarted.Lazily, null)
+        }
 
     // Should a tap timeout countdown begin
     val shouldWaitForTapTimeout: Flow<Boolean> = state.map { it == State.TIMEOUT_WAIT }
@@ -129,23 +121,7 @@ constructor(
         }
     }
 
-    /**
-     * Handle relevant touch events for the operation of a Tile.
-     *
-     * A click action is performed following the relevant logic that originates from the
-     * [MotionEvent.ACTION_UP] event depending on the current state.
-     */
-    @SuppressLint("ClickableViewAccessibility")
-    override fun onTouch(view: View?, event: MotionEvent?): Boolean {
-        when (event?.actionMasked) {
-            MotionEvent.ACTION_DOWN -> handleActionDown()
-            MotionEvent.ACTION_UP -> handleActionUp()
-            MotionEvent.ACTION_CANCEL -> handleActionCancel()
-        }
-        return true
-    }
-
-    private fun handleActionDown() {
+    fun handleActionDown() {
         when (_state.value) {
             State.IDLE -> {
                 setState(State.TIMEOUT_WAIT)
@@ -155,7 +131,7 @@ constructor(
         }
     }
 
-    private fun handleActionUp() {
+    fun handleActionUp() {
         when (_state.value) {
             State.TIMEOUT_WAIT -> {
                 _postedActionType.value = ActionType.CLICK
@@ -169,7 +145,7 @@ constructor(
         }
     }
 
-    private fun handleActionCancel() {
+    fun handleActionCancel() {
         when (_state.value) {
             State.TIMEOUT_WAIT -> {
                 setState(State.IDLE)
diff --git a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
index ddb9f35c74d9..2ef901d34dd1 100644
--- a/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
+++ b/packages/SystemUI/src/com/android/systemui/haptics/qs/QSLongPressEffectViewBinder.kt
@@ -16,6 +16,8 @@
 
 package com.android.systemui.haptics.qs
 
+import android.annotation.SuppressLint
+import android.view.MotionEvent
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.repeatOnLifecycle
 import com.android.app.tracing.coroutines.launch
@@ -25,10 +27,9 @@ import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.DisposableHandle
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.filter
-import kotlinx.coroutines.launch
 
-// TODO(b/332903800)
 object QSLongPressEffectViewBinder {
+
     fun bind(
         tile: QSTileViewImpl,
         qsLongPressEffect: QSLongPressEffect?,
@@ -36,11 +37,13 @@ object QSLongPressEffectViewBinder {
     ): DisposableHandle? {
         if (qsLongPressEffect == null) return null
 
+        // Set the touch listener as the long-press effect
+        setTouchListener(tile, qsLongPressEffect)
+
         return tile.repeatWhenAttached {
             repeatOnLifecycle(Lifecycle.State.CREATED) {
-                val tag = "${tileSpec ?: "unknownTileSpec"}#LongPressEffect"
                 // Progress of the effect
-                launch("$tag#progress") {
+                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#progress" }) {
                     qsLongPressEffect.effectProgress.collect { progress ->
                         progress?.let {
                             if (it == 0f) {
@@ -53,7 +56,7 @@ object QSLongPressEffectViewBinder {
                 }
 
                 // Action to perform
-                launch("$tag#action") {
+                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#action" }) {
                     qsLongPressEffect.actionType.collect { action ->
                         action?.let {
                             when (it) {
@@ -70,7 +73,7 @@ object QSLongPressEffectViewBinder {
                 }
 
                 // Tap timeout wait
-                launch("$tag#timeout") {
+                launch({ "${tileSpec ?: "unknownTileSpec"}#LongPressEffect#timeout" }) {
                     qsLongPressEffect.shouldWaitForTapTimeout
                         .filter { it }
                         .collect {
@@ -85,4 +88,16 @@ object QSLongPressEffectViewBinder {
             }
         }
     }
+
+    @SuppressLint("ClickableViewAccessibility")
+    private fun setTouchListener(tile: QSTileViewImpl, longPressEffect: QSLongPressEffect?) {
+        tile.setOnTouchListener { _, event ->
+            when (event.actionMasked) {
+                MotionEvent.ACTION_DOWN -> longPressEffect?.handleActionDown()
+                MotionEvent.ACTION_UP -> longPressEffect?.handleActionUp()
+                MotionEvent.ACTION_CANCEL -> longPressEffect?.handleActionCancel()
+            }
+            true
+        }
+    }
 }
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 ca71870845e0..40cf4a4fd403 100644
--- a/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/qs/tileimpl/QSTileViewImpl.kt
@@ -185,8 +185,9 @@ open class QSTileViewImpl @JvmOverloads constructor(
     private val colorEvaluator = ArgbEvaluator.getInstance()
     val isLongPressEffectInitialized: Boolean
         get() = longPressEffect?.hasInitialized == true
-    @VisibleForTesting
-    var longPressEffectHandle: DisposableHandle? = null
+    private var longPressEffectHandle: DisposableHandle? = null
+    val isLongPressEffectBound: Boolean
+        get() = longPressEffectHandle != null
 
     init {
         val typedValue = TypedValue()
@@ -621,11 +622,14 @@ open class QSTileViewImpl @JvmOverloads constructor(
         // Long-press effects
         if (state.handlesLongClick &&
             longPressEffect?.initializeEffect(longPressEffectDuration) == true) {
-            // set the valid long-press effect as the touch listener
-            if (longPressEffectHandle == null) {
+            // bind the long-press effect and set it as the touch listener
+            if (!isLongPressEffectBound) {
                 longPressEffectHandle =
-                    QSLongPressEffectViewBinder.bind(this, longPressEffect, state.spec)
-                setOnTouchListener(longPressEffect)
+                    QSLongPressEffectViewBinder.bind(
+                        this,
+                        longPressEffect,
+                        state.spec,
+                    )
             }
             showRippleEffect = false
             initializeLongPressProperties()
@@ -634,8 +638,7 @@ open class QSTileViewImpl @JvmOverloads constructor(
             // handle a long-press. In this case, we go back to the behaviour of a regular tile
             // and clean-up the resources
             setOnTouchListener(null)
-            longPressEffectHandle?.dispose()
-            longPressEffectHandle = null
+            unbindLongPressEffect()
             showRippleEffect = isClickable
             initialLongPressProperties = null
             finalLongPressProperties = null
@@ -827,6 +830,11 @@ open class QSTileViewImpl @JvmOverloads constructor(
         changeCornerRadius(newRadius)
     }
 
+    private fun unbindLongPressEffect() {
+        longPressEffectHandle?.dispose()
+        longPressEffectHandle = null
+    }
+
     private fun interpolateFloat(fraction: Float, start: Float, end: Float): Float =
         start + fraction * (end - start)
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
index 512ca5315530..ecbd0f54df5b 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/qs/tileimpl/QSTileViewImplTest.kt
@@ -385,7 +385,7 @@ class QSTileViewImplTest : SysuiTestCase() {
     }
 
     @Test
-    fun onStateChange_longPressEffectActive_withInvalidDuration_doesNotCreateEffect() {
+    fun onStateChange_longPressEffectActive_withInvalidDuration_doesNotInitializeEffect() {
         val state = QSTile.State() // A state that handles longPress
 
         // GIVEN an invalid long-press effect duration
@@ -399,7 +399,7 @@ class QSTileViewImplTest : SysuiTestCase() {
     }
 
     @Test
-    fun onStateChange_longPressEffectActive_withValidDuration_createsEffect() {
+    fun onStateChange_longPressEffectActive_withValidDuration_initializesEffect() {
         // GIVEN a test state that handles long-press and a valid long-press effect duration
         val state = QSTile.State()
 
@@ -420,7 +420,7 @@ class QSTileViewImplTest : SysuiTestCase() {
         tileView.changeState(state)
 
         // THEN the view binder no longer binds the view to the long-press effect
-        assertThat(tileView.longPressEffectHandle).isNull()
+        assertThat(tileView.isLongPressEffectBound).isFalse()
     }
 
     @Test
@@ -435,7 +435,7 @@ class QSTileViewImplTest : SysuiTestCase() {
         tileView.changeState(state)
 
         // THEN the view is bounded to the long-press effect
-        assertThat(tileView.longPressEffectHandle).isNotNull()
+        assertThat(tileView.isLongPressEffectBound).isTrue()
     }
 
     @Test
@@ -451,7 +451,7 @@ class QSTileViewImplTest : SysuiTestCase() {
         tileView.changeState(state)
 
         // THEN the view binder does not bind the view and no effect is initialized
-        assertThat(tileView.longPressEffectHandle).isNull()
+        assertThat(tileView.isLongPressEffectBound).isFalse()
         assertThat(tileView.isLongPressEffectInitialized).isFalse()
     }
 
@@ -470,7 +470,7 @@ class QSTileViewImplTest : SysuiTestCase() {
         tileView.changeState(state)
 
         // THEN the view binder does not bind the view and no effect is initialized
-        assertThat(tileView.longPressEffectHandle).isNull()
+        assertThat(tileView.isLongPressEffectBound).isFalse()
         assertThat(tileView.isLongPressEffectInitialized).isFalse()
     }
 
diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/haptics/qs/QSLongPressEffectKosmos.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/haptics/qs/QSLongPressEffectKosmos.kt
index 636d509663a2..24603ef200d9 100644
--- a/packages/SystemUI/tests/utils/src/com/android/systemui/haptics/qs/QSLongPressEffectKosmos.kt
+++ b/packages/SystemUI/tests/utils/src/com/android/systemui/haptics/qs/QSLongPressEffectKosmos.kt
@@ -19,7 +19,6 @@ package com.android.systemui.haptics.qs
 import com.android.systemui.haptics.vibratorHelper
 import com.android.systemui.keyguard.domain.interactor.keyguardInteractor
 import com.android.systemui.kosmos.Kosmos
-import com.android.systemui.kosmos.testScope
 
 val Kosmos.qsLongPressEffect by
-    Kosmos.Fixture { QSLongPressEffect(vibratorHelper, keyguardInteractor, testScope) }
+    Kosmos.Fixture { QSLongPressEffect(vibratorHelper, keyguardInteractor) }