Merge "[Sb refactor] address code comments" into tm-qpr-dev am: 44141cf13e am: 47203c5d6b

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21990125

Change-Id: I9d09c80d8c4cdfd389ec19873c3a4f5ff41d78d5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

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

@@ -105,10 +105,14 @@ class MobileConnectionRepositoryImpl(
      * The reason we need to do this is because TelephonyManager limits the number of registered
      * listeners per-process, so we don't want to create a new listener for every callback.
      *
-     * A note on the design for back pressure here: We use the [coalesce] operator here to change
-     * the backpressure strategy to store exactly the last callback event of _each type_ here, as
-     * opposed to the default strategy which is to drop the oldest event (regardless of type). This
-     * means that we should never miss any single event as long as the flow has been started.
+     * A note on the design for back pressure here: We don't control _which_ telephony callback
+     * comes in first, since we register every relevant bit of information as a batch. E.g., if a
+     * downstream starts collecting on a field which is backed by
+     * [TelephonyCallback.ServiceStateListener], it's not possible for us to guarantee that _that_
+     * callback comes in -- the first callback could very well be
+     * [TelephonyCallback.DataActivityListener], which would promptly be dropped if we didn't keep
+     * it tracked. We use the [scan] operator here to track the most recent callback of _each type_
+     * here. See [TelephonyCallbackState] to see how the callbacks are stored.
      */
     private val callbackEvents: StateFlow<TelephonyCallbackState> = run {
         val initial = TelephonyCallbackState()

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

@@ -25,7 +25,6 @@ import android.telephony.ServiceState.STATE_OUT_OF_SERVICE
 import android.telephony.TelephonyCallback
 import android.telephony.TelephonyCallback.DataActivityListener
 import android.telephony.TelephonyCallback.ServiceStateListener
-import android.telephony.TelephonyDisplayInfo
 import android.telephony.TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_LTE_CA
 import android.telephony.TelephonyDisplayInfo.OVERRIDE_NETWORK_TYPE_NONE
 import android.telephony.TelephonyManager
@@ -68,6 +67,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.toNetworkNameMo
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionsRepository
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository.Companion.DEFAULT_NUM_LEVELS
 import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileTelephonyHelpers.signalStrength
+import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileTelephonyHelpers.telephonyDisplayInfo
 import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy
 import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel
 import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel
@@ -392,13 +392,17 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
             val job = underTest.resolvedNetworkType.onEach { latest = it }.launchIn(this)
 
             val callback = getTelephonyCallbackForType<TelephonyCallback.DisplayInfoListener>()
-            val type = NETWORK_TYPE_UNKNOWN
-            val expected = UnknownNetworkType
-            val ti = mock<TelephonyDisplayInfo>().also { whenever(it.networkType).thenReturn(type) }
+            val ti =
+                telephonyDisplayInfo(
+                    networkType = NETWORK_TYPE_UNKNOWN,
+                    overrideNetworkType = NETWORK_TYPE_UNKNOWN,
+                )
+
             callback.onDisplayInfoChanged(ti)
 
+            val expected = UnknownNetworkType
             assertThat(latest).isEqualTo(expected)
-            assertThat(latest!!.lookupKey).isEqualTo(MobileMappings.toIconKey(type))
+            assertThat(latest!!.lookupKey).isEqualTo(MobileMappings.toIconKey(NETWORK_TYPE_UNKNOWN))
 
             job.cancel()
         }
@@ -412,14 +416,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
             val callback = getTelephonyCallbackForType<TelephonyCallback.DisplayInfoListener>()
             val overrideType = OVERRIDE_NETWORK_TYPE_NONE
             val type = NETWORK_TYPE_LTE
-            val expected = DefaultNetworkType(mobileMappings.toIconKey(type))
-            val ti =
-                mock<TelephonyDisplayInfo>().also {
-                    whenever(it.overrideNetworkType).thenReturn(overrideType)
-                    whenever(it.networkType).thenReturn(type)
-                }
+            val ti = telephonyDisplayInfo(networkType = type, overrideNetworkType = overrideType)
             callback.onDisplayInfoChanged(ti)
 
+            val expected = DefaultNetworkType(mobileMappings.toIconKey(type))
             assertThat(latest).isEqualTo(expected)
 
             job.cancel()
@@ -433,14 +433,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
 
             val callback = getTelephonyCallbackForType<TelephonyCallback.DisplayInfoListener>()
             val type = OVERRIDE_NETWORK_TYPE_LTE_CA
-            val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type))
-            val ti =
-                mock<TelephonyDisplayInfo>().also {
-                    whenever(it.networkType).thenReturn(type)
-                    whenever(it.overrideNetworkType).thenReturn(type)
-                }
+            val ti = telephonyDisplayInfo(networkType = type, overrideNetworkType = type)
             callback.onDisplayInfoChanged(ti)
 
