Merge "Media - Fix sysui crash on media image load" into rvc-qpr-dev
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
index 15c6092..865b11f 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.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.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 @@
private val mediaCarousel: MediaScrollView
private val mediaCarouselScrollHandler: MediaCarouselScrollHandler
val mediaFrame: ViewGroup
- val mediaPlayers: MutableMap<String, MediaControlPanel> = mutableMapOf()
private lateinit var settingsButton: View
- private val mediaData: MutableMap<String, MediaData> = mutableMapOf()
private val mediaContent: ViewGroup
private val pageIndicator: PageIndicator
private val visualStabilityCallback: VisualStabilityManager.Callback
@@ -123,7 +123,7 @@
set(value) {
if (field != value) {
field = value
- for (player in mediaPlayers.values) {
+ for (player in MediaPlayerData.players()) {
player.setListening(field)
}
}
@@ -168,20 +168,17 @@
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 @@
}
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 @@
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 @@
}
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 @@
currentStartLocation = startLocation
currentEndLocation = endLocation
currentTransitionProgress = progress
- for (mediaPlayer in mediaPlayers.values) {
+ for (mediaPlayer in MediaPlayerData.players()) {
updatePlayerToState(mediaPlayer, immediately)
}
maybeResetSettingsCog()
@@ -387,7 +362,7 @@
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 @@
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 @@
}
fun closeGuts() {
- mediaPlayers.values.forEach {
+ MediaPlayerData.players().forEach {
it.closeGuts(true)
}
}
@@ -498,3 +473,50 @@
}
}
}
+
+@VisibleForTesting
+internal object MediaPlayerData {
+ private data class MediaSortKey(
+ val data: MediaData,
+ val updateTime: Long = 0,
+ val isPlaying: Boolean = false
+ )
+
+ private val comparator =
+ compareByDescending<MediaSortKey> { it.isPlaying }
+ .thenByDescending { it.data.isLocalSession }
+ .thenByDescending { !it.data.resumption }
+ .thenByDescending { it.updateTime }
+
+ private val mediaPlayers = TreeMap<MediaSortKey, MediaControlPanel>(comparator)
+ private val mediaData: MutableMap<String, MediaSortKey> = 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 dafc52a..d6a0268 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt
@@ -82,6 +82,10 @@
*/
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 ad2bb39..7e246c8 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.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 @@
) {
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 @@
}
}
+ 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 @@
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 492b33e..2e794a4 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 @@
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 0000000..118cffc
--- /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<Int>(), "",
+ null, null, null, true, null, isLocalSession, resumption, null, false)
+}
