Prefer the VCN subscription when filtering for CBRS

This logic only matters in cases where there are exactly 2 subscriptions
with the same `groupUuid` and at least one of them is opportunistic. In
those cases, SystemUI was preferring to track only the subscriptionId
associated with the `activeDataSubscriptionId` (per telephony) and
filter out the other subscription.

With this change, we now track the `vcnSubId` of the default network.
`ConnectivityRepository` exposes the subscription ID of the current
cellular VCN if it exists. In the case where one of the opportunistic
networks is a VCN, we will prefer _that_ network rather than the current
active data subscriptionId.

The case that whis becomes relevant is any time the active data
subscriptionId differs from the subscription id reported by the
VcnTransportInfo. This case should only happen when telephony tries to
switch the active data subscription and finds it not validatable. What
happens then is that the VCN is still connected and we will continue to
show the subscription for that connection rather than the active, which
should switch back.

Test: ConnectivityRepositoryImplTest
Test: MobileIconsInteractorTest
Bug: 222699760
Change-Id: I486ee0c323556407441de36284a1c22d74f3b836

6 files changed, 271 insertions, 39 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
index eec91a0bca82..d42878c06bd7 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt
@@ -155,7 +155,8 @@ constructor(
         combine(
                 unfilteredSubscriptions,
                 mobileConnectionsRepo.activeMobileDataSubscriptionId,
-            ) { unfilteredSubs, activeId ->
+                connectivityRepository.vcnSubId,
+            ) { unfilteredSubs, activeId, vcnSubId ->
                 // Based on the old logic,
                 if (unfilteredSubs.size != 2) {
                     return@combine unfilteredSubs
@@ -182,7 +183,13 @@ constructor(
                     // return the non-opportunistic info
                     return@combine if (info1.isOpportunistic) listOf(info2) else listOf(info1)
                 } else {
-                    return@combine if (info1.subscriptionId == activeId) {
+                    // It's possible for the subId of the VCN to disagree with the active subId in
+                    // cases where the system has tried to switch but found no connection. In these
+                    // scenarios, VCN will always have the subId that we want to use, so use that
+                    // value instead of the activeId reported by telephony
+                    val subIdToKeep = vcnSubId ?: activeId
+
+                    return@combine if (info1.subscriptionId == subIdToKeep) {
                         listOf(info1)
                     } else {
                         listOf(info2)
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityInputLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityInputLogger.kt
index 82492babba46..f05073860c41 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityInputLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityInputLogger.kt
@@ -61,6 +61,10 @@ constructor(
             model::messagePrinter,
         )
     }
+
+    fun logVcnSubscriptionId(subId: Int) {
+        buffer.log(TAG, LogLevel.DEBUG, { int1 = subId }, { "vcnSubId changed: $int1" })
+    }
 }
 
 private const val TAG = "ConnectivityInputLogger"
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepository.kt
index 731f1e028470..7076f345df97 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepository.kt
@@ -27,6 +27,7 @@ import android.net.NetworkCapabilities.TRANSPORT_ETHERNET
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.vcn.VcnTransportInfo
 import android.net.wifi.WifiInfo
+import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID
 import androidx.annotation.ArrayRes
 import androidx.annotation.VisibleForTesting
 import com.android.systemui.Dumpable
@@ -50,10 +51,13 @@ import java.io.PrintWriter
 import javax.inject.Inject
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.channels.awaitClose
+import kotlinx.coroutines.flow.SharedFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.flow.stateIn
 
 /**
@@ -66,6 +70,16 @@ interface ConnectivityRepository {
 
     /** Observable for which connection(s) are currently default. */
     val defaultConnections: StateFlow<DefaultConnectionModel>
+
+    /**
+     * Subscription ID of the [VcnTransportInfo] for the default connection.
+     *
+     * If the default network has a [VcnTransportInfo], then that transport info contains a subId of
+     * the VCN. When VCN is connected and default, this subId is what SystemUI will care about. In
+     * cases where telephony's activeDataSubscriptionId differs from this value, it is expected to
+     * eventually catch up and reflect what is represented here in the VcnTransportInfo.
+     */
+    val vcnSubId: StateFlow<Int?>
 }
 
 @SuppressLint("MissingPermission")
@@ -118,24 +132,13 @@ constructor(
                 initialValue = defaultHiddenIcons
             )
 
-    @SuppressLint("MissingPermission")
-    override val defaultConnections: StateFlow<DefaultConnectionModel> =
+    private val defaultNetworkCapabilities: SharedFlow<NetworkCapabilities?> =
         conflatedCallbackFlow {
                 val callback =
                     object : ConnectivityManager.NetworkCallback(FLAG_INCLUDE_LOCATION_INFO) {
                         override fun onLost(network: Network) {
                             logger.logOnDefaultLost(network)
-                            // The system no longer has a default network, so everything is
-                            // non-default.
-                            trySend(
-                                DefaultConnectionModel(
-                                    Wifi(isDefault = false),
-                                    Mobile(isDefault = false),
-                                    CarrierMerged(isDefault = false),
-                                    Ethernet(isDefault = false),
-                                    isValidated = false,
-                                )
-                            )
+                            trySend(null)
                         }
 
                         override fun onCapabilitiesChanged(
@@ -143,30 +146,7 @@ constructor(
                             networkCapabilities: NetworkCapabilities,
                         ) {
                             logger.logOnDefaultCapabilitiesChanged(network, networkCapabilities)
-
-                            val wifiInfo =
-                                networkCapabilities.getMainOrUnderlyingWifiInfo(connectivityManager)
-
-                            val isWifiDefault =
-                                networkCapabilities.hasTransport(TRANSPORT_WIFI) || wifiInfo != null
-                            val isMobileDefault =
-                                networkCapabilities.hasTransport(TRANSPORT_CELLULAR)
-                            val isCarrierMergedDefault = wifiInfo?.isCarrierMerged == true
-                            val isEthernetDefault =
-                                networkCapabilities.hasTransport(TRANSPORT_ETHERNET)
-
-                            val isValidated =
-                                networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)
-
-                            trySend(
-                                DefaultConnectionModel(
-                                    Wifi(isWifiDefault),
-                                    Mobile(isMobileDefault),
-                                    CarrierMerged(isCarrierMergedDefault),
-                                    Ethernet(isEthernetDefault),
-                                    isValidated,
-                                )
-                            )
+                            trySend(networkCapabilities)
                         }
                     }
 
@@ -174,6 +154,61 @@ constructor(
 
                 awaitClose { connectivityManager.unregisterNetworkCallback(callback) }
             }
+            .shareIn(scope, SharingStarted.WhileSubscribed())
+
+    override val vcnSubId: StateFlow<Int?> =
+        defaultNetworkCapabilities
+            .map { networkCapabilities ->
+                networkCapabilities?.run {
+                    val subId = (transportInfo as? VcnTransportInfo)?.subId
+                    // Never return an INVALID_SUBSCRIPTION_ID (-1)
+                    if (subId != INVALID_SUBSCRIPTION_ID) {
+                        subId
+                    } else {
+                        null
+                    }
+                }
+            }
+            .distinctUntilChanged()
+            /* A note for logging: we use -2 here since -1 == INVALID_SUBSCRIPTION_ID */
+            .onEach { logger.logVcnSubscriptionId(it ?: -2) }
+            .stateIn(scope, SharingStarted.Eagerly, null)
+
+    @SuppressLint("MissingPermission")
+    override val defaultConnections: StateFlow<DefaultConnectionModel> =
+        defaultNetworkCapabilities
+            .map { networkCapabilities ->
+                if (networkCapabilities == null) {
+                    // The system no longer has a default network, so everything is
+                    // non-default.
+                    DefaultConnectionModel(
+                        Wifi(isDefault = false),
+                        Mobile(isDefault = false),
+                        CarrierMerged(isDefault = false),
+                        Ethernet(isDefault = false),
+                        isValidated = false,
+                    )
+                } else {
+                    val wifiInfo =
+                        networkCapabilities.getMainOrUnderlyingWifiInfo(connectivityManager)
+
+                    val isWifiDefault =
+                        networkCapabilities.hasTransport(TRANSPORT_WIFI) || wifiInfo != null
+                    val isMobileDefault = networkCapabilities.hasTransport(TRANSPORT_CELLULAR)
+                    val isCarrierMergedDefault = wifiInfo?.isCarrierMerged == true
+                    val isEthernetDefault = networkCapabilities.hasTransport(TRANSPORT_ETHERNET)
+
+                    val isValidated = networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)
+
+                    DefaultConnectionModel(
+                        Wifi(isWifiDefault),
+                        Mobile(isMobileDefault),
+                        CarrierMerged(isCarrierMergedDefault),
+                        Ethernet(isEthernetDefault),
+                        isValidated,
+                    )
+                }
+            }
             .distinctUntilChanged()
             .onEach { logger.logDefaultConnectionsChanged(it) }
             .stateIn(scope, SharingStarted.Eagerly, DefaultConnectionModel())
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
index 6e1ab58db56d..16104beb002c 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt
@@ -272,6 +272,52 @@ class MobileIconsInteractorTest : SysuiTestCase() {
         }
 
     @Test
+    fun filteredSubscriptions_vcnSubId_agreesWithActiveSubId_usesActiveAkaVcnSub() =
+        testScope.runTest {
+            val (sub1, sub3) =
+                createSubscriptionPair(
+                    subscriptionIds = Pair(SUB_1_ID, SUB_3_ID),
+                    opportunistic = Pair(true, true),
+                    grouped = true,
+                )
+            connectionsRepository.setSubscriptions(listOf(sub1, sub3))
+            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID)
+            connectivityRepository.vcnSubId.value = SUB_3_ID
+            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)
+                .thenReturn(false)
+
+            var latest: List<SubscriptionModel>? = null
+            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)
+
+            assertThat(latest).isEqualTo(listOf(sub3))
+
+            job.cancel()
+        }
+
+    @Test
+    fun filteredSubscriptions_vcnSubId_disagreesWithActiveSubId_usesVcnSub() =
+        testScope.runTest {
+            val (sub1, sub3) =
+                createSubscriptionPair(
+                    subscriptionIds = Pair(SUB_1_ID, SUB_3_ID),
+                    opportunistic = Pair(true, true),
+                    grouped = true,
+                )
+            connectionsRepository.setSubscriptions(listOf(sub1, sub3))
+            connectionsRepository.setActiveMobileDataSubscriptionId(SUB_3_ID)
+            connectivityRepository.vcnSubId.value = SUB_1_ID
+            whenever(carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault)
+                .thenReturn(false)
+
+            var latest: List<SubscriptionModel>? = null
+            val job = underTest.filteredSubscriptions.onEach { latest = it }.launchIn(this)
+
+            assertThat(latest).isEqualTo(listOf(sub1))
+
+            job.cancel()
+        }
+
+    @Test
     fun activeDataConnection_turnedOn() =
         testScope.runTest {
             CONNECTION_1.setDataEnabled(true)
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepositoryImplTest.kt
index 661002d275b0..fa4e91b68a5e 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepositoryImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/ConnectivityRepositoryImplTest.kt
@@ -24,6 +24,7 @@ import android.net.NetworkCapabilities.TRANSPORT_ETHERNET
 import android.net.NetworkCapabilities.TRANSPORT_WIFI
 import android.net.vcn.VcnTransportInfo
 import android.net.wifi.WifiInfo
+import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID
 import androidx.test.filters.SmallTest
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.dump.DumpManager
@@ -37,6 +38,7 @@ import com.android.systemui.statusbar.pipeline.shared.data.repository.Connectivi
 import com.android.systemui.tuner.TunerService
 import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.argumentCaptor
+import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.mock
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -655,6 +657,139 @@ class ConnectivityRepositoryImplTest : SysuiTestCase() {
         }
 
     @Test
+    fun vcnSubId_initiallyNull() {
+        assertThat(underTest.vcnSubId.value).isNull()
+    }
+
+    @Test
+    fun vcnSubId_tracksVcnTransportInfo() =
+        testScope.runTest {
+            val vcnInfo = VcnTransportInfo(SUB_1_ID)
+
+            var latest: Int? = null
+            val job = underTest.vcnSubId.onEach { latest = it }.launchIn(this)
+
+            val capabilities =
+                mock<NetworkCapabilities>().also {
+                    whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true)
+                    whenever(it.transportInfo).thenReturn(vcnInfo)
+                }
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isEqualTo(SUB_1_ID)
+            job.cancel()
+        }
+
+    @Test
+    fun vcnSubId_filersOutInvalid() =
+        testScope.runTest {
+            val vcnInfo = VcnTransportInfo(INVALID_SUBSCRIPTION_ID)
+
+            var latest: Int? = null
+            val job = underTest.vcnSubId.onEach { latest = it }.launchIn(this)
+
+            val capabilities =
+                mock<NetworkCapabilities>().also {
+                    whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true)
+                    whenever(it.transportInfo).thenReturn(vcnInfo)
+                }
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isNull()
+            job.cancel()
+        }
+
+    @Test
+    fun vcnSubId_nullIfNoTransportInfo() =
+        testScope.runTest {
+            var latest: Int? = null
+            val job = underTest.vcnSubId.onEach { latest = it }.launchIn(this)
+
+            val capabilities =
+                mock<NetworkCapabilities>().also {
+                    whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true)
+                    whenever(it.transportInfo).thenReturn(null)
+                }
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isNull()
+            job.cancel()
+        }
+
+    @Test
+    fun vcnSubId_nullIfVcnInfoIsNotCellular() =
+        testScope.runTest {
+            // If the underlying network of the VCN is a WiFi network, then there is no subId that
+            // could disagree with telephony's active data subscription id.
+
+            var latest: Int? = null
+            val job = underTest.vcnSubId.onEach { latest = it }.launchIn(this)
+
+            val wifiInfo = mock<WifiInfo>()
+            val vcnInfo = VcnTransportInfo(wifiInfo)
+            val capabilities =
+                mock<NetworkCapabilities>().also {
+                    whenever(it.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true)
+                    whenever(it.transportInfo).thenReturn(vcnInfo)
+                }
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isNull()
+            job.cancel()
+        }
+
+    @Test
+    fun vcnSubId_changingVcnInfoIsTracked() =
+        testScope.runTest {
+            var latest: Int? = null
+            val job = underTest.vcnSubId.onEach { latest = it }.launchIn(this)
+
+            val wifiInfo = mock<WifiInfo>()
+            val wifiVcnInfo = VcnTransportInfo(wifiInfo)
+            val sub1VcnInfo = VcnTransportInfo(SUB_1_ID)
+            val sub2VcnInfo = VcnTransportInfo(SUB_2_ID)
+
+            val capabilities =
+                mock<NetworkCapabilities>().also {
+                    whenever(it.hasTransport(TRANSPORT_WIFI)).thenReturn(true)
+                    whenever(it.transportInfo).thenReturn(wifiVcnInfo)
+                }
+
+            // WIFI VCN info
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isNull()
+
+            // Cellular VCN info with subId 1
+            whenever(capabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(true)
+            whenever(capabilities.transportInfo).thenReturn(sub1VcnInfo)
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isEqualTo(SUB_1_ID)
+
+            // Cellular VCN info with subId 2
+            whenever(capabilities.transportInfo).thenReturn(sub2VcnInfo)
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isEqualTo(SUB_2_ID)
+
+            // No VCN anymore
+            whenever(capabilities.transportInfo).thenReturn(null)
+
+            getDefaultNetworkCallback().onCapabilitiesChanged(NETWORK, capabilities)
+
+            assertThat(latest).isNull()
+
+            job.cancel()
+        }
+
+    @Test
     fun getMainOrUnderlyingWifiInfo_wifi_hasInfo() {
         val wifiInfo = mock<WifiInfo>()
         val capabilities =
@@ -964,6 +1099,9 @@ class ConnectivityRepositoryImplTest : SysuiTestCase() {
         private const val SLOT_WIFI = "wifi"
         private const val SLOT_MOBILE = "mobile"
 
+        private const val SUB_1_ID = 1
+        private const val SUB_2_ID = 2
+
         const val NETWORK_ID = 45
         val NETWORK = mock<Network>().apply { whenever(this.getNetId()).thenReturn(NETWORK_ID) }
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/FakeConnectivityRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/FakeConnectivityRepository.kt
index 9e825b704851..8f28cc003d67 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/FakeConnectivityRepository.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/data/repository/FakeConnectivityRepository.kt
@@ -30,6 +30,8 @@ class FakeConnectivityRepository : ConnectivityRepository {
     override val defaultConnections: StateFlow<DefaultConnectionModel> =
         MutableStateFlow(DefaultConnectionModel())
 
+    override val vcnSubId: MutableStateFlow<Int?> = MutableStateFlow(null)
+
     fun setForceHiddenIcons(hiddenIcons: Set<ConnectivitySlot>) {
         _forceHiddenIcons.value = hiddenIcons
     }