+            val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type))
             assertThat(latest).isEqualTo(expected)
 
             job.cancel()
@@ -455,14 +451,10 @@ class MobileConnectionRepositoryTest : SysuiTestCase() {
             val callback = getTelephonyCallbackForType<TelephonyCallback.DisplayInfoListener>()
             val unknown = NETWORK_TYPE_UNKNOWN
             val type = OVERRIDE_NETWORK_TYPE_LTE_CA
-            val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type))
-            val ti =
-                mock<TelephonyDisplayInfo>().also {
-                    whenever(it.networkType).thenReturn(unknown)
-                    whenever(it.overrideNetworkType).thenReturn(type)
-                }
+            val ti = telephonyDisplayInfo(unknown, type)
             callback.onDisplayInfoChanged(ti)
 
+            val expected = OverrideNetworkType(mobileMappings.toIconKeyOverride(type))
             assertThat(latest).isEqualTo(expected)
 
             job.cancel()

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

@@ -64,10 +64,14 @@ import org.mockito.MockitoAnnotations
  *
  * Kind of like an interaction test case build just for [TelephonyCallback]
  *
- * The list of telephony callbacks we use is: [TelephonyCallback.CarrierNetworkListener]
- * [TelephonyCallback.DataActivityListener] [TelephonyCallback.DataConnectionStateListener]
- * [TelephonyCallback.DataEnabledListener] [TelephonyCallback.DisplayInfoListener]
- * [TelephonyCallback.ServiceStateListener] [TelephonyCallback.SignalStrengthsListener]
+ * The list of telephony callbacks we use is:
+ * - [TelephonyCallback.CarrierNetworkListener]
+ * - [TelephonyCallback.DataActivityListener]
+ * - [TelephonyCallback.DataConnectionStateListener]
+ * - [TelephonyCallback.DataEnabledListener]
+ * - [TelephonyCallback.DisplayInfoListener]
+ * - [TelephonyCallback.ServiceStateListener]
+ * - [TelephonyCallback.SignalStrengthsListener]
  *
  * Because each of these callbacks comes in on the same callbackFlow, collecting on a field backed
  * by only a single callback can immediately create backpressure on the other fields related to a
@@ -201,7 +205,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() {
                 200 /* unused */
             )
 
-            // Send a bunch of events that we don't care about, to overrun the replay buffer
             flipActivity(100, activityCallback)
 
             val connectionJob = underTest.dataConnectionState.onEach { latest = it }.launchIn(this)
@@ -225,7 +228,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() {
 
             enabledCallback.onDataEnabledChanged(true, 1 /* unused */)
 
-            // Send a bunch of events that we don't care about, to overrun the replay buffer
             flipActivity(100, activityCallback)
 
             val job = underTest.dataEnabled.onEach { latest = it }.launchIn(this)
@@ -252,7 +254,6 @@ class MobileConnectionTelephonySmokeTests : SysuiTestCase() {
             val ti = mock<TelephonyDisplayInfo>().also { whenever(it.networkType).thenReturn(type) }
             displayInfoCallback.onDisplayInfoChanged(ti)
 
-            // Send a bunch of events that we don't care about, to overrun the replay buffer
             flipActivity(100, activityCallback)
 
             val job = underTest.resolvedNetworkType.onEach { latest = it }.launchIn(this)

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

@@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod
 import android.telephony.CellSignalStrengthCdma
 import android.telephony.SignalStrength
 import android.telephony.TelephonyCallback
+import android.telephony.TelephonyDisplayInfo
 import android.telephony.TelephonyManager
 import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.argumentCaptor
@@ -48,6 +49,12 @@ object MobileTelephonyHelpers {
         return signalStrength
     }
 
+    fun telephonyDisplayInfo(networkType: Int, overrideNetworkType: Int) =
+        mock<TelephonyDisplayInfo>().also {
+            whenever(it.networkType).thenReturn(networkType)
+            whenever(it.overrideNetworkType).thenReturn(overrideNetworkType)
+        }
+
     inline fun <reified T> getTelephonyCallbackForType(mockTelephonyManager: TelephonyManager): T {
         val cbs = getTelephonyCallbacks(mockTelephonyManager).filterIsInstance<T>()
         assertThat(cbs.size).isEqualTo(1)