[Audiosharing] Fix race condition for profile ready and flow collect
Since we can not garantee the order of profile ready and flow collect,
there is some cases AudioSharingRepository#inAudioSharing is always
flowOf(false) and volumeMap is always flowOf(emptyMap()) if the profile
is not available when flow is collected.
Test: atest
Bug: 336716411
Flag: com.android.settingslib.flags.volume_dialog_audio_sharing_fix
Change-Id: Idfc4f68c381a875737b718dd39877d15bcfc69bd
diff --git a/packages/SettingsLib/src/com/android/settingslib/bluetooth/LocalBluetoothProfileManagerCallbackExt.kt b/packages/SettingsLib/src/com/android/settingslib/bluetooth/LocalBluetoothProfileManagerCallbackExt.kt
new file mode 100644
index 0000000..b7338fc
--- /dev/null
+++ b/packages/SettingsLib/src/com/android/settingslib/bluetooth/LocalBluetoothProfileManagerCallbackExt.kt
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.settingslib.bluetooth
+
+import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.channels.awaitClose
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.buffer
+import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.launch
+
+/** [Flow] for [LocalBluetoothProfileManager.ServiceListener] service state changes */
+val LocalBluetoothProfileManager.onServiceStateChanged: Flow<Unit>
+ get() =
+ callbackFlow {
+ val listener =
+ object : LocalBluetoothProfileManager.ServiceListener {
+ override fun onServiceConnected() {
+ launch { trySend(Unit) }
+ }
+
+ override fun onServiceDisconnected() {
+ launch { trySend(Unit) }
+ }
+ }
+ addServiceListener(listener)
+ awaitClose { removeServiceListener(listener) }
+ }
+ .buffer(capacity = Channel.CONFLATED)
\ No newline at end of file
diff --git a/packages/SettingsLib/src/com/android/settingslib/volume/data/repository/AudioSharingRepository.kt b/packages/SettingsLib/src/com/android/settingslib/volume/data/repository/AudioSharingRepository.kt
index 99d5891..7a66335 100644
--- a/packages/SettingsLib/src/com/android/settingslib/volume/data/repository/AudioSharingRepository.kt
+++ b/packages/SettingsLib/src/com/android/settingslib/volume/data/repository/AudioSharingRepository.kt
@@ -30,6 +30,7 @@
import com.android.settingslib.bluetooth.LocalBluetoothManager
import com.android.settingslib.bluetooth.onBroadcastStartedOrStopped
import com.android.settingslib.bluetooth.onProfileConnectionStateChanged
+import com.android.settingslib.bluetooth.onServiceStateChanged
import com.android.settingslib.bluetooth.onSourceConnectedOrRemoved
import com.android.settingslib.volume.data.repository.AudioSharingRepository.Companion.AUDIO_SHARING_VOLUME_MAX
import com.android.settingslib.volume.data.repository.AudioSharingRepository.Companion.AUDIO_SHARING_VOLUME_MIN
@@ -90,13 +91,24 @@
private val coroutineScope: CoroutineScope,
private val backgroundCoroutineContext: CoroutineContext,
) : AudioSharingRepository {
+ private val isAudioSharingProfilesReady: StateFlow<Boolean> =
+ btManager.profileManager.onServiceStateChanged
+ .map { isAudioSharingProfilesReady() }
+ .onStart { emit(isAudioSharingProfilesReady()) }
+ .flowOn(backgroundCoroutineContext)
+ .stateIn(coroutineScope, SharingStarted.WhileSubscribed(), false)
+
override val inAudioSharing: Flow<Boolean> =
- btManager.profileManager.leAudioBroadcastProfile?.let { broadcast ->
- broadcast.onBroadcastStartedOrStopped
- .map { isBroadcasting() }
- .onStart { emit(isBroadcasting()) }
- .flowOn(backgroundCoroutineContext)
- } ?: flowOf(false)
+ isAudioSharingProfilesReady.flatMapLatest { ready ->
+ if (ready) {
+ btManager.profileManager.leAudioBroadcastProfile.onBroadcastStartedOrStopped
+ .map { isBroadcasting() }
+ .onStart { emit(isBroadcasting()) }
+ .flowOn(backgroundCoroutineContext)
+ } else {
+ flowOf(false)
+ }
+ }
private val primaryChange: Flow<Unit> = callbackFlow {
val callback =
@@ -108,7 +120,8 @@
contentResolver.registerContentObserver(
Settings.Secure.getUriFor(BluetoothUtils.getPrimaryGroupIdUriForBroadcast()),
false,
- callback)
+ callback
+ )
awaitClose { contentResolver.unregisterContentObserver(callback) }
}
@@ -120,64 +133,80 @@
.stateIn(
coroutineScope,
SharingStarted.WhileSubscribed(),
- BluetoothUtils.getPrimaryGroupIdForBroadcast(contentResolver))
+ BluetoothCsipSetCoordinator.GROUP_ID_INVALID
+ )
override val secondaryGroupId: StateFlow<Int> =
merge(
- btManager.profileManager.leAudioBroadcastAssistantProfile
- ?.onSourceConnectedOrRemoved
- ?.map { getSecondaryGroupId() } ?: emptyFlow(),
- btManager.eventManager.onProfileConnectionStateChanged
- .filter { profileConnection ->
- profileConnection.state == BluetoothAdapter.STATE_DISCONNECTED &&
+ isAudioSharingProfilesReady.flatMapLatest { ready ->
+ if (ready) {
+ btManager.profileManager.leAudioBroadcastAssistantProfile
+ .onSourceConnectedOrRemoved
+ .map { getSecondaryGroupId() }
+ } else {
+ emptyFlow()
+ }
+ },
+ btManager.eventManager.onProfileConnectionStateChanged
+ .filter { profileConnection ->
+ profileConnection.state == BluetoothAdapter.STATE_DISCONNECTED &&
profileConnection.bluetoothProfile ==
- BluetoothProfile.LE_AUDIO_BROADCAST_ASSISTANT
- }
- .map { getSecondaryGroupId() },
- primaryGroupId.map { getSecondaryGroupId() })
+ BluetoothProfile.LE_AUDIO_BROADCAST_ASSISTANT
+ }
+ .map { getSecondaryGroupId() },
+ primaryGroupId.map { getSecondaryGroupId() })
.onStart { emit(getSecondaryGroupId()) }
.flowOn(backgroundCoroutineContext)
- .stateIn(coroutineScope, SharingStarted.WhileSubscribed(), getSecondaryGroupId())
+ .stateIn(
+ coroutineScope,
+ SharingStarted.WhileSubscribed(),
+ BluetoothCsipSetCoordinator.GROUP_ID_INVALID
+ )
override val volumeMap: StateFlow<GroupIdToVolumes> =
- (btManager.profileManager.volumeControlProfile?.let { volumeControl ->
- inAudioSharing.flatMapLatest { isSharing ->
- if (isSharing) {
- callbackFlow {
- val callback =
- object : BluetoothVolumeControl.Callback {
- override fun onDeviceVolumeChanged(
- device: BluetoothDevice,
- @IntRange(
- from = AUDIO_SHARING_VOLUME_MIN.toLong(),
- to = AUDIO_SHARING_VOLUME_MAX.toLong())
- volume: Int
- ) {
- launch { send(Pair(device, volume)) }
- }
- }
- // Once registered, we will receive the initial volume of all
- // connected BT devices on VolumeControlProfile via callbacks
- volumeControl.registerCallback(
- ConcurrentUtils.DIRECT_EXECUTOR, callback)
- awaitClose { volumeControl.unregisterCallback(callback) }
+ inAudioSharing.flatMapLatest { isSharing ->
+ if (isSharing) {
+ callbackFlow {
+ val callback =
+ object : BluetoothVolumeControl.Callback {
+ override fun onDeviceVolumeChanged(
+ device: BluetoothDevice,
+ @IntRange(
+ from = AUDIO_SHARING_VOLUME_MIN.toLong(),
+ to = AUDIO_SHARING_VOLUME_MAX.toLong()
+ )
+ volume: Int
+ ) {
+ launch { send(Pair(device, volume)) }
}
- .runningFold(emptyMap<Int, Int>()) { acc, value ->
- val groupId =
- BluetoothUtils.getGroupId(
- btManager.cachedDeviceManager.findDevice(value.first))
- if (groupId != BluetoothCsipSetCoordinator.GROUP_ID_INVALID) {
- acc + Pair(groupId, value.second)
- } else {
- acc
- }
- }
- .flowOn(backgroundCoroutineContext)
- } else {
- emptyFlow()
+ }
+ // Once registered, we will receive the initial volume of all
+ // connected BT devices on VolumeControlProfile via callbacks
+ btManager.profileManager.volumeControlProfile.registerCallback(
+ ConcurrentUtils.DIRECT_EXECUTOR, callback
+ )
+ awaitClose {
+ btManager.profileManager.volumeControlProfile.unregisterCallback(
+ callback
+ )
}
}
- } ?: emptyFlow())
+ .runningFold(emptyMap<Int, Int>()) { acc, value ->
+ val groupId =
+ BluetoothUtils.getGroupId(
+ btManager.cachedDeviceManager.findDevice(value.first)
+ )
+ if (groupId != BluetoothCsipSetCoordinator.GROUP_ID_INVALID) {
+ acc + Pair(groupId, value.second)
+ } else {
+ acc
+ }
+ }
+ .flowOn(backgroundCoroutineContext)
+ } else {
+ emptyFlow()
+ }
+ }
.stateIn(coroutineScope, SharingStarted.WhileSubscribed(), emptyMap())
override suspend fun setSecondaryVolume(
@@ -196,12 +225,25 @@
}
}
+ private fun isBroadcastProfileReady(): Boolean =
+ btManager.profileManager.leAudioBroadcastProfile?.isProfileReady ?: false
+
+ private fun isAssistantProfileReady(): Boolean =
+ btManager.profileManager.leAudioBroadcastAssistantProfile?.isProfileReady ?: false
+
+ private fun isVolumeControlProfileReady(): Boolean =
+ btManager.profileManager.volumeControlProfile?.isProfileReady ?: false
+
+ private fun isAudioSharingProfilesReady(): Boolean =
+ isBroadcastProfileReady() && isAssistantProfileReady() && isVolumeControlProfileReady()
+
private fun isBroadcasting(): Boolean =
btManager.profileManager.leAudioBroadcastProfile?.isEnabled(null) ?: false
private fun getSecondaryGroupId(): Int =
BluetoothUtils.getGroupId(
- BluetoothUtils.getSecondaryDeviceForBroadcast(contentResolver, btManager))
+ BluetoothUtils.getSecondaryDeviceForBroadcast(contentResolver, btManager)
+ )
}
class AudioSharingRepositoryEmptyImpl : AudioSharingRepository {
@@ -215,5 +257,6 @@
override suspend fun setSecondaryVolume(
@IntRange(from = AUDIO_SHARING_VOLUME_MIN.toLong(), to = AUDIO_SHARING_VOLUME_MAX.toLong())
volume: Int
- ) {}
+ ) {
+ }
}
diff --git a/packages/SettingsLib/tests/integ/src/com/android/settingslib/volume/data/repository/AudioSharingRepositoryTest.kt b/packages/SettingsLib/tests/integ/src/com/android/settingslib/volume/data/repository/AudioSharingRepositoryTest.kt
index c54a2e4..078f0c8 100644
--- a/packages/SettingsLib/tests/integ/src/com/android/settingslib/volume/data/repository/AudioSharingRepositoryTest.kt
+++ b/packages/SettingsLib/tests/integ/src/com/android/settingslib/volume/data/repository/AudioSharingRepositoryTest.kt
@@ -57,6 +57,7 @@
import org.mockito.ArgumentMatchers.eq
import org.mockito.Captor
import org.mockito.Mock
+import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.mockito.Spy
@@ -145,8 +146,11 @@
}
@Test
- fun audioSharingStateChange_emitValues() {
+ fun audioSharingStateChange_profileReady_emitValues() {
testScope.runTest {
+ `when`(broadcast.isProfileReady).thenReturn(true)
+ `when`(assistant.isProfileReady).thenReturn(true)
+ `when`(volumeControl.isProfileReady).thenReturn(true)
val states = mutableListOf<Boolean?>()
underTest.inAudioSharing.onEach { states.add(it) }.launchIn(backgroundScope)
runCurrent()
@@ -155,7 +159,19 @@
triggerAudioSharingStateChange(TriggerType.BROADCAST_START, broadcastStarted)
runCurrent()
- Truth.assertThat(states).containsExactly(true, false, true)
+ Truth.assertThat(states).containsExactly(false, true, false, true)
+ }
+ }
+
+ @Test
+ fun audioSharingStateChange_profileNotReady_broadcastCallbackNotRegistered() {
+ testScope.runTest {
+ val states = mutableListOf<Boolean?>()
+ underTest.inAudioSharing.onEach { states.add(it) }.launchIn(backgroundScope)
+ runCurrent()
+ verify(broadcast, never()).registerServiceCallBack(any(), any())
+
+ Truth.assertThat(states).containsExactly(false)
}
}
@@ -176,11 +192,24 @@
}
@Test
- fun secondaryGroupIdChange_emitValues() {
+ fun secondaryGroupIdChange_profileNotReady_assistantCallbackNotRegistered() {
testScope.runTest {
val groupIds = mutableListOf<Int?>()
underTest.secondaryGroupId.onEach { groupIds.add(it) }.launchIn(backgroundScope)
runCurrent()
+ verify(assistant, never()).registerServiceCallBack(any(), any())
+ }
+ }
+
+ @Test
+ fun secondaryGroupIdChange_profileReady_emitValues() {
+ testScope.runTest {
+ `when`(broadcast.isProfileReady).thenReturn(true)
+ `when`(assistant.isProfileReady).thenReturn(true)
+ `when`(volumeControl.isProfileReady).thenReturn(true)
+ val groupIds = mutableListOf<Int?>()
+ underTest.secondaryGroupId.onEach { groupIds.add(it) }.launchIn(backgroundScope)
+ runCurrent()
triggerSourceAdded()
runCurrent()
triggerContentObserverChange()
@@ -211,8 +240,11 @@
}
@Test
- fun volumeMapChange_emitValues() {
+ fun volumeMapChange_profileReady_emitValues() {
testScope.runTest {
+ `when`(broadcast.isProfileReady).thenReturn(true)
+ `when`(assistant.isProfileReady).thenReturn(true)
+ `when`(volumeControl.isProfileReady).thenReturn(true)
val volumeMaps = mutableListOf<GroupIdToVolumes?>()
underTest.volumeMap.onEach { volumeMaps.add(it) }.launchIn(backgroundScope)
runCurrent()
@@ -234,6 +266,16 @@
}
@Test
+ fun volumeMapChange_profileNotReady_volumeControlCallbackNotRegistered() {
+ testScope.runTest {
+ val volumeMaps = mutableListOf<GroupIdToVolumes?>()
+ underTest.volumeMap.onEach { volumeMaps.add(it) }.launchIn(backgroundScope)
+ runCurrent()
+ verify(volumeControl, never()).registerCallback(any(), any())
+ }
+ }
+
+ @Test
fun setSecondaryVolume_setValue() {
testScope.runTest {
Settings.Secure.putInt(
@@ -258,6 +300,7 @@
`when`(broadcast.isEnabled(null)).thenReturn(true)
broadcastCallbackCaptor.value.broadcastAction()
}
+
TriggerType.BROADCAST_STOP -> {
`when`(broadcast.isEnabled(null)).thenReturn(false)
broadcastCallbackCaptor.value.broadcastAction()