Fix sorting issues with remote and resume controls

- Sort paused controls by last active time, regardless of whether they
are local or remote sessions
- Ensure resume controls have isPlaying=false
- Add test for ordering

Fixes: 198099352
Fixes: 192259825
Test: manual
Test: atest com.android.systemui.media
Change-Id: Ie5717cbf2dae3701e0ebff6b693a3bb5640accf5

packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt

@@ -833,11 +833,12 @@ internal object MediaPlayerData {
     )
 
     private val comparator =
-            compareByDescending<MediaSortKey> { it.data.isPlaying }
+            compareByDescending<MediaSortKey> { it.data.isPlaying == true && it.data.isLocalSession }
+                    .thenByDescending { it.data.isPlaying }
                     .thenByDescending { if (shouldPrioritizeSs) it.isSsMediaRec else !it.isSsMediaRec }
-                    .thenByDescending { it.data.isLocalSession }
                     .thenByDescending { !it.data.resumption }
                     .thenByDescending { it.updateTime }
+                    .thenByDescending { !it.data.isLocalSession }
 
     private val mediaPlayers = TreeMap<MediaSortKey, MediaControlPanel>(comparator)
     private val mediaData: MutableMap<String, MediaSortKey> = mutableMapOf()

packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt

@@ -768,7 +768,7 @@ class MediaDataManager(
             val resumeAction = getResumeMediaAction(removed.resumeAction!!)
             val updated = removed.copy(token = null, actions = listOf(resumeAction),
                     actionsToShowInCompact = listOf(0), active = false, resumption = true,
-                    isClearable = true)
+                    isPlaying = false, isClearable = true)
             val pkg = removed.packageName
             val migrate = mediaEntries.put(pkg, updated) == null
             // Notify listeners of "new" controls when migrating or removed and update when not

packages/SystemUI/tests/src/com/android/systemui/media/MediaCarouselControllerTest.kt

