From 07fd8cc1020350e811ed7a6773803919494e804b Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Tue, 18 Apr 2023 19:34:46 -0500 Subject: [PATCH] Stop seekbar listening when carousel is not visible The seekbar would continue to update if the media was swiped away while still active, even though the QQS and lockscreen host locations would no longer display the carousel. This updates the visibility status to account for whether there is actually any "active" media when the carousel is in those locations. Bug: 278145877 Test: atest MediaHierarchyManagerTest MediaCarouselControllerTest Test: manual - swipe away while playing, seekbar only updates in QS Change-Id: I0e1bd89ec575f239d8fdb09b5d1289b9d184c1db --- .../controls/ui/MediaCarouselController.kt | 2 ++ .../controls/ui/MediaHierarchyManager.kt | 29 +++++++++---------- .../ui/MediaCarouselControllerTest.kt | 14 +++++++++ .../controls/ui/MediaHierarchyManagerTest.kt | 15 ++++++++++ 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt index ab394428ad8d7..0aa434976ce76 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt @@ -166,6 +166,7 @@ constructor( } } + /** Whether the media card currently has the "expanded" layout */ @VisibleForTesting var currentlyExpanded = true set(value) { @@ -501,6 +502,7 @@ constructor( mediaHostStatesManager.addCallback( object : MediaHostStatesManager.Callback { override fun onHostStateChanged(location: Int, mediaHostState: MediaHostState) { + updateUserVisibility() if (location == desiredLocation) { onDesiredLocationChanged(desiredLocation, mediaHostState, animate = false) } diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaHierarchyManager.kt b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaHierarchyManager.kt index 54237ce7cf251..49e1665a0de92 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaHierarchyManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaHierarchyManager.kt @@ -257,7 +257,7 @@ constructor( if (value && (isLockScreenShadeVisibleToUser() || isHomeScreenShadeVisibleToUser())) { mediaCarouselController.logSmartspaceImpression(value) } - mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = isVisibleToUser() + updateUserVisibility() } /** @@ -460,8 +460,7 @@ constructor( ) { mediaCarouselController.logSmartspaceImpression(qsExpanded) } - mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = - isVisibleToUser() + updateUserVisibility() } override fun onDozeAmountChanged(linear: Float, eased: Float) { @@ -480,8 +479,7 @@ constructor( qsExpanded = false closeGuts() } - mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = - isVisibleToUser() + updateUserVisibility() } override fun onExpandedChanged(isExpanded: Boolean) { @@ -489,8 +487,7 @@ constructor( if (isHomeScreenShadeVisibleToUser()) { mediaCarouselController.logSmartspaceImpression(qsExpanded) } - mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = - isVisibleToUser() + updateUserVisibility() } } ) @@ -532,9 +529,7 @@ constructor( } ) - mediaCarouselController.updateUserVisibility = { - mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = isVisibleToUser() - } + mediaCarouselController.updateUserVisibility = this::updateUserVisibility mediaCarouselController.updateHostVisibility = { mediaHosts.forEach { it?.updateViewVisibility() } } @@ -1180,11 +1175,15 @@ constructor( return isCrossFadeAnimatorRunning } - /** Returns true when the media card could be visible to the user if existed. */ - private fun isVisibleToUser(): Boolean { - return isLockScreenVisibleToUser() || - isLockScreenShadeVisibleToUser() || - isHomeScreenShadeVisibleToUser() + /** Update whether or not the media carousel could be visible to the user */ + private fun updateUserVisibility() { + val shadeVisible = + isLockScreenVisibleToUser() || + isLockScreenShadeVisibleToUser() || + isHomeScreenShadeVisibleToUser() + val mediaVisible = qsExpanded || hasActiveMediaOrRecommendation + mediaCarouselController.mediaCarouselScrollHandler.visibleToUser = + shadeVisible && mediaVisible } private fun isLockScreenVisibleToUser(): Boolean { diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt index a72634bcb807e..1a00ac2722fe9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt @@ -110,6 +110,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { lateinit var configListener: ArgumentCaptor @Captor lateinit var visualStabilityCallback: ArgumentCaptor @Captor lateinit var keyguardCallback: ArgumentCaptor + @Captor lateinit var hostStateCallback: ArgumentCaptor private val clock = FakeSystemClock() private lateinit var mediaCarouselController: MediaCarouselController @@ -143,6 +144,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { verify(visualStabilityProvider) .addPersistentReorderingAllowedListener(capture(visualStabilityCallback)) verify(keyguardUpdateMonitor).registerCallback(capture(keyguardCallback)) + verify(mediaHostStatesManager).addCallback(capture(hostStateCallback)) whenever(mediaControlPanelFactory.get()).thenReturn(panel) whenever(panel.mediaViewController).thenReturn(mediaViewController) whenever(mediaDataManager.smartspaceMediaData).thenReturn(smartspaceMediaData) @@ -832,4 +834,16 @@ class MediaCarouselControllerTest : SysuiTestCase() { // Verify that seekbar listening attribute in media control panel is set to false. verify(panel, times(MediaPlayerData.players().size)).listening = false } + + @Test + fun testOnHostStateChanged_updateVisibility() { + var stateUpdated = false + mediaCarouselController.updateUserVisibility = { stateUpdated = true } + + // When the host state updates + hostStateCallback.value!!.onHostStateChanged(LOCATION_QS, mediaHostState) + + // Then the carousel visibility is updated + assertTrue(stateUpdated) + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaHierarchyManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaHierarchyManagerTest.kt index eb78ded008b27..2ce236d4ba897 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaHierarchyManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaHierarchyManagerTest.kt @@ -470,6 +470,21 @@ class MediaHierarchyManagerTest : SysuiTestCase() { ) } + @Test + fun testQsExpandedChanged_noQqsMedia() { + // When we are looking at QQS with active media + whenever(statusBarStateController.state).thenReturn(StatusBarState.SHADE) + whenever(statusBarStateController.isExpanded).thenReturn(true) + + // When there is no longer any active media + whenever(mediaDataManager.hasActiveMediaOrRecommendation()).thenReturn(false) + mediaHierarchyManager.qsExpanded = false + + // Then the carousel is set to not visible + verify(mediaCarouselScrollHandler).visibleToUser = false + assertThat(mediaCarouselScrollHandler.visibleToUser).isFalse() + } + private fun enableSplitShade() { context .getOrCreateTestableResources()