From 22aea1c4c3c398da93bce398fabac9dc0971c1f5 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Fri, 26 Aug 2022 10:54:58 -0500 Subject: [PATCH] Fix device name string on output switcher chip The output switcher chip should show a default 'Other Device' string when we do not have reliable device info to show. However MediaDeviceManager was always using the local device name, and MediaControlPanel never actually used the fallback string. This sets the device name to null when it is untrusted, and fixes MCP to show the fallback value in that case. Fixes: 240185112 Test: atest MediaDeviceManagerTest MediaControlPanelTest Test: manual (see bug) Change-Id: I4135b6bedb7f1780e2068673e8980bf7f7b14015 --- .../systemui/media/MediaControlPanel.java | 24 +++++++++----- .../systemui/media/MediaDeviceManager.kt | 14 +++++--- .../systemui/media/MediaControlPanelTest.kt | 11 +++++++ .../systemui/media/MediaDeviceManagerTest.kt | 33 +++++++++++++++---- 4 files changed, 61 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java index 012d76651b23f..b02393b4f73ac 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java +++ b/packages/SystemUI/src/com/android/systemui/media/MediaControlPanel.java @@ -488,8 +488,8 @@ public class MediaControlPanel { TextView deviceName = mMediaViewHolder.getSeamlessText(); final MediaDeviceData device = data.getDevice(); - final boolean enabled; - final boolean seamlessDisabled; + final boolean isTapEnabled; + final boolean useDisabledAlpha; final int iconResource; CharSequence deviceString; if (showBroadcastButton) { @@ -499,21 +499,25 @@ public class MediaControlPanel { && TextUtils.equals(device.getName(), MediaDataUtils.getAppLabel(mContext, mPackageName, mContext.getString( R.string.bt_le_audio_broadcast_dialog_unknown_name))); - seamlessDisabled = !mIsCurrentBroadcastedApp; + useDisabledAlpha = !mIsCurrentBroadcastedApp; // Always be enabled if the broadcast button is shown - enabled = true; + isTapEnabled = true; + + // Defaults for broadcasting state deviceString = mContext.getString(R.string.bt_le_audio_broadcast_dialog_unknown_name); iconResource = R.drawable.settings_input_antenna; } else { // Disable clicking on output switcher for invalid devices and resumption controls - seamlessDisabled = (device != null && !device.getEnabled()) || data.getResumption(); - enabled = !seamlessDisabled; + useDisabledAlpha = (device != null && !device.getEnabled()) || data.getResumption(); + isTapEnabled = !useDisabledAlpha; + + // Defaults for non-broadcasting state deviceString = mContext.getString(R.string.media_seamless_other_device); iconResource = R.drawable.ic_media_home_devices; } - mMediaViewHolder.getSeamlessButton().setAlpha(seamlessDisabled ? DISABLED_ALPHA : 1.0f); - seamlessView.setEnabled(enabled); + mMediaViewHolder.getSeamlessButton().setAlpha(useDisabledAlpha ? DISABLED_ALPHA : 1.0f); + seamlessView.setEnabled(isTapEnabled); if (device != null) { Drawable icon = device.getIcon(); @@ -524,7 +528,9 @@ public class MediaControlPanel { } else { iconView.setImageDrawable(icon); } - deviceString = device.getName(); + if (device.getName() != null) { + deviceString = device.getName(); + } } else { // Set to default icon iconView.setImageResource(iconResource); diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt index 83050503a18f2..267c1f55a6814 100644 --- a/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/MediaDeviceManager.kt @@ -265,7 +265,6 @@ class MediaDeviceManager @Inject constructor( updateCurrent() } - override fun onBroadcastStarted(reason: Int, broadcastId: Int) { if (DEBUG) { Log.d(TAG, "onBroadcastStarted(), reason = $reason , broadcastId = $broadcastId") @@ -279,8 +278,10 @@ class MediaDeviceManager @Inject constructor( } } - override fun onBroadcastMetadataChanged(broadcastId: Int, - metadata: BluetoothLeBroadcastMetadata) { + override fun onBroadcastMetadataChanged( + broadcastId: Int, + metadata: BluetoothLeBroadcastMetadata + ) { if (DEBUG) { Log.d(TAG, "onBroadcastMetadataChanged(), broadcastId = $broadcastId , " + "metadata = $metadata") @@ -291,7 +292,6 @@ class MediaDeviceManager @Inject constructor( override fun onBroadcastStopped(reason: Int, broadcastId: Int) { if (DEBUG) { Log.d(TAG, "onBroadcastStopped(), reason = $reason , broadcastId = $broadcastId") - } updateCurrent() } @@ -344,7 +344,11 @@ class MediaDeviceManager @Inject constructor( // If we have a controller but get a null route, then don't trust the device val enabled = device != null && (controller == null || route != null) - val name = route?.name?.toString() ?: device?.name + val name = if (controller == null || route != null) { + route?.name?.toString() ?: device?.name + } else { + null + } current = MediaDeviceData(enabled, device?.iconWithoutBackground, name, id = device?.id, showBroadcastButton = false) } diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt index 178502269e738..bef46953395ba 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaControlPanelTest.kt @@ -1050,6 +1050,17 @@ public class MediaControlPanelTest : SysuiTestCase() { assertThat(seamless.contentDescription).isEqualTo(fallbackString) } + @Test + fun bindDeviceWithNullName() { + val fallbackString = context.getResources().getString(R.string.media_seamless_other_device) + player.attachPlayer(viewHolder) + val state = mediaData.copy(device = device.copy(name = null)) + player.bindPlayer(state, PACKAGE) + assertThat(seamless.isEnabled()).isTrue() + assertThat(seamlessText.getText()).isEqualTo(fallbackString) + assertThat(seamless.contentDescription).isEqualTo(fallbackString) + } + @Test fun bindDeviceResumptionPlayer() { player.attachPlayer(viewHolder) 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 ee104262dc291..121c8946d1642 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/MediaDeviceManagerTest.kt @@ -59,8 +59,8 @@ 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 +import org.mockito.junit.MockitoJUnit private const val KEY = "TEST_KEY" private const val KEY_OLD = "TEST_KEY_OLD" @@ -402,9 +402,10 @@ public class MediaDeviceManagerTest : SysuiTestCase() { manager.onMediaDataLoaded(KEY, null, mediaData) fakeBgExecutor.runAllReady() fakeFgExecutor.runAllReady() - // THEN the device is disabled + // THEN the device is disabled and name is set to null val data = captureDeviceData(KEY) assertThat(data.enabled).isFalse() + assertThat(data.name).isNull() } @Test @@ -421,9 +422,10 @@ public class MediaDeviceManagerTest : SysuiTestCase() { deviceCallback.onSelectedDeviceStateChanged(device, 1) fakeBgExecutor.runAllReady() fakeFgExecutor.runAllReady() - // THEN the device is disabled + // THEN the device is disabled and name is set to null val data = captureDeviceData(KEY) assertThat(data.enabled).isFalse() + assertThat(data.name).isNull() } @Test @@ -440,9 +442,24 @@ public class MediaDeviceManagerTest : SysuiTestCase() { deviceCallback.onDeviceListUpdate(mutableListOf(device)) fakeBgExecutor.runAllReady() fakeFgExecutor.runAllReady() - // THEN the device is disabled + // THEN the device is disabled and name is set to null val data = captureDeviceData(KEY) assertThat(data.enabled).isFalse() + assertThat(data.name).isNull() + } + + @Test + fun mr2ReturnsRouteWithNullName_useLocalDeviceName() { + // GIVEN that MR2Manager returns a routing session that does not have a name + whenever(route.name).thenReturn(null) + // WHEN a notification is added + manager.onMediaDataLoaded(KEY, null, mediaData) + fakeBgExecutor.runAllReady() + fakeFgExecutor.runAllReady() + // THEN the device is enabled and uses the current connected device name + val data = captureDeviceData(KEY) + assertThat(data.name).isEqualTo(DEVICE_NAME) + assertThat(data.enabled).isTrue() } @Test @@ -647,12 +664,14 @@ public class MediaDeviceManagerTest : SysuiTestCase() { override fun onPlaybackStopped(reason: Int, broadcastId: Int) {} override fun onBroadcastUpdated(reason: Int, broadcastId: Int) {} override fun onBroadcastUpdateFailed(reason: Int, broadcastId: Int) {} - override fun onBroadcastMetadataChanged(broadcastId: Int, - metadata: BluetoothLeBroadcastMetadata) {} + override fun onBroadcastMetadataChanged( + broadcastId: Int, + metadata: BluetoothLeBroadcastMetadata + ) {} } bluetoothLeBroadcast.registerCallback(fakeFgExecutor, callback) - return callback; + return callback } fun setupLeAudioConfiguration(isLeAudio: Boolean) {