Merge "Revert "Run inflations of UMO in a background thread"" into tm-qpr-dev

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(

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,9 +673,17 @@ class MediaCarouselControllerTest : SysuiTestCase() {
     }
 
     @Test
-    fun testOnUiModeChanged_playersAreAddedBack() {
-        mediaCarouselController.pageIndicator = pageIndicator
-
+    fun testOnConfigChanged_playersAreAddedBack() {
+        listener.value.onMediaDataLoaded(
+            "playing local",
+            null,
+            DATA.copy(
+                active = true,
+                isPlaying = true,
+                playbackLocation = MediaData.PLAYBACK_LOCAL,
+                resumption = false
+            )
+        )
         listener.value.onMediaDataLoaded(
             "paused local",
             null,
@@ -697,75 +694,14 @@ class MediaCarouselControllerTest : SysuiTestCase() {
                 resumption = false
             )
         )
-        runAllReady()
 
         val playersSize = MediaPlayerData.players().size
-        configListener.value.onUiModeChanged()
-        runAllReady()
 
-        verify(pageIndicator).tintList =
-            ColorStateList.valueOf(context.getColor(R.color.media_paging_indicator))
+        configListener.value.onConfigChanged(Configuration())
+
         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,
-            DATA.copy(
-                active = true,
-                isPlaying = false,
-                playbackLocation = MediaData.PLAYBACK_LOCAL,
-                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
-
-        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()
-    }
 }