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

Commit 65e73e30 authored by Evan Laird's avatar Evan Laird
Browse files

[Mobile] Wrap AccessibilityContentDescriptions in a static getter 

The content descriptions are mapped to level by their position in the
PHONE_SIGNAL_STRENGTH array, and thus are susceptible to out of bounds
index exceptions.

This change adds accessor methods to AccessibilityContentDescriptions
so that they can be checked (it's the responsibility of the caller to
check against a 0 return value which will throw if trying to get the
resource).

We also make the content description nullable and handle the resid=0
case.

Test: MobileIconViewModelTest
Bug: 328547883
Flag: NONE
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:6e67f7d49206325e9f0fd0189b8b3a1c5a2094a8)
Merged-In: Ifa55ad8184a5337390d9fa245311b4e9513c0233
Change-Id: Ifa55ad8184a5337390d9fa245311b4e9513c0233
parent afe50c04

packages/SettingsLib/res/values/strings.xml

+2 −0
Original line number Diff line number Diff line
@@ -1645,6 +1645,8 @@ 
    <string name="accessibility_phone_two_bars">Phone two bars.</string>

    <!-- Content description of the phone signal when it is three bars for accessibility (not shown on the screen). [CHAR LIMIT=NONE] -->

    <string name="accessibility_phone_three_bars">Phone three bars.</string>

    <!-- Content description of the phone signal when it is four bars for accessibility (not shown on the screen). [CHAR LIMIT=NONE] -->

    <string name="accessibility_phone_four_bars">Phone four bars.</string>

    <!-- Content description of the phone signal when it is full for accessibility (not shown on the screen). [CHAR LIMIT=NONE] -->

    <string name="accessibility_phone_signal_full">Phone signal full.</string>

packages/SettingsLib/src/com/android/settingslib/AccessibilityContentDescriptions.java