@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.media
+
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper
+import androidx.test.filters.SmallTest
+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.plugins.ActivityStarter
+import com.android.systemui.plugins.FalsingManager
+import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager
+import com.android.systemui.statusbar.policy.ConfigurationController
+import com.android.systemui.util.concurrency.DelayableExecutor
+import com.android.systemui.util.time.FakeSystemClock
+import junit.framework.Assert.assertEquals
+import junit.framework.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.Mock
+import org.mockito.MockitoAnnotations
+import javax.inject.Provider
+
+private val DATA = MediaData(
+    userId = -1,
+    initialized = false,
+    backgroundColor = 0,
+    app = null,
+    appIcon = null,
+    artist = null,
+    song = null,
+    artwork = null,
+    actions = emptyList(),
+    actionsToShowInCompact = emptyList(),
+    packageName = "INVALID",
+    token = null,
+    clickIntent = null,
+    device = null,
+    active = true,
+    resumeAction = null)
+
+private val SMARTSPACE_KEY = "smartspace"
+
+@SmallTest
+@TestableLooper.RunWithLooper(setAsMainLooper = true)
+@RunWith(AndroidTestingRunner::class)
+class MediaCarouselControllerTest : SysuiTestCase() {
+
+    @Mock lateinit var mediaControlPanelFactory: Provider<MediaControlPanel>
+    @Mock lateinit var panel: MediaControlPanel
+    @Mock lateinit var visualStabilityManager: VisualStabilityManager
+    @Mock lateinit var mediaHostStatesManager: MediaHostStatesManager
+    @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
+    @Mock lateinit var falsingManager: FalsingManager
+    @Mock lateinit var dumpManager: DumpManager
+
+    private val clock = FakeSystemClock()
+    private lateinit var mediaCarouselController: MediaCarouselController
+
+    @Before
+    fun setup() {
+        MockitoAnnotations.initMocks(this)
+
+        mediaCarouselController = MediaCarouselController(
+            context,
+            mediaControlPanelFactory,
+            visualStabilityManager,
+            mediaHostStatesManager,
+            activityStarter,
+            clock,
+            executor,
+            mediaDataManager,
+            configurationController,
+            falsingCollector,
+            falsingManager,
+            dumpManager
+        )
+
+        MediaPlayerData.clear()
+    }
+
+    @Test
+    fun testPlayerOrdering() {
+        // Test values: key, data, last active time
+        val playingLocal = Triple("playing local",
+            DATA.copy(active = true, isPlaying = true, isLocalSession = true, resumption = false),
+            4500L)
+
+        val playingRemote = Triple("playing remote",
+            DATA.copy(active = true, isPlaying = true, isLocalSession = false, resumption = false),
+            5000L)
+
+        val pausedLocal = Triple("paused local",
+            DATA.copy(active = true, isPlaying = false, isLocalSession = true, resumption = false),
+            1000L)
+
+        val pausedRemote = Triple("paused remote",
+            DATA.copy(active = true, isPlaying = false, isLocalSession = false, resumption = false),
+            2000L)
+
+        val resume1 = Triple("resume 1",
+            DATA.copy(active = false, isPlaying = false, isLocalSession = true, resumption = true),
+            500L)
+
+        val resume2 = Triple("resume 2",
+            DATA.copy(active = false, isPlaying = false, isLocalSession = true, resumption = true),
+            1000L)
+
+        // Expected ordering for media players:
+        // Actively playing local sessions
+        // Actively playing remote sessions
+        // Paused sessions, by last active
+        // Resume controls, by last active
+
+        val expected = listOf(playingLocal, playingRemote, pausedRemote, pausedLocal, resume2,
+            resume1)
+
+        expected.forEach {
+            clock.setCurrentTimeMillis(it.third)
+            MediaPlayerData.addMediaPlayer(it.first, it.second.copy(notificationKey = it.first),
+                panel, clock)
+        }
+
+        for ((index, key) in MediaPlayerData.playerKeys().withIndex()) {
+            assertEquals(expected.get(index).first, key.data.notificationKey)
+        }
+    }
+
+    @Test
+    fun testOrderWithSmartspace_prioritized() {
+        testPlayerOrdering()
+
+        // If smartspace is prioritized
+        MediaPlayerData.addMediaRecommendation(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA, panel,
+            true, clock)
+
+        // Then it should be shown immediately after any actively playing controls
+        assertTrue(MediaPlayerData.playerKeys().elementAt(2).isSsMediaRec)
+    }
+
+    @Test
+    fun testOrderWithSmartspace_notPrioritized() {
+        testPlayerOrdering()
+
+        // If smartspace is not prioritized
+        MediaPlayerData.addMediaRecommendation(SMARTSPACE_KEY, EMPTY_SMARTSPACE_MEDIA_DATA, panel,
+            false, clock)
+
+        // Then it should be shown at the end of the carousel
+        val size = MediaPlayerData.playerKeys().size
+        assertTrue(MediaPlayerData.playerKeys().elementAt(size - 1).isSsMediaRec)
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/media/MediaDataFilterTest.kt

@@ -86,6 +86,7 @@ class MediaDataFilterTest : SysuiTestCase() {
     @Before
     fun setup() {
         MockitoAnnotations.initMocks(this)
+        MediaPlayerData.clear()
         mediaDataFilter = MediaDataFilter(context, broadcastDispatcher, mediaResumeListener,
                 lockscreenUserManager, executor, clock)
         mediaDataFilter.mediaDataManager = mediaDataManager

packages/SystemUI/tests/src/com/android/systemui/media/MediaDataManagerTest.kt

@@ -255,6 +255,7 @@ class MediaDataManagerTest : SysuiTestCase() {
             .onMediaDataLoaded(eq(PACKAGE_NAME), eq(KEY), capture(mediaDataCaptor), eq(true),
                     eq(false))
         assertThat(mediaDataCaptor.value.resumption).isTrue()
+        assertThat(mediaDataCaptor.value.isPlaying).isFalse()
     }
 
     @Test

packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt

@@ -127,7 +127,7 @@ public class MediaPlayerDataTest : SysuiTestCase() {
         val players = MediaPlayerData.players()
         assertThat(players).hasSize(6)
         assertThat(players).containsExactly(playerIsPlaying, playerIsPlayingAndRemote,
-            playerIsStoppedAndLocal, playerCanResume, playerIsStoppedAndRemote,
+            playerIsStoppedAndRemote, playerIsStoppedAndLocal, playerCanResume,
             playerUndetermined).inOrder()
     }