From 0c5affb4c42d1dcb41395df4ea1a8100d2033e64 Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Tue, 11 Aug 2020 09:07:20 -0400 Subject: [PATCH] Media - Add player sorting Use various criteria to maintain a sorting order of media players. Leverage TreeMap to maintain order upon player add/update. Bug: 161002989 Bug: 160242133 Merged-in: I07d0219523289fc8c5950d078bd0960bbdb1cc37 Test: manual, using various player types Change-Id: I07d0219523289fc8c5950d078bd0960bbdb1cc37 (cherry picked from commit 642eae6b2f9aca15058e3943042ada18818929bc) --- .../systemui/media/MediaCarouselController.kt | 130 ++++++++++-------- .../com/android/systemui/media/MediaData.kt | 4 + .../systemui/media/MediaDataManager.kt | 11 +- .../media/MediaDataCombineLatestTest.java | 4 +- .../systemui/media/MediaPlayerDataTest.kt | 122 ++++++++++++++++ 5 files changed, 212 insertions(+), 59 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt index 15c60921cca53..865b11f7b7382 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt @@ -11,6 +11,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.LinearLayout +import androidx.annotation.VisibleForTesting import com.android.systemui.R import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.plugins.ActivityStarter @@ -22,6 +23,7 @@ import com.android.systemui.util.Utils import com.android.systemui.util.animation.UniqueObjectHostView import com.android.systemui.util.animation.requiresRemeasuring import com.android.systemui.util.concurrency.DelayableExecutor +import java.util.TreeMap import javax.inject.Inject import javax.inject.Provider import javax.inject.Singleton @@ -103,9 +105,7 @@ class MediaCarouselController @Inject constructor( private val mediaCarousel: MediaScrollView private val mediaCarouselScrollHandler: MediaCarouselScrollHandler val mediaFrame: ViewGroup - val mediaPlayers: MutableMap = mutableMapOf() private lateinit var settingsButton: View - private val mediaData: MutableMap = mutableMapOf() private val mediaContent: ViewGroup private val pageIndicator: PageIndicator private val visualStabilityCallback: VisualStabilityManager.Callback @@ -123,7 +123,7 @@ class MediaCarouselController @Inject constructor( set(value) { if (field != value) { field = value - for (player in mediaPlayers.values) { + for (player in MediaPlayerData.players()) { player.setListening(field) } } @@ -168,20 +168,17 @@ class MediaCarouselController @Inject constructor( true /* persistent */) mediaManager.addListener(object : MediaDataManager.Listener { override fun onMediaDataLoaded(key: String, oldKey: String?, data: MediaData) { - oldKey?.let { mediaData.remove(it) } if (!data.active && !Utils.useMediaResumption(context)) { // This view is inactive, let's remove this! This happens e.g when dismissing / // timing out a view. We still have the data around because resumption could // be on, but we should save the resources and release this. onMediaDataRemoved(key) } else { - mediaData.put(key, data) addOrUpdatePlayer(key, oldKey, data) } } override fun onMediaDataRemoved(key: String) { - mediaData.remove(key) removePlayer(key) } }) @@ -224,53 +221,36 @@ class MediaCarouselController @Inject constructor( } private fun reorderAllPlayers() { - for (mediaPlayer in mediaPlayers.values) { - val view = mediaPlayer.view?.player - if (mediaPlayer.isPlaying && mediaContent.indexOfChild(view) != 0) { - mediaContent.removeView(view) - mediaContent.addView(view, 0) + mediaContent.removeAllViews() + for (mediaPlayer in MediaPlayerData.players()) { + mediaPlayer.view?.let { + mediaContent.addView(it.player) } } mediaCarouselScrollHandler.onPlayersChanged() } private fun addOrUpdatePlayer(key: String, oldKey: String?, data: MediaData) { - // If the key was changed, update entry - val oldData = mediaPlayers[oldKey] - if (oldData != null) { - val oldData = mediaPlayers.remove(oldKey) - mediaPlayers.put(key, oldData!!)?.let { - Log.wtf(TAG, "new key $key already exists when migrating from $oldKey") - } - } - var existingPlayer = mediaPlayers[key] + val existingPlayer = MediaPlayerData.getMediaPlayer(key, oldKey) if (existingPlayer == null) { - existingPlayer = mediaControlPanelFactory.get() - existingPlayer.attach(PlayerViewHolder.create(LayoutInflater.from(context), - mediaContent)) - existingPlayer.mediaViewController.sizeChangedListener = this::updateCarouselDimensions - mediaPlayers[key] = existingPlayer + var newPlayer = mediaControlPanelFactory.get() + newPlayer.attach(PlayerViewHolder.create(LayoutInflater.from(context), mediaContent)) + newPlayer.mediaViewController.sizeChangedListener = this::updateCarouselDimensions + MediaPlayerData.addMediaPlayer(key, data, newPlayer) val lp = LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.WRAP_CONTENT) - existingPlayer.view?.player?.setLayoutParams(lp) - existingPlayer.bind(data) - existingPlayer.setListening(currentlyExpanded) - updatePlayerToState(existingPlayer, noAnimation = true) - if (existingPlayer.isPlaying) { - mediaContent.addView(existingPlayer.view?.player, 0) - } else { - mediaContent.addView(existingPlayer.view?.player) - } + newPlayer.view?.player?.setLayoutParams(lp) + newPlayer.bind(data) + newPlayer.setListening(currentlyExpanded) + updatePlayerToState(newPlayer, noAnimation = true) + reorderAllPlayers() } else { existingPlayer.bind(data) - if (existingPlayer.isPlaying && - mediaContent.indexOfChild(existingPlayer.view?.player) != 0) { - if (visualStabilityManager.isReorderingAllowed) { - mediaContent.removeView(existingPlayer.view?.player) - mediaContent.addView(existingPlayer.view?.player, 0) - } else { - needsReordering = true - } + MediaPlayerData.addMediaPlayer(key, data, existingPlayer) + if (visualStabilityManager.isReorderingAllowed) { + reorderAllPlayers() + } else { + needsReordering = true } } updatePageIndicator() @@ -278,13 +258,13 @@ class MediaCarouselController @Inject constructor( mediaCarousel.requiresRemeasuring = true // Check postcondition: mediaContent should have the same number of children as there are // elements in mediaPlayers. - if (mediaPlayers.size != mediaContent.childCount) { + if (MediaPlayerData.players().size != mediaContent.childCount) { Log.wtf(TAG, "Size of players list and number of views in carousel are out of sync") } } private fun removePlayer(key: String) { - val removed = mediaPlayers.remove(key) + val removed = MediaPlayerData.removeMediaPlayer(key) removed?.apply { mediaCarouselScrollHandler.onPrePlayerRemoved(removed) mediaContent.removeView(removed.view?.player) @@ -295,12 +275,7 @@ class MediaCarouselController @Inject constructor( } private fun recreatePlayers() { - // Note that this will scramble the order of players. Actively playing sessions will, at - // least, still be put in the front. If we want to maintain order, then more work is - // needed. - mediaData.forEach { - key, data -> - removePlayer(key) + MediaPlayerData.mediaData().forEach { (key, data) -> addOrUpdatePlayer(key = key, oldKey = null, data = data) } } @@ -338,7 +313,7 @@ class MediaCarouselController @Inject constructor( currentStartLocation = startLocation currentEndLocation = endLocation currentTransitionProgress = progress - for (mediaPlayer in mediaPlayers.values) { + for (mediaPlayer in MediaPlayerData.players()) { updatePlayerToState(mediaPlayer, immediately) } maybeResetSettingsCog() @@ -387,7 +362,7 @@ class MediaCarouselController @Inject constructor( private fun updateCarouselDimensions() { var width = 0 var height = 0 - for (mediaPlayer in mediaPlayers.values) { + for (mediaPlayer in MediaPlayerData.players()) { val controller = mediaPlayer.mediaViewController // When transitioning the view to gone, the view gets smaller, but the translation // Doesn't, let's add the translation @@ -449,7 +424,7 @@ class MediaCarouselController @Inject constructor( this.desiredLocation = desiredLocation this.desiredHostState = it currentlyExpanded = it.expansion > 0 - for (mediaPlayer in mediaPlayers.values) { + for (mediaPlayer in MediaPlayerData.players()) { if (animate) { mediaPlayer.mediaViewController.animatePendingStateChange( duration = duration, @@ -471,7 +446,7 @@ class MediaCarouselController @Inject constructor( } fun closeGuts() { - mediaPlayers.values.forEach { + MediaPlayerData.players().forEach { it.closeGuts(true) } } @@ -498,3 +473,50 @@ class MediaCarouselController @Inject constructor( } } } + +@VisibleForTesting +internal object MediaPlayerData { + private data class MediaSortKey( + val data: MediaData, + val updateTime: Long = 0, + val isPlaying: Boolean = false + ) + + private val comparator = + compareByDescending { it.isPlaying } + .thenByDescending { it.data.isLocalSession } + .thenByDescending { !it.data.resumption } + .thenByDescending { it.updateTime } + + private val mediaPlayers = TreeMap(comparator) + private val mediaData: MutableMap = mutableMapOf() + + fun addMediaPlayer(key: String, data: MediaData, player: MediaControlPanel) { + removeMediaPlayer(key) + val sortKey = MediaSortKey(data, System.currentTimeMillis(), player.isPlaying()) + mediaData.put(key, sortKey) + mediaPlayers.put(sortKey, player) + } + + fun getMediaPlayer(key: String, oldKey: String?): MediaControlPanel? { + // If the key was changed, update entry + oldKey?.let { + if (it != key) { + mediaData.remove(it)?.let { sortKey -> mediaData.put(key, sortKey) } + } + } + return mediaData.get(key)?.let { mediaPlayers.get(it) } + } + + fun removeMediaPlayer(key: String) = mediaData.remove(key)?.let { mediaPlayers.remove(it) } + + fun mediaData() = mediaData.entries.map { e -> Pair(e.key, e.value.data) } + + fun players() = mediaPlayers.values + + @VisibleForTesting + fun clear() { + mediaData.clear() + mediaPlayers.clear() + } +} diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt index dafc52ad80255..d6a02687c905a 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt @@ -81,6 +81,10 @@ data class MediaData( * Action that should be performed to restart a non active session. */ var resumeAction: Runnable?, + /** + * Local or remote playback + */ + var isLocalSession: Boolean = true, /** * Indicates that this player is a resumption player (ie. It only shows a play actions which * will start the app and start playing). diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt index 33475aca0bfbe..1c6fb2034b5f2 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt @@ -31,6 +31,7 @@ import android.graphics.drawable.Drawable import android.graphics.drawable.Icon import android.media.MediaDescription import android.media.MediaMetadata +import android.media.session.MediaController import android.media.session.MediaSession import android.net.Uri import android.os.UserHandle @@ -337,7 +338,8 @@ class MediaDataManager( ) { val token = sbn.notification.extras.getParcelable(Notification.EXTRA_MEDIA_SESSION) as MediaSession.Token? - val metadata = mediaControllerFactory.create(token).metadata + val mediaController = mediaControllerFactory.create(token) + val metadata = mediaController.metadata // Foreground and Background colors computed from album art val notif: Notification = sbn.notification @@ -429,6 +431,9 @@ class MediaDataManager( } } + val isLocalSession = mediaController.playbackInfo?.playbackType == + MediaController.PlaybackInfo.PLAYBACK_TYPE_LOCAL ?: true + foregroundExecutor.execute { val resumeAction: Runnable? = mediaEntries[key]?.resumeAction val hasCheckedForResume = mediaEntries[key]?.hasCheckedForResume == true @@ -436,8 +441,8 @@ class MediaDataManager( onMediaDataLoaded(key, oldKey, MediaData(sbn.normalizedUserId, true, bgColor, app, smallIconDrawable, artist, song, artWorkIcon, actionIcons, actionsToShowCollapsed, sbn.packageName, token, notif.contentIntent, null, - active, resumeAction = resumeAction, notificationKey = key, - hasCheckedForResume = hasCheckedForResume)) + active, resumeAction = resumeAction, isLocalSession = isLocalSession, + notificationKey = key, hasCheckedForResume = hasCheckedForResume)) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java index 492b33e3c4a61..2e794a40d2385 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDataCombineLatestTest.java @@ -83,8 +83,8 @@ public class MediaDataCombineLatestTest extends SysuiTestCase { mManager.addListener(mListener); mMediaData = new MediaData(USER_ID, true, BG_COLOR, APP, null, ARTIST, TITLE, null, - new ArrayList<>(), new ArrayList<>(), PACKAGE, null, null, null, true, null, false, - KEY, false); + new ArrayList<>(), new ArrayList<>(), PACKAGE, null, null, null, true, null, true, + false, KEY, false); mDeviceData = new MediaDeviceData(true, null, DEVICE_NAME); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt new file mode 100644 index 0000000000000..118cffc2d5b81 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaPlayerDataTest.kt @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2020 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 androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` as whenever + +@SmallTest +@RunWith(AndroidTestingRunner::class) +public class MediaPlayerDataTest : SysuiTestCase() { + + companion object { + val LOCAL = true + val RESUMPTION = true + } + + @Before + fun setup() { + MediaPlayerData.clear() + } + + @Test + fun addPlayingThenRemote() { + val playerIsPlaying = mock(MediaControlPanel::class.java) + whenever(playerIsPlaying.isPlaying).thenReturn(true) + val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION) + + val playerIsRemote = mock(MediaControlPanel::class.java) + whenever(playerIsRemote.isPlaying).thenReturn(false) + val dataIsRemote = createMediaData(!LOCAL, !RESUMPTION) + + MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying) + MediaPlayerData.addMediaPlayer("2", dataIsRemote, playerIsRemote) + + val players = MediaPlayerData.players() + assertThat(players).hasSize(2) + assertThat(players).containsExactly(playerIsPlaying, playerIsRemote).inOrder() + } + + @Test + fun switchPlayersPlaying() { + val playerIsPlaying1 = mock(MediaControlPanel::class.java) + whenever(playerIsPlaying1.isPlaying).thenReturn(true) + val dataIsPlaying1 = createMediaData(LOCAL, !RESUMPTION) + + val playerIsPlaying2 = mock(MediaControlPanel::class.java) + whenever(playerIsPlaying2.isPlaying).thenReturn(false) + val dataIsPlaying2 = createMediaData(LOCAL, !RESUMPTION) + + MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1) + MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2) + + whenever(playerIsPlaying1.isPlaying).thenReturn(false) + whenever(playerIsPlaying2.isPlaying).thenReturn(true) + + MediaPlayerData.addMediaPlayer("1", dataIsPlaying1, playerIsPlaying1) + MediaPlayerData.addMediaPlayer("2", dataIsPlaying2, playerIsPlaying2) + + val players = MediaPlayerData.players() + assertThat(players).hasSize(2) + assertThat(players).containsExactly(playerIsPlaying2, playerIsPlaying1).inOrder() + } + + @Test + fun fullOrderTest() { + val playerIsPlaying = mock(MediaControlPanel::class.java) + whenever(playerIsPlaying.isPlaying).thenReturn(true) + val dataIsPlaying = createMediaData(LOCAL, !RESUMPTION) + + val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java) + whenever(playerIsPlayingAndRemote.isPlaying).thenReturn(true) + val dataIsPlayingAndRemote = createMediaData(!LOCAL, !RESUMPTION) + + val playerIsStoppedAndLocal = mock(MediaControlPanel::class.java) + whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false) + val dataIsStoppedAndLocal = createMediaData(LOCAL, !RESUMPTION) + + val playerIsStoppedAndRemote = mock(MediaControlPanel::class.java) + whenever(playerIsStoppedAndLocal.isPlaying).thenReturn(false) + val dataIsStoppedAndRemote = createMediaData(!LOCAL, !RESUMPTION) + + val playerCanResume = mock(MediaControlPanel::class.java) + whenever(playerCanResume.isPlaying).thenReturn(false) + val dataCanResume = createMediaData(LOCAL, RESUMPTION) + + MediaPlayerData.addMediaPlayer("3", dataIsStoppedAndLocal, playerIsStoppedAndLocal) + MediaPlayerData.addMediaPlayer("5", dataIsStoppedAndRemote, playerIsStoppedAndRemote) + MediaPlayerData.addMediaPlayer("4", dataCanResume, playerCanResume) + MediaPlayerData.addMediaPlayer("1", dataIsPlaying, playerIsPlaying) + MediaPlayerData.addMediaPlayer("2", dataIsPlayingAndRemote, playerIsPlayingAndRemote) + + val players = MediaPlayerData.players() + assertThat(players).hasSize(5) + assertThat(players).containsExactly(playerIsPlaying, playerIsPlayingAndRemote, + playerIsStoppedAndLocal, playerCanResume, playerIsStoppedAndRemote).inOrder() + } + + private fun createMediaData(isLocalSession: Boolean, resumption: Boolean) = + MediaData(0, false, 0, null, null, null, null, null, emptyList(), emptyList(), "", + null, null, null, true, null, isLocalSession, resumption, null, false) +}