From 18614852377e2a131d740af9be1a86b6b7945594 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Thu, 30 Mar 2023 12:17:26 -0400 Subject: [PATCH] [Sb refactor] Ensure we fetch default data subId on flow start The old pipeline aggressively polls SubscriptionManager for the `defaultDataSubId`. We don't want to do that since it's going to cause IPC on the main thread, and it's not necessary. This change enforces that we poll SubscriptionManager.getDefaultDataSubscrpitionId() when the flow is started, so that we always know the current value when collectors show up. Further updates are still expected to come in through the broadcast. Test: MobileConnectionsRepositoryTest Bug: 270315194 Change-Id: I74aa6b9b456ca497b02de48d4c932a2b4814c97a --- .../dagger/StatusBarPipelineModule.kt | 10 ++++- .../prod/MobileConnectionsRepositoryImpl.kt | 14 +++---- .../mobile/util/SubscriptionManagerProxy.kt | 30 +++++++++++++ .../MobileRepositorySwitcherTest.kt | 3 ++ .../prod/MobileConnectionsRepositoryTest.kt | 42 +++++++++++++++++++ .../util/FakeSubscriptionManagerProxy.kt | 27 ++++++++++++ 6 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/util/SubscriptionManagerProxy.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/util/FakeSubscriptionManagerProxy.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt index adfea80715a2d..eaa145582ba30 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt @@ -37,6 +37,8 @@ import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIc import com.android.systemui.statusbar.pipeline.mobile.ui.MobileUiAdapter import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxyImpl +import com.android.systemui.statusbar.pipeline.mobile.util.SubscriptionManagerProxy +import com.android.systemui.statusbar.pipeline.mobile.util.SubscriptionManagerProxyImpl import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepositoryImpl import com.android.systemui.statusbar.pipeline.wifi.data.repository.RealWifiRepository @@ -65,8 +67,7 @@ abstract class StatusBarPipelineModule { @Binds abstract fun wifiRepository(impl: WifiRepositorySwitcher): WifiRepository - @Binds - abstract fun wifiInteractor(impl: WifiInteractorImpl): WifiInteractor + @Binds abstract fun wifiInteractor(impl: WifiInteractorImpl): WifiInteractor @Binds abstract fun mobileConnectionsRepository( @@ -77,6 +78,11 @@ abstract class StatusBarPipelineModule { @Binds abstract fun mobileMappingsProxy(impl: MobileMappingsProxyImpl): MobileMappingsProxy + @Binds + abstract fun subscriptionManagerProxy( + impl: SubscriptionManagerProxyImpl + ): SubscriptionManagerProxy + @Binds abstract fun mobileIconsInteractor(impl: MobileIconsInteractorImpl): MobileIconsInteractor diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt index 8c93bf7c21985..45d50c1039097 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt @@ -44,6 +44,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameMode import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy +import com.android.systemui.statusbar.pipeline.mobile.util.SubscriptionManagerProxy import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiNetworkModel @@ -65,6 +66,7 @@ import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.withContext @@ -76,6 +78,7 @@ class MobileConnectionsRepositoryImpl constructor( connectivityRepository: ConnectivityRepository, private val subscriptionManager: SubscriptionManager, + private val subscriptionManagerProxy: SubscriptionManagerProxy, private val telephonyManager: TelephonyManager, private val logger: MobileInputLogger, @MobileSummaryLog private val tableLogger: TableLogBuffer, @@ -195,7 +198,7 @@ constructor( override val defaultDataSubId: StateFlow = broadcastDispatcher .broadcastFlow( - IntentFilter(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED) + IntentFilter(TelephonyManager.ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED), ) { intent, _ -> intent.getIntExtra(PhoneConstants.SUBSCRIPTION_KEY, INVALID_SUBSCRIPTION_ID) } @@ -204,14 +207,11 @@ constructor( tableLogger, LOGGING_PREFIX, columnName = "defaultSubId", - initialValue = SubscriptionManager.getDefaultDataSubscriptionId(), + initialValue = INVALID_SUBSCRIPTION_ID, ) + .onStart { emit(subscriptionManagerProxy.getDefaultDataSubscriptionId()) } .onEach { defaultDataSubIdChangeEvent.tryEmit(Unit) } - .stateIn( - scope, - SharingStarted.WhileSubscribed(), - SubscriptionManager.getDefaultDataSubscriptionId() - ) + .stateIn(scope, SharingStarted.WhileSubscribed(), INVALID_SUBSCRIPTION_ID) private val carrierConfigChangedEvent = broadcastDispatcher diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/util/SubscriptionManagerProxy.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/util/SubscriptionManagerProxy.kt new file mode 100644 index 0000000000000..22d048343bc9e --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/util/SubscriptionManagerProxy.kt @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2023 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.systemui.statusbar.pipeline.mobile.util + +import android.telephony.SubscriptionManager +import javax.inject.Inject + +interface SubscriptionManagerProxy { + fun getDefaultDataSubscriptionId(): Int +} + +/** Injectable proxy class for [SubscriptionManager]'s static methods */ +class SubscriptionManagerProxyImpl @Inject constructor() : SubscriptionManagerProxy { + /** The system default data subscription id, or INVALID_SUBSCRIPTION_ID on error */ + override fun getDefaultDataSubscriptionId() = SubscriptionManager.getDefaultDataSubscriptionId() +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt index 0b20285323079..1fdcf7f27dbfe 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt @@ -34,6 +34,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.model import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.validMobileEvent import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileConnectionsRepositoryImpl import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy +import com.android.systemui.statusbar.pipeline.mobile.util.FakeSubscriptionManagerProxy import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository @@ -89,6 +90,7 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { private val fakeNetworkEventsFlow = MutableStateFlow(null) private val mobileMappings = FakeMobileMappingsProxy() + private val subscriptionManagerProxy = FakeSubscriptionManagerProxy() private val scope = CoroutineScope(IMMEDIATE) @@ -117,6 +119,7 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { MobileConnectionsRepositoryImpl( connectivityRepository, subscriptionManager, + subscriptionManagerProxy, telephonyManager, logger, summaryLogger, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt index d65277f37ec48..ddff17aef2dea 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt @@ -47,6 +47,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierCon import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.FullMobileConnectionRepository.Factory.Companion.tableBufferLogName import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy +import com.android.systemui.statusbar.pipeline.mobile.util.FakeSubscriptionManagerProxy import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlots import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepositoryImpl @@ -98,6 +99,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { @Mock private lateinit var logBufferFactory: TableLogBufferFactory private val mobileMappings = FakeMobileMappingsProxy() + private val subscriptionManagerProxy = FakeSubscriptionManagerProxy() private val scope = CoroutineScope(IMMEDIATE) @@ -179,6 +181,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { MobileConnectionsRepositoryImpl( connectivityRepository, subscriptionManager, + subscriptionManagerProxy, telephonyManager, logger, summaryLogger, @@ -662,6 +665,8 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { var latest: Int? = null val job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + assertThat(latest).isEqualTo(INVALID_SUBSCRIPTION_ID) + fakeBroadcastDispatcher.registeredReceivers.forEach { receiver -> receiver.onReceive( context, @@ -685,6 +690,42 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { job.cancel() } + @Test + fun defaultDataSubId_fetchesInitialValueOnStart() = + runBlocking(IMMEDIATE) { + subscriptionManagerProxy.defaultDataSubId = 2 + var latest: Int? = null + val job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + + assertThat(latest).isEqualTo(2) + + job.cancel() + } + + @Test + fun defaultDataSubId_fetchesCurrentOnRestart() = + runBlocking(IMMEDIATE) { + subscriptionManagerProxy.defaultDataSubId = 2 + var latest: Int? = null + var job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + + assertThat(latest).isEqualTo(2) + + job.cancel() + + // Collectors go away but come back later + + latest = null + + subscriptionManagerProxy.defaultDataSubId = 1 + + job = underTest.defaultDataSubId.onEach { latest = it }.launchIn(this) + + assertThat(latest).isEqualTo(1) + + job.cancel() + } + @Test fun mobileIsDefault_startsAsFalse() { assertThat(underTest.mobileIsDefault.value).isFalse() @@ -902,6 +943,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { MobileConnectionsRepositoryImpl( connectivityRepository, subscriptionManager, + subscriptionManagerProxy, telephonyManager, logger, summaryLogger, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/util/FakeSubscriptionManagerProxy.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/util/FakeSubscriptionManagerProxy.kt new file mode 100644 index 0000000000000..3dc7de6884464 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/util/FakeSubscriptionManagerProxy.kt @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2023 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.systemui.statusbar.pipeline.mobile.util + +import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID + +/** Fake of [SubscriptionManagerProxy] for easy testing */ +class FakeSubscriptionManagerProxy( + /** Set the default data subId to be returned in [getDefaultDataSubscriptionId] */ + var defaultDataSubId: Int = INVALID_SUBSCRIPTION_ID +) : SubscriptionManagerProxy { + override fun getDefaultDataSubscriptionId(): Int = defaultDataSubId +}