From 9969334647b4b9439f549c0c0b2543fb3dec8813 Mon Sep 17 00:00:00 2001 From: Chaohui Wang Date: Thu, 26 Sep 2024 12:00:47 +0800 Subject: [PATCH] Fix Can't Able to Click Sims The root cause is SubscriptionManager.OnSubscriptionsChangedListener .onSubscriptionsChanged() not invoked in some cases. Even the SubscriptionManager.addOnSubscriptionsChangedListener's doc says the onSubscriptionsChanged() method will also be invoked once initially when calling it, there still case that the onSubscriptionsChanged() method is not invoked initially. For example, when the onSubscriptionsChanged event never happens before, on a device never ever has any subscriptions. Adding a .onStart { emit(Unit) } to fix. Also make the subscriptionsChangedFlow() a shared flow to mitigate the extra emit cost. Bug: 369276595 Flag: EXEMPT bug fix Test: manual - factory reset & no any sim Test: atest SubscriptionRepositoryTest Change-Id: Ic32a5666f14373926b5dfedb5dedadb4369acfc7 --- .../telephony/SubscriptionRepository.kt | 70 +++++++++++++++---- .../MobileNetworkSwitchControllerTest.kt | 5 ++ .../telephony/SubscriptionRepositoryTest.kt | 18 ++++- 3 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/com/android/settings/network/telephony/SubscriptionRepository.kt b/src/com/android/settings/network/telephony/SubscriptionRepository.kt index 6b5b4cb4d43..43bba0759a2 100644 --- a/src/com/android/settings/network/telephony/SubscriptionRepository.kt +++ b/src/com/android/settings/network/telephony/SubscriptionRepository.kt @@ -23,11 +23,13 @@ import android.util.Log import androidx.lifecycle.LifecycleOwner import com.android.settings.network.SubscriptionUtil import com.android.settingslib.spa.framework.util.collectLatestWithLifecycle +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.asExecutor import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.conflate import kotlinx.coroutines.flow.distinctUntilChanged @@ -36,6 +38,8 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart +import kotlinx.coroutines.flow.shareIn private const val TAG = "SubscriptionRepository" @@ -132,20 +136,7 @@ class SubscriptionRepository(private val context: Context) { fun canDisablePhysicalSubscription() = subscriptionManager.canDisablePhysicalSubscription() /** Flow for subscriptions changes. */ - fun subscriptionsChangedFlow() = callbackFlow { - val listener = object : SubscriptionManager.OnSubscriptionsChangedListener() { - override fun onSubscriptionsChanged() { - trySend(Unit) - } - } - - subscriptionManager.addOnSubscriptionsChangedListener( - Dispatchers.Default.asExecutor(), - listener, - ) - - awaitClose { subscriptionManager.removeOnSubscriptionsChangedListener(listener) } - }.conflate().onEach { Log.d(TAG, "subscriptions changed") }.flowOn(Dispatchers.Default) + fun subscriptionsChangedFlow() = getSharedSubscriptionsChangedFlow(context) /** Flow of active subscription ids. */ fun activeSubscriptionIdListFlow(): Flow> = @@ -172,6 +163,57 @@ class SubscriptionRepository(private val context: Context) { flowOf(null) } } + + companion object { + private lateinit var SharedSubscriptionsChangedFlow: Flow + + private fun getSharedSubscriptionsChangedFlow(context: Context): Flow { + if (!this::SharedSubscriptionsChangedFlow.isInitialized) { + SharedSubscriptionsChangedFlow = + context.applicationContext + .requireSubscriptionManager() + .subscriptionsChangedFlow() + .shareIn( + scope = CoroutineScope(Dispatchers.Default), + started = SharingStarted.WhileSubscribed(), + replay = 1, + ) + } + return SharedSubscriptionsChangedFlow + } + + /** + * Flow for subscriptions changes. + * + * Note: Even the SubscriptionManager.addOnSubscriptionsChangedListener's doc says the + * SubscriptionManager.OnSubscriptionsChangedListener.onSubscriptionsChanged() method will + * also be invoked once initially when calling it, there still case that the + * onSubscriptionsChanged() method is not invoked initially. For example, when the + * onSubscriptionsChanged event never happens before, on a device never ever has any + * subscriptions. + */ + private fun SubscriptionManager.subscriptionsChangedFlow() = + callbackFlow { + val listener = + object : SubscriptionManager.OnSubscriptionsChangedListener() { + override fun onSubscriptionsChanged() { + trySend(Unit) + } + + override fun onAddListenerFailed() { + close() + } + } + + addOnSubscriptionsChangedListener(Dispatchers.Default.asExecutor(), listener) + + awaitClose { removeOnSubscriptionsChangedListener(listener) } + } + .onStart { emit(Unit) } // Ensure this flow is never empty + .conflate() + .onEach { Log.d(TAG, "subscriptions changed") } + .flowOn(Dispatchers.Default) + } } val Context.subscriptionManager: SubscriptionManager? diff --git a/tests/spa_unit/src/com/android/settings/network/telephony/MobileNetworkSwitchControllerTest.kt b/tests/spa_unit/src/com/android/settings/network/telephony/MobileNetworkSwitchControllerTest.kt index ca370829b6f..f47c6354498 100644 --- a/tests/spa_unit/src/com/android/settings/network/telephony/MobileNetworkSwitchControllerTest.kt +++ b/tests/spa_unit/src/com/android/settings/network/telephony/MobileNetworkSwitchControllerTest.kt @@ -62,10 +62,15 @@ class MobileNetworkSwitchControllerTest { on { isSubscriptionEnabledFlow(SUB_ID) } doReturn flowOf(false) } + private val mockSubscriptionActivationRepository = mock { + on { isActivationChangeableFlow() } doReturn flowOf(true) + } + private val controller = MobileNetworkSwitchController( context = context, preferenceKey = TEST_KEY, subscriptionRepository = mockSubscriptionRepository, + subscriptionActivationRepository = mockSubscriptionActivationRepository, ).apply { init(SUB_ID) } @Test diff --git a/tests/spa_unit/src/com/android/settings/network/telephony/SubscriptionRepositoryTest.kt b/tests/spa_unit/src/com/android/settings/network/telephony/SubscriptionRepositoryTest.kt index 5052f57c588..ba5142e0eb0 100644 --- a/tests/spa_unit/src/com/android/settings/network/telephony/SubscriptionRepositoryTest.kt +++ b/tests/spa_unit/src/com/android/settings/network/telephony/SubscriptionRepositoryTest.kt @@ -91,7 +91,23 @@ class SubscriptionRepositoryTest { subInfoListener?.onSubscriptionsChanged() - assertThat(listDeferred.await()).hasSize(2) + assertThat(listDeferred.await().size).isAtLeast(2) + } + + @Test + fun subscriptionsChangedFlow_managerNotCallOnSubscriptionsChangedInitially() = runBlocking { + mockSubscriptionManager.stub { + on { addOnSubscriptionsChangedListener(any(), any()) } doAnswer + { + subInfoListener = + it.arguments[1] as SubscriptionManager.OnSubscriptionsChangedListener + // not call onSubscriptionsChanged here + } + } + + val initialValue = repository.subscriptionsChangedFlow().firstWithTimeoutOrNull() + + assertThat(initialValue).isSameInstanceAs(Unit) } @Test