From f35cfddd0375ecf834e0a05dbcc75827c98c83a1 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 | 18 +++- .../systemui/media/MediaDeviceManager.kt | 4 +- .../systemui/media/MediaDeviceManagerTest.kt | 86 +++++++++++++++++++ 3 files changed, 105 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 360f86548e13e..aeeef6b24a1c4 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaData.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaData.kt @@ -216,4 +216,20 @@ data class MediaDeviceData /** Unique id for this device */ val id: String? = null -) +) { + /** + * 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 + } +} diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt index 8558859638d5b..baed1c4286e9c 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt @@ -155,8 +155,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 18ee79138b525..187210a40b6dd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt @@ -47,7 +47,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 @@ -55,6 +57,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" @@ -456,6 +459,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)