+53 −0
Original line number Diff line number Diff line
@@ -33,6 +33,59 @@ public class AccessibilityContentDescriptions { 
        R.string.accessibility_phone_signal_full

    };



    /**

     * @param level int in range [0-4] that describes the signal level

     * @return the appropriate content description for that signal strength, or 0 if the param is

     *         invalid

     */

    public static int getDescriptionForLevel(int level) {

        if (level > 4 || level < 0) {

            return 0;

        }



        return PHONE_SIGNAL_STRENGTH[level];

    }



    public static final int[] PHONE_SIGNAL_STRENGTH_INFLATED = {

            PHONE_SIGNAL_STRENGTH_NONE,

            R.string.accessibility_phone_one_bar,

            R.string.accessibility_phone_two_bars,

            R.string.accessibility_phone_three_bars,

            R.string.accessibility_phone_four_bars,

            R.string.accessibility_phone_signal_full

    };



    /**

     * @param level int in range [0-5] that describes the inflated signal level

     * @return the appropriate content description for that signal strength, or 0 if the param is

     *         invalid

     */

    public static int getDescriptionForInflatedLevel(int level) {

        if (level > 5 || level < 0) {

            return 0;

        }



        return PHONE_SIGNAL_STRENGTH_INFLATED[level];

    }



    /**

     * @param level int in range [0-5] that describes the inflated signal level

     * @param numberOfLevels one of (5, 6) that describes the default number of levels, or the

     *                       inflated number of levels. The level param should be relative to the

     *                       number of levels. This won't do any inflation.

     * @return the appropriate content description for that signal strength, or 0 if the param is

     *         invalid

     */

    public static int getDescriptionForLevel(int level, int numberOfLevels) {

        if (numberOfLevels == 5) {

            return getDescriptionForLevel(level);

        } else if (numberOfLevels == 6) {

            return getDescriptionForInflatedLevel(level);

        } else {

            return 0;

        }

    }



    public static final int[] DATA_CONNECTION_STRENGTH = {

        R.string.accessibility_no_data,

        R.string.accessibility_data_one_bar,

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt

+22 −8
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ 


package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel



import com.android.settingslib.AccessibilityContentDescriptions.PHONE_SIGNAL_STRENGTH

import com.android.settingslib.AccessibilityContentDescriptions

import com.android.systemui.Flags.statusBarStaticInoutIndicators

import com.android.systemui.common.shared.model.ContentDescription

import com.android.systemui.common.shared.model.Icon
@@ -50,7 +50,7 @@ interface MobileIconViewModelCommon { 
    /** True if this view should be visible at all. */

    val isVisible: StateFlow<Boolean>

    val icon: Flow<SignalIconModel>

    val contentDescription: Flow<ContentDescription>

    val contentDescription: Flow<ContentDescription?>

    val roaming: Flow<Boolean>

    /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */

    val networkTypeIcon: Flow<Icon.Resource?>
@@ -123,7 +123,7 @@ class MobileIconViewModel( 


    override val icon: Flow<SignalIconModel> = vmProvider.flatMapLatest { it.icon }



    override val contentDescription: Flow<ContentDescription> =

    override val contentDescription: Flow<ContentDescription?> =

        vmProvider.flatMapLatest { it.contentDescription }



    override val roaming: Flow<Boolean> = vmProvider.flatMapLatest { it.roaming }
@@ -206,12 +206,26 @@ private class CellularIconViewModel( 


    override val icon: Flow<SignalIconModel> = iconInteractor.signalLevelIcon



    override val contentDescription: Flow<ContentDescription> = run {

        val initial = ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[0])

    override val contentDescription: Flow<ContentDescription?> =

        iconInteractor.signalLevelIcon

            .map { ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[it.level]) }

            .stateIn(scope, SharingStarted.WhileSubscribed(), initial)

            .map {

                // We expect the signal icon to be cellular here since this is the cellular vm

                if (it !is SignalIconModel.Cellular) {

                    null

                } else {

                    val resId =

                        AccessibilityContentDescriptions.getDescriptionForLevel(

                            it.level,

                            it.numberOfLevels

                        )

                    if (resId != 0) {

                        ContentDescription.Resource(resId)

                    } else {

                        null

                    }

                }

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), null)



    private val showNetworkTypeIcon: Flow<Boolean> =

        combine(

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt

+34 −0
Original line number Diff line number Diff line
@@ -54,6 +54,7 @@ import com.android.systemui.statusbar.policy.data.repository.FakeUserSetupReposi 
import com.android.systemui.util.CarrierConfigTracker

import com.android.systemui.util.mockito.whenever

import com.google.common.truth.Truth.assertThat

import com.google.common.truth.Truth.assertWithMessage

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.flow.filterIsInstance

import kotlinx.coroutines.flow.launchIn
@@ -278,6 +279,39 @@ class MobileIconViewModelTest : SysuiTestCase() { 
            job.cancel()

        }



    @Test

    fun contentDescription_nonInflated_invalidLevelIsNull() =

        testScope.runTest {

            val latest by collectLastValue(underTest.contentDescription)



            repository.setAllLevels(-1)

            assertThat(latest).isNull()



            repository.setAllLevels(100)

            assertThat(latest).isNull()

        }



    @Test

    fun contentDescription_nonInflated_testABunchOfLevelsForNull() =

        testScope.runTest {

            val latest by collectLastValue(underTest.contentDescription)



            repository.numberOfLevels.value = 5



            // -1 and 5 are out of the bounds for non-inflated content descriptions

            for (i in -1..5) {

                repository.setAllLevels(i)

                when (i) {

                    -1,

                    5 -> assertWithMessage("Level $i is expected to be null").that(latest).isNull()

                    else ->

                        assertWithMessage("Level $i is expected not to be null")

                            .that(latest)

                            .isNotNull()

                }

            }

        }



    @Test

    fun networkType_dataEnabled_groupIsRepresented() =

        testScope.runTest {