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

Commit 6114e164 authored by Caitlin Shkuratov's avatar Caitlin Shkuratov
Browse files

[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
parent 5f1c1da0

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/prod/WifiRepositoryImpl.kt

+14 −3
Original line number Diff line number Diff line
@@ -130,7 +130,8 @@ constructor( 
                            // 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 @@ constructor( 
    }



    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 @@ constructor( 
        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

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiNetworkModel.kt

+9 −8
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import com.android.systemui.log.table.Diffable 
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 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
     */

    object Unavailable : WifiNetworkModel() {

        override fun toString() = "WifiNetwork.Unavailable"



        override fun logDiffs(prevVal: WifiNetworkModel, row: TableRowLogger) {

            if (prevVal is Unavailable) {

                return
@@ -67,6 +69,7 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
        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 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
    ) : 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 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
    ) : 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 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { 
        }



        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

    }



    /**