Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa287c4e authored by Yiyi Shen's avatar Yiyi Shen
Browse files

[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
parent 409d712e

packages/SettingsLib/src/com/android/settingslib/bluetooth/LocalBluetoothProfileManagerCallbackExt.kt

0 → 100644
+43 −0
Original line number Diff line number Diff line 
/*

 * 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

packages/SettingsLib/src/com/android/settingslib/volume/data/repository/AudioSharingRepository.kt

+99 −56
Original line number Diff line number Diff line
@@ -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

        isAudioSharingProfilesReady.flatMapLatest { ready ->

            if (ready) {

                btManager.profileManager.leAudioBroadcastProfile.onBroadcastStartedOrStopped

                    .map { isBroadcasting() }

                    .onStart { emit(isBroadcasting()) }

                    .flowOn(backgroundCoroutineContext)

        } ?: flowOf(false)

            } 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,13 +133,20 @@ class AudioSharingRepositoryImpl( 
            .stateIn(

                coroutineScope,

                SharingStarted.WhileSubscribed(),

                BluetoothUtils.getPrimaryGroupIdForBroadcast(contentResolver))

                BluetoothCsipSetCoordinator.GROUP_ID_INVALID

            )



    override val secondaryGroupId: StateFlow<Int> =

        merge(

            isAudioSharingProfilesReady.flatMapLatest { ready ->

                if (ready) {

                    btManager.profileManager.leAudioBroadcastAssistantProfile

                    ?.onSourceConnectedOrRemoved

                    ?.map { getSecondaryGroupId() } ?: emptyFlow(),

                        .onSourceConnectedOrRemoved

                        .map { getSecondaryGroupId() }

                } else {

                    emptyFlow()

                }

            },

            btManager.eventManager.onProfileConnectionStateChanged

                .filter { profileConnection ->

                    profileConnection.state == BluetoothAdapter.STATE_DISCONNECTED &&
@@ -137,10 +157,13 @@ class AudioSharingRepositoryImpl( 
            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 {
@@ -150,7 +173,8 @@ class AudioSharingRepositoryImpl( 
                                device: BluetoothDevice,

                                @IntRange(

                                    from = AUDIO_SHARING_VOLUME_MIN.toLong(),

                                                to = AUDIO_SHARING_VOLUME_MAX.toLong())

                                    to = AUDIO_SHARING_VOLUME_MAX.toLong()

                                )

                                volume: Int

                            ) {

                                launch { send(Pair(device, volume)) }
@@ -158,14 +182,20 @@ class AudioSharingRepositoryImpl( 
                        }

                    // 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) }

                    btManager.profileManager.volumeControlProfile.registerCallback(

                        ConcurrentUtils.DIRECT_EXECUTOR, callback

                    )

                    awaitClose {

                        btManager.profileManager.volumeControlProfile.unregisterCallback(

                            callback

                        )

                    }

                }

                    .runningFold(emptyMap<Int, Int>()) { acc, value ->

                        val groupId =

                            BluetoothUtils.getGroupId(

                                        btManager.cachedDeviceManager.findDevice(value.first))

                                btManager.cachedDeviceManager.findDevice(value.first)

                            )

                        if (groupId != BluetoothCsipSetCoordinator.GROUP_ID_INVALID) {

                            acc + Pair(groupId, value.second)

                        } else {
@@ -177,7 +207,6 @@ class AudioSharingRepositoryImpl( 
                emptyFlow()

            }

        }

            } ?: 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

    ) {}

    ) {

    }

}

packages/SettingsLib/tests/integ/src/com/android/settingslib/volume/data/repository/AudioSharingRepositoryTest.kt

+47 −4
Original line number Diff line number Diff line
@@ -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()
@@ -233,6 +265,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 {
@@ -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()