Merge "Reduce chance for leak in MediaPlayerData" into sc-dev am: 8e4f478fb8 am: c35463b10a (054b9a20) · Commits · e / os / android_frameworks_base

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

+16 −10

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

	fun getMediaPlayer(key: String, oldKey: String?): MediaControlPanel? {		fun moveIfExists(oldKey: String?, newKey: String) {
	// If the key was changed, update entry		if (oldKey == null \|\| oldKey == newKey) {
	oldKey?.let {		return
	if (it != key) {		}
	mediaData.remove(it)?.let { sortKey -> mediaData.put(key, sortKey) }
			mediaData.remove(oldKey)?.let {
			removeMediaPlayer(newKey)
			mediaData.put(newKey, it)
	}		}
	}		}

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

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

+30 −2

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

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

			@Mock
			private lateinit var playerIsPlaying: MediaControlPanel

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

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

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

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

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

	val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)		val playerIsPlayingAndRemote = mock(MediaControlPanel::class.java)
	@@ -115,6 +123,26 @@ public class MediaPlayerDataTest : SysuiTestCase() {
	playerUndetermined).inOrder()		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(		private fun createMediaData(
	app: String,		app: String,
	isPlaying: Boolean?,		isPlaying: Boolean?,