[Motion][Split shade] Fix left side of split shade sometimes not overscrolling

The issue was caused by the split shade overscroll classes sometimes having a reference to an old instance of QSFragment. To fix it, we now make sure that it always uses the latest. + Drive by addition of making changed classes dumpable Test: Manually. Wasn't able reproduce the issue anymore Test: `adb shell dumpsys activity service SystemUI` to test the output of the new dumpables Test: ShadeTransitionControllerTest.kt Test: SplitShadeOverScrollerTest.kt Test: SplitShadeLockScreenOverScrollerTest.kt Fixes: 236221279 Change-Id: Ic8af4215efd1333f142f0197b76db88e2e57ead4
2022-06-16 08:41:21 +00:00
parent 527d13ae79
commit 0aa9e8082a
7 changed files with 80 additions and 20 deletions
--- a/packages/SystemUI/src/com/android/systemui/statusbar/LockscreenShadeTransitionController.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/LockscreenShadeTransitionController.kt
@@ -178,7 +178,7 @@ class LockscreenShadeTransitionController @Inject constructor(
    val touchHelper = DragDownHelper(falsingManager, falsingCollector, this, context)

    private val splitShadeOverScroller: SplitShadeLockScreenOverScroller by lazy {
-        splitShadeOverScrollerFactory.create(qS, nsslController)
+        splitShadeOverScrollerFactory.create({ qS }, { nsslController })
    }

    private val phoneShadeOverScroller: SingleShadeLockScreenOverScroller by lazy {
@@ -911,4 +911,4 @@ class DragDownHelper(
        host.getLocationOnScreen(temp2)
        return expandCallback.getChildAtRawPosition(x + temp2[0], y + temp2[1])
    }
-}
+}
--- a/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScroller.kt
@@ -9,6 +9,7 @@ import android.view.animation.PathInterpolator
 import com.android.internal.annotations.VisibleForTesting
 import com.android.systemui.R
 import com.android.systemui.animation.Interpolators
+import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.qs.QS
 import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
 import com.android.systemui.statusbar.phone.ScrimController
@@ -16,16 +17,18 @@ import com.android.systemui.statusbar.policy.ConfigurationController
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
+import java.io.PrintWriter

 class SplitShadeLockScreenOverScroller
