Merge "Reduce chance for leak in MediaPlayerData" into sc-dev (8e4f478f) · Commits · e / os / android_frameworks_base

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

+16 −10

Original line number	Diff line number	Diff line
		@@ -204,7 +204,7 @@ class MediaCarouselController @Inject constructor(
		isSsReactivated: Boolean
		) {
		if (addOrUpdatePlayer(key, oldKey, data)) {
		MediaPlayerData.getMediaPlayer(key, null)?.let {
		MediaPlayerData.getMediaPlayer(key)?.let {
		logSmartspaceCardReported(759, // SMARTSPACE_CARD_RECEIVED
		it.mInstanceId,
		/* isRecommendationCard */ false,
		@@ -241,7 +241,7 @@ class MediaCarouselController @Inject constructor(
		if (DEBUG) Log.d(TAG, "Loading Smartspace media update")
		if (data.isActive) {
		addSmartspaceMediaRecommendations(key, data, shouldPrioritize)
		MediaPlayerData.getMediaPlayer(key, null)?.let {
		MediaPlayerData.getMediaPlayer(key)?.let {
		logSmartspaceCardReported(759, // SMARTSPACE_CARD_RECEIVED
		it.mInstanceId,
		/* isRecommendationCard */ true,
		@@ -344,7 +344,8 @@ class MediaCarouselController @Inject constructor(
		// Returns true if new player is added
		private fun addOrUpdatePlayer(key: String, oldKey: String?, data: MediaData): Boolean {
		val dataCopy = data.copy(backgroundColor = bgColor)
		val existingPlayer = MediaPlayerData.getMediaPlayer(key, oldKey)
		MediaPlayerData.moveIfExists(oldKey, key)
		val existingPlayer = MediaPlayerData.getMediaPlayer(key)
		val curVisibleMediaKey = MediaPlayerData.playerKeys()
		.elementAtOrNull(mediaCarouselScrollHandler.visibleMediaIndex)
		if (existingPlayer == null) {
		@@ -386,7 +387,7 @@ class MediaCarouselController @Inject constructor(
		shouldPrioritize: Boolean
		) {
		if (DEBUG) Log.d(TAG, "Updating smartspace target in carousel")
		if (MediaPlayerData.getMediaPlayer(key, null) != null) {
		if (MediaPlayerData.getMediaPlayer(key) != null) {
		Log.w(TAG, "Skip adding smartspace target in carousel")
		return
		}
		@@ -795,13 +796,18 @@ internal object MediaPlayerData {
		smartspaceMediaData = data
		}

		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) }
		fun moveIfExists(oldKey: String?, newKey: String) {
		if (oldKey == null \|\| oldKey == newKey) {
		return
		}

		mediaData.remove(oldKey)?.let {
		removeMediaPlayer(newKey)
		mediaData.put(newKey, it)
		}
		}

		fun getMediaPlayer(key: String): MediaControlPanel? {
		return mediaData.get(key)?.let { mediaPlayers.get(it) }
		}

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

+30 −2

Original line number	Diff line number	Diff line
		@@ -22,14 +22,24 @@ import com.android.systemui.SysuiTestCase
		import com.google.common.truth.Truth.assertThat
		import org.junit.Before
		import org.junit.Ignore
		import org.junit.Rule
		import org.junit.Test
		import org.junit.runner.RunWith
		import org.mockito.Mock
		import org.mockito.Mockito.mock
		import org.mockito.junit.MockitoJUnit

		@SmallTest
		@RunWith(AndroidTestingRunner::class)
		public class MediaPlayerDataTest : SysuiTestCase() {

		@Mock
		private lateinit var playerIsPlaying: MediaControlPanel

		@JvmField
		@Rule
		val mockito = MockitoJUnit.rule()

		companion object {
		val LOCAL = true
		val RESUMPTION = true
		@@ -44,7 +54,6 @@ public class MediaPlayerDataTest : SysuiTestCase() {

		@Test
		fun addPlayingThenRemote() {
		val playerIsPlaying = mock(MediaControlPanel::class.java)
		val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

		val playerIsRemote = mock(MediaControlPanel::class.java)
		@@ -83,7 +92,6 @@ public class MediaPlayerDataTest : SysuiTestCase() {

		@Test
		fun fullOrderTest() {
		val playerIsPlaying = mock(MediaControlPanel::class.java)
		val dataIsPlaying = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

		val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
		@@ -115,6 +123,26 @@ public class MediaPlayerDataTest : SysuiTestCase() {
		playerUndetermined).inOrder()
		}

		@Test
		fun testMoveMediaKeysAround() {
		val keyA = "a"
		val keyB = "b"

		val data = createMediaData("app1", PLAYING, LOCAL, !RESUMPTION)

		MediaPlayerData.addMediaPlayer(keyA, data, playerIsPlaying)
		MediaPlayerData.addMediaPlayer(keyB, data, playerIsPlaying)

		assertThat(MediaPlayerData.players()).hasSize(2)

		MediaPlayerData.moveIfExists(keyA, keyB)

		assertThat(MediaPlayerData.players()).hasSize(1)

		assertThat(MediaPlayerData.getMediaPlayer(keyA)).isNull()
		assertThat(MediaPlayerData.getMediaPlayer(keyB)).isNotNull()
		}

		private fun createMediaData(
		app: String,
		isPlaying: Boolean?,