diff options
2 files changed, 57 insertions, 137 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 9c7b48d2514d..0176c4273a68 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 @@ -39,7 +39,6 @@ import com.android.systemui.Dumpable import com.android.systemui.R import com.android.systemui.classifier.FalsingCollector import com.android.systemui.dagger.SysUISingleton -import com.android.systemui.dagger.qualifiers.Background import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.dump.DumpManager import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor @@ -69,7 +68,6 @@ import com.android.systemui.util.time.SystemClock import com.android.systemui.util.traceSection import java.io.PrintWriter import java.util.TreeMap -import java.util.concurrent.Executor import javax.inject.Inject import javax.inject.Provider import kotlinx.coroutines.CoroutineScope @@ -95,8 +93,7 @@ constructor( private val mediaHostStatesManager: MediaHostStatesManager, private val activityStarter: ActivityStarter, private val systemClock: SystemClock, - @Main private val mainExecutor: DelayableExecutor, - @Background private val backgroundExecutor: Executor, + @Main executor: DelayableExecutor, private val mediaManager: MediaDataManager, configurationController: ConfigurationController, falsingCollector: FalsingCollector, @@ -253,7 +250,7 @@ constructor( MediaCarouselScrollHandler( mediaCarousel, pageIndicator, - mainExecutor, + executor, this::onSwipeToDismiss, this::updatePageIndicatorLocation, this::updateSeekbarListening, @@ -615,50 +612,10 @@ constructor( MediaPlayerData.visiblePlayerKeys() .elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex) if (existingPlayer == null) { - setupNewPlayer(key, data, isSsReactivated, curVisibleMediaKey) - } else { - existingPlayer.bindPlayer(data, key) - MediaPlayerData.addMediaPlayer( - key, - data, - existingPlayer, - systemClock, - isSsReactivated, - debugLogger - ) - val packageName = MediaPlayerData.smartspaceMediaData?.packageName ?: String() - // In case of recommendations hits. - // Check the playing status of media player and the package name. - // To make sure we scroll to the right app's media player. - if ( - isReorderingAllowed || - shouldScrollToKey && - data.isPlaying == true && - packageName == data.packageName - ) { - reorderAllPlayers(curVisibleMediaKey, key) - } else { - needsReordering = true - } - updatePageIndicator() - mediaCarouselScrollHandler.onPlayersChanged() - mediaFrame.requiresRemeasuring = true - } - return existingPlayer == null - } - - private fun setupNewPlayer( - key: String, - data: MediaData, - isSsReactivated: Boolean, - curVisibleMediaKey: MediaPlayerData.MediaSortKey?, - ) { - backgroundExecutor.execute { - val mediaViewHolder = createMediaViewHolderInBg() - // Add the new player in the main thread. - mainExecutor.execute { val newPlayer = mediaControlPanelFactory.get() - newPlayer.attachPlayer(mediaViewHolder) + newPlayer.attachPlayer( + MediaViewHolder.create(LayoutInflater.from(context), mediaContent) + ) newPlayer.mediaViewController.sizeChangedListener = this::updateCarouselDimensions val lp = LinearLayout.LayoutParams( @@ -688,16 +645,36 @@ constructor( } else { needsReordering = true } - updatePageIndicator() - mediaCarouselScrollHandler.onPlayersChanged() - mediaFrame.requiresRemeasuring = true + } else { + existingPlayer.bindPlayer(data, key) + MediaPlayerData.addMediaPlayer( + key, + data, + existingPlayer, + systemClock, + isSsReactivated, + debugLogger + ) + val packageName = MediaPlayerData.smartspaceMediaData?.packageName ?: String() + // In case of recommendations hits. + // Check the playing status of media player and the package name. + // To make sure we scroll to the right app's media player. + if ( + isReorderingAllowed || + shouldScrollToKey && + data.isPlaying == true && + packageName == data.packageName + ) { + reorderAllPlayers(curVisibleMediaKey, key) + } else { + needsReordering = true + } } + updatePageIndicator() + mediaCarouselScrollHandler.onPlayersChanged() + mediaFrame.requiresRemeasuring = true + return existingPlayer == null } - } - - private fun createMediaViewHolderInBg(): MediaViewHolder { - return MediaViewHolder.create(LayoutInflater.from(context), mediaContent) - } private fun addSmartspaceMediaRecommendations( key: String, @@ -731,14 +708,15 @@ constructor( debugLogger.logPotentialMemoryLeak(existingSmartspaceMediaKey) } } + val newRecs = mediaControlPanelFactory.get() - val recommendationViewHolder = + newRecs.attachRecommendation( RecommendationViewHolder.create( LayoutInflater.from(context), mediaContent, mediaFlags.isRecommendationCardUpdateEnabled() ) - newRecs.attachRecommendation(recommendationViewHolder) + ) newRecs.mediaViewController.sizeChangedListener = this::updateCarouselDimensions val lp = LinearLayout.LayoutParams( @@ -762,6 +740,17 @@ constructor( reorderAllPlayers(curVisibleMediaKey) updatePageIndicator() mediaFrame.requiresRemeasuring = true + // Check postcondition: mediaContent should have the same number of children as there + // are + // elements in mediaPlayers. + if (MediaPlayerData.players().size != mediaContent.childCount) { + Log.e( + TAG, + "Size of players list and number of views in carousel are out of sync. " + + "Players size is ${MediaPlayerData.players().size}. " + + "View count is ${mediaContent.childCount}." + ) + } } fun removePlayer( 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 e0ca90ec2c58..a72634bcb807 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 @@ -17,7 +17,7 @@ package com.android.systemui.media.controls.ui import android.app.PendingIntent -import android.content.res.ColorStateList +import android.content.res.Configuration import android.testing.AndroidTestingRunner import android.testing.TestableLooper import android.util.MathUtils.abs @@ -26,9 +26,9 @@ import androidx.test.filters.SmallTest import com.android.internal.logging.InstanceId import com.android.keyguard.KeyguardUpdateMonitor import com.android.keyguard.KeyguardUpdateMonitorCallback -import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.classifier.FalsingCollector +import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.dump.DumpManager import com.android.systemui.keyguard.data.repository.FakeKeyguardTransitionRepository import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor @@ -49,7 +49,7 @@ import com.android.systemui.qs.PageIndicator import com.android.systemui.statusbar.notification.collection.provider.OnReorderingAllowedListener import com.android.systemui.statusbar.notification.collection.provider.VisualStabilityProvider import com.android.systemui.statusbar.policy.ConfigurationController -import com.android.systemui.util.concurrency.FakeExecutor +import com.android.systemui.util.concurrency.DelayableExecutor import com.android.systemui.util.mockito.capture import com.android.systemui.util.mockito.eq import com.android.systemui.util.time.FakeSystemClock @@ -89,6 +89,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { @Mock lateinit var mediaHostStatesManager: MediaHostStatesManager @Mock lateinit var mediaHostState: MediaHostState @Mock lateinit var activityStarter: ActivityStarter + @Mock @Main private lateinit var executor: DelayableExecutor @Mock lateinit var mediaDataManager: MediaDataManager @Mock lateinit var configurationController: ConfigurationController @Mock lateinit var falsingCollector: FalsingCollector @@ -112,15 +113,11 @@ class MediaCarouselControllerTest : SysuiTestCase() { private val clock = FakeSystemClock() private lateinit var mediaCarouselController: MediaCarouselController - private lateinit var mainExecutor: FakeExecutor - private lateinit var backgroundExecutor: FakeExecutor @Before fun setup() { MockitoAnnotations.initMocks(this) transitionRepository = FakeKeyguardTransitionRepository() - mainExecutor = FakeExecutor(clock) - backgroundExecutor = FakeExecutor(clock) mediaCarouselController = MediaCarouselController( context, @@ -129,8 +126,7 @@ class MediaCarouselControllerTest : SysuiTestCase() { mediaHostStatesManager, activityStarter, clock, - mainExecutor, - backgroundExecutor, + executor, mediaDataManager, configurationController, falsingCollector, @@ -405,7 +401,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { resumption = true ) ) - runAllReady() assertEquals( MediaPlayerData.getMediaPlayerIndex("paused local"), @@ -515,8 +510,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { false ) mediaCarouselController.shouldScrollToKey = true - runAllReady() - // switching between media players. listener.value.onMediaDataLoaded( "playing local", @@ -538,7 +531,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { resumption = false ) ) - runAllReady() assertEquals( MediaPlayerData.getMediaPlayerIndex("paused local"), @@ -563,7 +555,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { resumption = false ) ) - runAllReady() var playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local") assertEquals( @@ -586,8 +577,6 @@ class MediaCarouselControllerTest : SysuiTestCase() { packageName = "PACKAGE_NAME" ) ) - runAllReady() - playerIndex = MediaPlayerData.getMediaPlayerIndex("playing local") assertEquals(playerIndex, 0) } @@ -684,38 +673,17 @@ class MediaCarouselControllerTest : SysuiTestCase() { } @Test - fun testOnUiModeChanged_playersAreAddedBack() { - mediaCarouselController.pageIndicator = pageIndicator - + fun testOnConfigChanged_playersAreAddedBack() { listener.value.onMediaDataLoaded( - "paused local", + "playing local", null, DATA.copy( active = true, - isPlaying = false, + isPlaying = true, playbackLocation = MediaData.PLAYBACK_LOCAL, resumption = false ) ) - runAllReady() - - val playersSize = MediaPlayerData.players().size - configListener.value.onUiModeChanged() - runAllReady() - - verify(pageIndicator).tintList = - ColorStateList.valueOf(context.getColor(R.color.media_paging_indicator)) - assertEquals(playersSize, MediaPlayerData.players().size) - assertEquals( - MediaPlayerData.getMediaPlayerIndex("paused local"), - mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex - ) - } - - @Test - fun testOnDensityOrFontScaleChanged_playersAreAddedBack() { - mediaCarouselController.pageIndicator = pageIndicator - listener.value.onMediaDataLoaded( "paused local", null, @@ -726,46 +694,14 @@ class MediaCarouselControllerTest : SysuiTestCase() { resumption = false ) ) - runAllReady() val playersSize = MediaPlayerData.players().size - configListener.value.onDensityOrFontScaleChanged() - runAllReady() - - verify(pageIndicator).tintList = - ColorStateList.valueOf(context.getColor(R.color.media_paging_indicator)) - assertEquals(playersSize, MediaPlayerData.players().size) - assertEquals( - MediaPlayerData.getMediaPlayerIndex("paused local"), - mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex - ) - } - @Test - fun testOnThemeChanged_playersAreAddedBack() { - mediaCarouselController.pageIndicator = pageIndicator + configListener.value.onConfigChanged(Configuration()) - listener.value.onMediaDataLoaded( - "paused local", - null, - DATA.copy( - active = true, - isPlaying = false, - playbackLocation = MediaData.PLAYBACK_LOCAL, - resumption = false - ) - ) - runAllReady() - - val playersSize = MediaPlayerData.players().size - configListener.value.onThemeChanged() - runAllReady() - - verify(pageIndicator).tintList = - ColorStateList.valueOf(context.getColor(R.color.media_paging_indicator)) assertEquals(playersSize, MediaPlayerData.players().size) assertEquals( - MediaPlayerData.getMediaPlayerIndex("paused local"), + MediaPlayerData.getMediaPlayerIndex("playing local"), mediaCarouselController.mediaCarouselScrollHandler.visibleMediaIndex ) } @@ -896,9 +832,4 @@ class MediaCarouselControllerTest : SysuiTestCase() { // Verify that seekbar listening attribute in media control panel is set to false. verify(panel, times(MediaPlayerData.players().size)).listening = false } - - private fun runAllReady() { - backgroundExecutor.runAllReady() - mainExecutor.runAllReady() - } } |