@AssistedInject
 constructor(
    configurationController: ConfigurationController,
+    dumpManager: DumpManager,
    private val context: Context,
    private val scrimController: ScrimController,
    private val statusBarStateController: SysuiStatusBarStateController,
-    @Assisted private val qS: QS,
-    @Assisted private val nsslController: NotificationStackScrollLayoutController
+    @Assisted private val qSProvider: () -> QS,
+    @Assisted private val nsslControllerProvider: () -> NotificationStackScrollLayoutController
 ) : LockScreenShadeOverScroller {

    private var releaseOverScrollAnimator: Animator? = null
@@ -34,6 +37,12 @@ constructor(
    private var maxOverScrollAmount = 0
    private var previousOverscrollAmount = 0

+    private val qS: QS
+        get() = qSProvider()
+
+    private val nsslController: NotificationStackScrollLayoutController
+        get() = nsslControllerProvider()
+
    init {
        updateResources()
        configurationController.addCallback(
@@ -42,6 +51,9 @@ constructor(
                    updateResources()
                }
            })
+        dumpManager.registerDumpable("SplitShadeLockscreenOverScroller") { printWriter, _ ->
+            dump(printWriter)
+        }
    }

    private fun updateResources() {
@@ -114,11 +126,25 @@ constructor(
        releaseOverScrollAnimator = null
    }

+    private fun dump(printWriter: PrintWriter) {
+        printWriter.println(
+            """
+            SplitShadeLockScreenOverScroller:
+                Resources:
+                    transitionToFullShadeDistance: $transitionToFullShadeDistance
+                    maxOverScrollAmount: $maxOverScrollAmount
+                    releaseOverScrollDuration: $releaseOverScrollDuration
+                State:
+                    previousOverscrollAmount: $previousOverscrollAmount
+                    expansionDragDownAmount: $expansionDragDownAmount
+            """.trimIndent())
+    }
+
    @AssistedFactory
    fun interface Factory {
        fun create(
-            qS: QS,
-            nsslController: NotificationStackScrollLayoutController
+            qSProvider: () -> QS,
+            nsslControllerProvider: () -> NotificationStackScrollLayoutController
        ): SplitShadeLockScreenOverScroller
    }

--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/shade/transition/ShadeTransitionController.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/shade/transition/ShadeTransitionController.kt
@@ -4,6 +4,7 @@ import android.content.Context
 import android.content.res.Configuration
 import com.android.systemui.R
 import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.qs.QS
 import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
 import com.android.systemui.statusbar.phone.NotificationPanelViewController
@@ -11,6 +12,7 @@ import com.android.systemui.statusbar.phone.panelstate.PanelExpansionChangeEvent
 import com.android.systemui.statusbar.phone.panelstate.PanelExpansionStateManager
 import com.android.systemui.statusbar.phone.panelstate.PanelState
 import com.android.systemui.statusbar.policy.ConfigurationController
+import java.io.PrintWriter
 import javax.inject.Inject

 /** Controls the shade expansion transition on non-lockscreen. */
@@ -20,6 +22,7 @@ class ShadeTransitionController
 constructor(
    configurationController: ConfigurationController,
    panelExpansionStateManager: PanelExpansionStateManager,
+    dumpManager: DumpManager,
    private val context: Context,
    private val splitShadeOverScrollerFactory: SplitShadeOverScroller.Factory,
    private val noOpOverScroller: NoOpOverScroller,
@@ -33,7 +36,7 @@ constructor(
    private var inSplitShade = false

    private val splitShadeOverScroller by lazy {
-        splitShadeOverScrollerFactory.create(qs, notificationStackScrollLayoutController)
+        splitShadeOverScrollerFactory.create({ qs }, { notificationStackScrollLayoutController })
    }
    private val shadeOverScroller: ShadeOverScroller
        get() =
@@ -53,6 +56,9 @@ constructor(
            })
        panelExpansionStateManager.addExpansionListener(this::onPanelExpansionChanged)
        panelExpansionStateManager.addStateListener(this::onPanelStateChanged)
+        dumpManager.registerDumpable("ShadeTransitionController") { printWriter, _ ->
+            dump(printWriter)
+        }
    }

    private fun updateResources() {
@@ -72,4 +78,15 @@ constructor(
        this::qs.isInitialized &&
            this::notificationPanelViewController.isInitialized &&
            this::notificationStackScrollLayoutController.isInitialized
+
+    private fun dump(pw: PrintWriter) {
+        pw.println(
+            """
+            ShadeTransitionController:
+                inSplitShade: $inSplitShade
+                qs.isInitialized: ${this::qs.isInitialized}
+                npvc.isInitialized: ${this::notificationPanelViewController.isInitialized}
+                nssl.isInitialized: ${this::notificationStackScrollLayoutController.isInitialized}
+            """.trimIndent())
+    }
 }
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/shade/transition/SplitShadeOverScroller.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/shade/transition/SplitShadeOverScroller.kt
@@ -28,8 +28,8 @@ constructor(
    dumpManager: DumpManager,
    private val context: Context,
    private val scrimController: ScrimController,
-    @Assisted private val qS: QS,
-    @Assisted private val nsslController: NotificationStackScrollLayoutController
+    @Assisted private val qSProvider: () -> QS,
+    @Assisted private val nsslControllerProvider: () -> NotificationStackScrollLayoutController
 ) : ShadeOverScroller {

    private var releaseOverScrollDuration = 0L
@@ -39,6 +39,12 @@ constructor(
    @PanelState private var panelState: Int = STATE_CLOSED
    private var releaseOverScrollAnimator: Animator? = null

+    private val qS: QS
+        get() = qSProvider()
+
+    private val nsslController: NotificationStackScrollLayoutController
+        get() = nsslControllerProvider()
+
    init {
        updateResources()
        configurationController.addCallback(
@@ -47,7 +53,9 @@ constructor(
                    updateResources()
                }
            })
-        dumpManager.registerDumpable(this::dump)
+        dumpManager.registerDumpable("SplitShadeOverScroller") { printWriter, _ ->
+            dump(printWriter)
+        }
    }

    private fun updateResources() {
@@ -118,7 +126,7 @@ constructor(
        releaseOverScrollAnimator = null
    }

-    private fun dump(pw: PrintWriter, strings: Array<String>) {
+    private fun dump(pw: PrintWriter) {
        pw.println(
            """
            SplitShadeOverScroller:
@@ -129,15 +137,14 @@ constructor(
                    previousOverscrollAmount: $previousOverscrollAmount
                    dragDownAmount: $dragDownAmount
                    panelState: $panelState
-            """.trimIndent()
-        )
+            """.trimIndent())
    }

    @AssistedFactory
    fun interface Factory {
        fun create(
-            qS: QS,
-            nsslController: NotificationStackScrollLayoutController
+            qSProvider: () -> QS,
+            nsslControllerProvider: () -> NotificationStackScrollLayoutController
        ): SplitShadeOverScroller
    }
 }
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScrollerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/SplitShadeLockScreenOverScrollerTest.kt
@@ -1,10 +1,10 @@
 package com.android.systemui.statusbar

-import org.mockito.Mockito.`when` as whenever
 import android.testing.AndroidTestingRunner
 import android.testing.TestableLooper
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.qs.QS
 import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
 import com.android.systemui.statusbar.phone.ScrimController
@@ -19,6 +19,7 @@ import org.mockito.Mockito.reset
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyNoMoreInteractions
 import org.mockito.Mockito.verifyZeroInteractions
+import org.mockito.Mockito.`when` as whenever
 import org.mockito.MockitoAnnotations

@RunWith(AndroidTestingRunner::class)
@@ -32,6 +33,7 @@ class SplitShadeLockScreenOverScrollerTest : SysuiTestCase() {
    @Mock private lateinit var statusBarStateController: SysuiStatusBarStateController
    @Mock private lateinit var qS: QS
    @Mock private lateinit var nsslController: NotificationStackScrollLayoutController
+    @Mock private lateinit var dumpManager: DumpManager

    private lateinit var overScroller: SplitShadeLockScreenOverScroller

@@ -44,11 +46,12 @@ class SplitShadeLockScreenOverScrollerTest : SysuiTestCase() {
        overScroller =
            SplitShadeLockScreenOverScroller(
                configurationController,
+                dumpManager,
                context,
                scrimController,
                statusBarStateController,
-                qS,
-                nsslController)
+                { qS },
+                { nsslController })
    }

    @Test
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/shade/transition/ShadeTransitionControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/shade/transition/ShadeTransitionControllerTest.kt
@@ -4,6 +4,7 @@ import android.testing.AndroidTestingRunner
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
+import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.qs.QS
 import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController
 import com.android.systemui.statusbar.phone.NotificationPanelViewController
@@ -29,6 +30,7 @@ class ShadeTransitionControllerTest : SysuiTestCase() {
    @Mock private lateinit var noOpOverScroller: NoOpOverScroller
    @Mock private lateinit var splitShadeOverScroller: SplitShadeOverScroller
    @Mock private lateinit var scrimShadeTransitionController: ScrimShadeTransitionController
+    @Mock private lateinit var dumpManager: DumpManager

    private lateinit var controller: ShadeTransitionController

@@ -43,6 +45,7 @@ class ShadeTransitionControllerTest : SysuiTestCase() {
            ShadeTransitionController(
                configurationController,
                panelExpansionStateManager,
+                dumpManager,
                context,
                splitShadeOverScrollerFactory = { _, _ -> splitShadeOverScroller },
                noOpOverScroller,
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/shade/transition/SplitShadeOverScrollerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/shade/transition/SplitShadeOverScrollerTest.kt
@@ -43,8 +43,12 @@ class SplitShadeOverScrollerTest : SysuiTestCase() {
        whenever(nsslController.height).thenReturn(1000)
        overScroller =
            SplitShadeOverScroller(
-                configurationController, dumpManager, context, scrimController, qs, nsslController
-            )
+                configurationController,
+                dumpManager,
+                context,
+                scrimController,
+                { qs },
+                { nsslController })
    }

    @Test