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

Commit 18614852 authored by Evan Laird's avatar Evan Laird
Browse files

[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
parent db1dbe4d

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt

+8 −2
Original line number Diff line number Diff line
@@ -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

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryImpl.kt

+7 −7
Original line number Diff line number Diff line
@@ -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<Int> =

        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

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/util/SubscriptionManagerProxy.kt

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

 * 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()

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt

+3 −0
Original line number Diff line number Diff line
@@ -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<FakeNetworkEventModel?>(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,

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionsRepositoryTest.kt

+42 −0
Original line number Diff line number Diff line
@@ -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,