[SB][Wifi] Save WifiEntry.level to local variable to avoid crashes.
WifiEntry.level can be changed at any time, even mid-SysUI method.
WifiRepository checks the level's validity before creating a
WifiNetworkModel.Active instance, and WifiNetworkModel.Active *also*
checks the level's validity. This means that it's possible for the level
to be valid when the repository checks it, then become invalid before we
create WifiNetworkModel.Active, which then causes a crash.
This CL saves the level to a local variable so that it can't change
between the repo checking it and creating the model.
This CL also updates WifiNetworkModel's min & max level constants to
match WifiTrackerLib, now that we're fully migrated over.
Fixes: 362384551
Flag: EXEMPT bugfix
Test: atest WifiRepositoryImplTest WifiNetworkModelTest
Change-Id: Iefcadcf4b8ca76cf96ded27f599e42966f112c94
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt
index 885abca..5a10dfe 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt
@@ -130,7 +130,8 @@
// into our internal model immediately. [toWifiNetworkModel] always
// returns a new instance, so the flow is guaranteed to emit.
send(
- newPrimaryNetwork = connectedEntry?.toPrimaryWifiNetworkModel()
+ newPrimaryNetwork =
+ connectedEntry?.toPrimaryWifiNetworkModel()
?: WIFI_NETWORK_DEFAULT,
newSecondaryNetworks = secondaryNetworks,
newIsDefault = connectedEntry?.isDefaultNetwork ?: false,
@@ -270,7 +271,17 @@
}
private fun WifiEntry.convertNormalToModel(): WifiNetworkModel {
- if (this.level == WIFI_LEVEL_UNREACHABLE || this.level !in WIFI_LEVEL_MIN..WIFI_LEVEL_MAX) {
+ // WifiEntry instance values aren't guaranteed to be stable between method calls because
+ // WifiPickerTracker is continuously updating the same object. Save the level in a local
+ // variable so that checking the level validity here guarantees that the level will still be
+ // valid when we create the `WifiNetworkModel.Active` instance later. Otherwise, the level
+ // could be valid here but become invalid later, and `WifiNetworkModel.Active` will throw
+ // an exception. See b/362384551.
+ val currentLevel = this.level
+ if (
+ currentLevel == WIFI_LEVEL_UNREACHABLE ||
+ currentLevel !in WIFI_LEVEL_MIN..WIFI_LEVEL_MAX
+ ) {
// If our level means the network is unreachable or the level is otherwise invalid, we
// don't have an active network.
return WifiNetworkModel.Inactive
@@ -286,7 +297,7 @@
return WifiNetworkModel.Active(
networkId = NETWORK_ID,
isValidated = this.hasInternetAccess(),
- level = this.level,
+ level = currentLevel,
ssid = this.title,
hotspotDeviceType = hotspotDeviceType,
// With WifiTrackerLib, [WifiEntry.title] will appropriately fetch the SSID for
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt
index 7078a2e..03a6548 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt
@@ -24,6 +24,7 @@
import com.android.systemui.log.table.TableRowLogger
import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository
import com.android.wifitrackerlib.HotspotNetworkEntry.DeviceType
+import com.android.wifitrackerlib.WifiEntry
/** Provides information about the current wifi network. */
sealed class WifiNetworkModel : Diffable<WifiNetworkModel> {
@@ -38,6 +39,7 @@
*/
object Unavailable : WifiNetworkModel() {
override fun toString() = "WifiNetwork.Unavailable"
+
override fun logDiffs(prevVal: WifiNetworkModel, row: TableRowLogger) {
if (prevVal is Unavailable) {
return
@@ -67,6 +69,7 @@
val invalidReason: String,
) : WifiNetworkModel() {
override fun toString() = "WifiNetwork.Invalid[$invalidReason]"
+
override fun logDiffs(prevVal: WifiNetworkModel, row: TableRowLogger) {
if (prevVal !is Invalid) {
logFull(row)
@@ -154,7 +157,8 @@
) : WifiNetworkModel() {
init {
require(level in MIN_VALID_LEVEL..numberOfLevels) {
- "0 <= wifi level <= $numberOfLevels required; level was $level"
+ "CarrierMerged: $MIN_VALID_LEVEL <= wifi level <= $numberOfLevels required; " +
+ "level was $level"
}
require(subscriptionId != SubscriptionManager.INVALID_SUBSCRIPTION_ID) {
"subscription ID cannot be invalid"
@@ -232,7 +236,8 @@
) : WifiNetworkModel() {
init {
require(level in MIN_VALID_LEVEL..MAX_VALID_LEVEL) {
- "0 <= wifi level <= 4 required; level was $level"
+ "Active: $MIN_VALID_LEVEL <= wifi level <= $MAX_VALID_LEVEL required; " +
+ "level was $level"
}
}
@@ -314,16 +319,12 @@
}
companion object {
- // TODO(b/292534484): Use [com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_MAX] instead
- // once the migration to WifiTrackerLib is complete.
- @VisibleForTesting internal const val MAX_VALID_LEVEL = 4
+ @VisibleForTesting internal const val MAX_VALID_LEVEL = WifiEntry.WIFI_LEVEL_MAX
}
}
companion object {
- // TODO(b/292534484): Use [com.android.wifitrackerlib.WifiEntry.WIFI_LEVEL_MIN] instead
- // once the migration to WifiTrackerLib is complete.
- @VisibleForTesting internal const val MIN_VALID_LEVEL = 0
+ @VisibleForTesting internal const val MIN_VALID_LEVEL = WifiEntry.WIFI_LEVEL_MIN
}
/**