From 4c8144ee848bcba10f27ef035f43e4ac40d2bde5 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Mon, 13 Jun 2022 18:50:27 -0500 Subject: [PATCH] Forward device updates when info changes In some cases, the device/route name could change without also updating the ID. This updates the check to include all fields of MediaDeviceData except for the icon (which can change reference more frequently than is necessary to reload). Now if the name, status, or intent has changed, listeners will be updated to have the new info. Bug: 235627592 Test: manual Test: atest MediaDataManagerTest Change-Id: Iefd7ce0e8859b89db2bc75c243b3a04a0cb3915d Merged-In: Iefd7ce0e8859b89db2bc75c243b3a04a0cb3915d --- .../com/android/systemui/media/MediaData.kt | 19 +++- .../systemui/media/MediaDeviceManager.kt | 4 +- .../systemui/media/MediaDeviceManagerTest.kt | 86 +++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt index b2ab12a81bcc6..5b2cda038bbdd 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt @@ -219,4 +219,21 @@ data class MediaDeviceData /** Whether or not to show the broadcast button */ val showBroadcastButton: Boolean -) +) { + /** + * Check whether [MediaDeviceData] objects are equal in all fields except the icon. The icon + * is ignored because it can change by reference frequently depending on the device type's + * implementation, but this is not usually relevant unless other info has changed + */ + fun equalsWithoutIcon(other: MediaDeviceData?): Boolean { + if (other == null) { + return false + } + + return enabled == other.enabled && + name == other.name && + intent == other.intent && + id == other.id && + showBroadcastButton == other.showBroadcastButton + } +} diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt index b552d9fb584ae..83050503a18f2 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt @@ -168,8 +168,8 @@ class MediaDeviceManager @Inject constructor( private var playbackType = PLAYBACK_TYPE_UNKNOWN private var current: MediaDeviceData? = null set(value) { - val hasSameId = value?.id != null && value.id == field?.id - if (!started || (!hasSameId && value != field)) { + val sameWithoutIcon = value != null && value.equalsWithoutIcon(field) + if (!started || !sameWithoutIcon) { field = value fgExecutor.execute { processDevice(key, oldKey, value) diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt index 11fe87d363a02..ee104262dc291 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt @@ -56,7 +56,9 @@ import org.mockito.Mockito.any import org.mockito.Mockito.mock import org.mockito.Mockito.never import org.mockito.Mockito.reset +import org.mockito.Mockito.times import org.mockito.Mockito.verify +import org.mockito.Mockito.verifyNoMoreInteractions import org.mockito.junit.MockitoJUnit import org.mockito.Mockito.`when` as whenever @@ -64,6 +66,7 @@ private const val KEY = "TEST_KEY" private const val KEY_OLD = "TEST_KEY_OLD" private const val PACKAGE = "PKG" private const val SESSION_KEY = "SESSION_KEY" +private const val DEVICE_ID = "DEVICE_ID" private const val DEVICE_NAME = "DEVICE_NAME" private const val REMOTE_DEVICE_NAME = "REMOTE_DEVICE_NAME" private const val BROADCAST_APP_NAME = "BROADCAST_APP_NAME" @@ -477,6 +480,89 @@ public class MediaDeviceManagerTest : SysuiTestCase() { verify(mr2, never()).getRoutingSessionForMediaController(eq(controller)) } + @Test + fun deviceIdChanged_informListener() { + // GIVEN a notification is added, with a particular device connected + whenever(device.id).thenReturn(DEVICE_ID) + manager.onMediaDataLoaded(KEY, null, mediaData) + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + + // and later the manager gets a new device ID + val deviceCallback = captureCallback() + val updatedId = DEVICE_ID + "_new" + whenever(device.id).thenReturn(updatedId) + deviceCallback.onDeviceListUpdate(mutableListOf(device)) + + // THEN the listener gets the updated info + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + + val dataCaptor = ArgumentCaptor.forClass(MediaDeviceData::class.java) + verify(listener, times(2)).onMediaDeviceChanged(eq(KEY), any(), dataCaptor.capture()) + + val firstDevice = dataCaptor.allValues.get(0) + assertThat(firstDevice.id).isEqualTo(DEVICE_ID) + + val secondDevice = dataCaptor.allValues.get(1) + assertThat(secondDevice.id).isEqualTo(updatedId) + } + + @Test + fun deviceNameChanged_informListener() { + // GIVEN a notification is added, with a particular device connected + whenever(device.id).thenReturn(DEVICE_ID) + whenever(device.name).thenReturn(DEVICE_NAME) + manager.onMediaDataLoaded(KEY, null, mediaData) + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + + // and later the manager gets a new device name + val deviceCallback = captureCallback() + val updatedName = DEVICE_NAME + "_new" + whenever(device.name).thenReturn(updatedName) + deviceCallback.onDeviceListUpdate(mutableListOf(device)) + + // THEN the listener gets the updated info + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + + val dataCaptor = ArgumentCaptor.forClass(MediaDeviceData::class.java) + verify(listener, times(2)).onMediaDeviceChanged(eq(KEY), any(), dataCaptor.capture()) + + val firstDevice = dataCaptor.allValues.get(0) + assertThat(firstDevice.name).isEqualTo(DEVICE_NAME) + + val secondDevice = dataCaptor.allValues.get(1) + assertThat(secondDevice.name).isEqualTo(updatedName) + } + + @Test + fun deviceIconChanged_doesNotCallListener() { + // GIVEN a notification is added, with a particular device connected + whenever(device.id).thenReturn(DEVICE_ID) + whenever(device.name).thenReturn(DEVICE_NAME) + val firstIcon = mock(Drawable::class.java) + whenever(device.icon).thenReturn(firstIcon) + manager.onMediaDataLoaded(KEY, null, mediaData) + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + + val dataCaptor = ArgumentCaptor.forClass(MediaDeviceData::class.java) + verify(listener).onMediaDeviceChanged(eq(KEY), any(), dataCaptor.capture()) + + // and later the manager gets a callback with only the icon changed + val deviceCallback = captureCallback() + val secondIcon = mock(Drawable::class.java) + whenever(device.icon).thenReturn(secondIcon) + deviceCallback.onDeviceListUpdate(mutableListOf(device)) + + // THEN the listener is not called again + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + verifyNoMoreInteractions(listener) + } + @Test fun testRemotePlaybackDeviceOverride() { whenever(route.name).thenReturn(DEVICE_NAME)