Merge "[Audiosharing] Fix race condition for profile ready and flow collect" into main

3 files changed, 189 insertions, 60 deletions

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 000000000000..b7338fca54ff
--- /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 99d5891818b6..7a66335ef22f 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.BluetoothUtils
 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 @@ class AudioSharingRepositoryImpl(
     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 @@ class AudioSharingRepositoryImpl(
         contentResolver.registerContentObserver(
             Settings.Secure.getUriFor(BluetoothUtils.getPrimaryGroupIdUriForBroadcast()),
             false,
-            callback)
+            callback
+        )
         awaitClose { contentResolver.unregisterContentObserver(callback) }
     }
 
@@ -120,64 +133,80 @@ class AudioSharingRepositoryImpl(
             .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 @@ class AudioSharingRepositoryImpl(
         }
     }
 
+    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 @@ class AudioSharingRepositoryEmptyImpl : AudioSharingRepository {
     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 c54a2e4d479b..078f0c8adba5 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.any
 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 @@ class AudioSharingRepositoryTest {
     }
 
     @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 @@ class AudioSharingRepositoryTest {
             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,8 +192,21 @@ class AudioSharingRepositoryTest {
     }
 
     @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()
@@ -211,8 +240,11 @@ class AudioSharingRepositoryTest {
     }
 
     @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 @@ class AudioSharingRepositoryTest {
     }
 
     @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 @@ class AudioSharingRepositoryTest {
                 `when`(broadcast.isEnabled(null)).thenReturn(true)
                 broadcastCallbackCaptor.value.broadcastAction()
             }
+
             TriggerType.BROADCAST_STOP -> {
                 `when`(broadcast.isEnabled(null)).thenReturn(false)
                 broadcastCallbackCaptor.value.broadcastAction()