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

Commit 9f825a8b authored by Dave Mankoff's avatar Dave Mankoff
Browse files

Include all flags in our flag list. 

This improves the use of reflection to include all the flags listed
in Flags.kt. One interseting finding: I can't figure out how to
get the value of a field marked with just `@Keep`. It needs to
also have `@JvmField` or not have any annotations at all.

More changes to this code will come in follow up cls, using string
names where appropriate, but this improves existing functionality.

Key takeaways:
- Reflection is terrible
- Reflection in Kotlin is worse
- Reflection on `object` instances is worse still
- Reflection mixing Java with Kotlin follows the trend line
- Reflection on annotated fields is nigh impossible

Bug: 249121873
Test: adb shell cmd statusbar flag
Change-Id: I69a96501876b01570233c3fd80b6bc7334d9fe73
parent 3345f99f

packages/SystemUI/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -115,6 +115,7 @@ android_library { 
        "androidx-constraintlayout_constraintlayout",

        "androidx.exifinterface_exifinterface",

        "com.google.android.material_material",

        "kotlin-reflect",

        "kotlinx_coroutines_android",

        "kotlinx_coroutines",

        "iconloader_base",

packages/SystemUI/shared/src/com/android/systemui/flags/Flag.kt

+13 −13
Original line number Diff line number Diff line
@@ -89,7 +89,7 @@ abstract class BooleanFlag constructor( 
 *

 * It can be changed or overridden in debug builds but not in release builds.

 */

data class UnreleasedFlag @JvmOverloads constructor(

data class UnreleasedFlag constructor(

    override val id: Int,

    override val teamfood: Boolean = false,

    override val overridden: Boolean = false
@@ -100,7 +100,7 @@ data class UnreleasedFlag @JvmOverloads constructor( 
 *

 * It can be changed or overridden in any build, meaning it can be turned off if needed.

 */

data class ReleasedFlag @JvmOverloads constructor(

data class ReleasedFlag constructor(

    override val id: Int,

    override val teamfood: Boolean = false,

    override val overridden: Boolean = false
@@ -111,7 +111,7 @@ data class ReleasedFlag @JvmOverloads constructor( 
 *

 * Prefer [UnreleasedFlag] and [ReleasedFlag].

 */

data class ResourceBooleanFlag @JvmOverloads constructor(

data class ResourceBooleanFlag constructor(

    override val id: Int,

    @BoolRes override val resourceId: Int,

    override val teamfood: Boolean = false
@@ -124,7 +124,7 @@ data class ResourceBooleanFlag @JvmOverloads constructor( 
 *

 * Prefer [UnreleasedFlag] and [ReleasedFlag].

 */

data class DeviceConfigBooleanFlag @JvmOverloads constructor(

data class DeviceConfigBooleanFlag constructor(

    override val id: Int,

    override val name: String,

    override val namespace: String,
@@ -139,7 +139,7 @@ data class DeviceConfigBooleanFlag @JvmOverloads constructor( 
 *

 * Prefer [UnreleasedFlag] and [ReleasedFlag].

 */

data class SysPropBooleanFlag @JvmOverloads constructor(

data class SysPropBooleanFlag constructor(

    override val id: Int,

    override val name: String,

    override val default: Boolean = false
@@ -148,7 +148,7 @@ data class SysPropBooleanFlag @JvmOverloads constructor( 
    override val teamfood: Boolean = false

}



data class StringFlag @JvmOverloads constructor(

data class StringFlag constructor(

    override val id: Int,

    override val default: String = "",

    override val teamfood: Boolean = false,
@@ -173,13 +173,13 @@ data class StringFlag @JvmOverloads constructor( 
    }

}



data class ResourceStringFlag @JvmOverloads constructor(

data class ResourceStringFlag constructor(

    override val id: Int,

    @StringRes override val resourceId: Int,

    override val teamfood: Boolean = false

) : ResourceFlag<String>



data class IntFlag @JvmOverloads constructor(

data class IntFlag constructor(

    override val id: Int,

    override val default: Int = 0,

    override val teamfood: Boolean = false,
@@ -205,13 +205,13 @@ data class IntFlag @JvmOverloads constructor( 
    }

}



data class ResourceIntFlag @JvmOverloads constructor(

data class ResourceIntFlag constructor(

    override val id: Int,

    @IntegerRes override val resourceId: Int,

    override val teamfood: Boolean = false

) : ResourceFlag<Int>



data class LongFlag @JvmOverloads constructor(

data class LongFlag constructor(

    override val id: Int,

    override val default: Long = 0,

    override val teamfood: Boolean = false,
@@ -237,7 +237,7 @@ data class LongFlag @JvmOverloads constructor( 
    }

}



data class FloatFlag @JvmOverloads constructor(

data class FloatFlag constructor(

    override val id: Int,

    override val default: Float = 0f,

    override val teamfood: Boolean = false,
@@ -263,13 +263,13 @@ data class FloatFlag @JvmOverloads constructor( 
    }

}



data class ResourceFloatFlag @JvmOverloads constructor(

data class ResourceFloatFlag constructor(

    override val id: Int,

    override val resourceId: Int,

    override val teamfood: Boolean = false

) : ResourceFlag<Int>



data class DoubleFlag @JvmOverloads constructor(

data class DoubleFlag constructor(

    override val id: Int,

    override val default: Double = 0.0,

    override val teamfood: Boolean = false,

packages/SystemUI/src/com/android/systemui/flags/FlagCommand.java

+13 −26
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@ import androidx.annotation.NonNull; 
import com.android.systemui.statusbar.commandline.Command;



import java.io.PrintWriter;

import java.lang.reflect.Field;

import java.util.List;

import java.util.Map;
@@ -230,33 +229,22 @@ public class FlagCommand implements Command { 
    }



    private int flagNameToId(String flagName) {

        List<Field> fields = Flags.getFlagFields();

        for (Field field : fields) {

            if (flagName.equals(field.getName())) {

                return fieldToId(field);

        Map<String, Flag<?>> flagFields = Flags.getFlagFields();

        for (String fieldName : flagFields.keySet()) {

            if (flagName.equals(fieldName)) {

                return flagFields.get(fieldName).getId();

            }

        }



        return 0;

    }



    private int fieldToId(Field field) {

        try {

            Flag<?> flag = (Flag<?>) field.get(null);

            return flag.getId();

        } catch (IllegalAccessException e) {

            // no-op

        }



        return 0;

    }



    private void printKnownFlags(PrintWriter pw) {

        List<Field> fields = Flags.getFlagFields();

        Map<String, Flag<?>> fields = Flags.getFlagFields();



        int longestFieldName = 0;

        for (Field field : fields) {

            longestFieldName = Math.max(longestFieldName, field.getName().length());

        for (String fieldName : fields.keySet()) {

            longestFieldName = Math.max(longestFieldName, fieldName.length());

        }



        pw.println("Known Flags:");
@@ -268,16 +256,15 @@ public class FlagCommand implements Command { 
        for (int i = 0; i < longestFieldName; i++) {

            pw.print("=");

        }

        pw.println(" ==== =====");

        for (Field field : fields) {

            int id = fieldToId(field);

            Flag<?> flag = mAllFlags.get(id);



        pw.println(" ==== ========");

        for (String fieldName : fields.keySet()) {

            Flag<?> flag = fields.get(fieldName);

            int id = flag.getId();

            if (id == 0 || !mAllFlags.containsKey(id)) {

                continue;

            }

            pw.print(field.getName());

            int fieldWidth = field.getName().length();

            pw.print(fieldName);

            int fieldWidth = fieldName.length();

            for (int i = 0; i < longestFieldName - fieldWidth + 1; i++) {

                pw.print(" ");

            }

packages/SystemUI/src/com/android/systemui/flags/Flags.kt

+58 −28
Original line number Diff line number Diff line
@@ -17,8 +17,12 @@ package com.android.systemui.flags 


import android.provider.DeviceConfig

import com.android.internal.annotations.Keep

import com.android.internal.annotations.VisibleForTesting

import com.android.systemui.R

import java.lang.reflect.Field

import kotlin.reflect.KClass

import kotlin.reflect.full.declaredMembers

import kotlin.reflect.full.isSubclassOf

import kotlin.reflect.full.staticProperties



/**

 * List of [Flag] objects for use in SystemUI.
@@ -63,6 +67,7 @@ object Flags { 
    @JvmField val NOTIFICATION_DISMISSAL_FADE = UnreleasedFlag(113, teamfood = true)

    val STABILITY_INDEX_FIX = UnreleasedFlag(114, teamfood = true)

    val SEMI_STABLE_SORT = UnreleasedFlag(115, teamfood = true)



    @JvmField val NOTIFICATION_GROUP_CORNER = UnreleasedFlag(116, teamfood = true)

    // next id: 117
@@ -148,6 +153,7 @@ object Flags { 
    // TODO(b/254512321): Tracking Bug

    @JvmField val COMBINED_QS_HEADERS = UnreleasedFlag(501, teamfood = true)

    val PEOPLE_TILE = ResourceBooleanFlag(502, R.bool.flag_conversations)



    @JvmField

    val QS_USER_DETAIL_SHORTCUT =

        ResourceBooleanFlag(503, R.bool.flag_lockscreen_qs_user_detail_shortcut)
@@ -244,14 +250,14 @@ object Flags { 
    @JvmField val ROUNDED_BOX_RIPPLE = ReleasedFlag(1002)



    // 1100 - windowing

    @JvmField

    @Keep

    @JvmField

    val WM_ENABLE_SHELL_TRANSITIONS =

        SysPropBooleanFlag(1100, "persist.wm.debug.shell_transit", false)



    // TODO(b/254513207): Tracking Bug

    @JvmField

    @Keep

    @JvmField

    val WM_ENABLE_PARTIAL_SCREEN_SHARING =

        DeviceConfigBooleanFlag(

            1102,
@@ -262,30 +268,30 @@ object Flags { 
        )



    // TODO(b/254512674): Tracking Bug

    @JvmField

    @Keep

    @JvmField

    val HIDE_NAVBAR_WINDOW = SysPropBooleanFlag(1103, "persist.wm.debug.hide_navbar_window", false)



    @JvmField

    @Keep

    @JvmField

    val WM_DESKTOP_WINDOWING = SysPropBooleanFlag(1104, "persist.wm.debug.desktop_mode", false)



    @JvmField

    @Keep

    @JvmField

    val WM_CAPTION_ON_SHELL = SysPropBooleanFlag(1105, "persist.wm.debug.caption_on_shell", false)



    @JvmField

    @Keep

    @JvmField

    val ENABLE_FLING_TO_DISMISS_BUBBLE =

        SysPropBooleanFlag(1108, "persist.wm.debug.fling_to_dismiss_bubble", true)



    @JvmField

    @Keep

    @JvmField

    val ENABLE_FLING_TO_DISMISS_PIP =

        SysPropBooleanFlag(1109, "persist.wm.debug.fling_to_dismiss_pip", true)



    @JvmField

    @Keep

    @JvmField

    val ENABLE_PIP_KEEP_CLEAR_ALGORITHM =

        SysPropBooleanFlag(1110, "persist.wm.debug.enable_pip_keep_clear_algorithm", false)
@@ -293,18 +299,18 @@ object Flags { 
    @JvmField @Keep val WM_BUBBLE_BAR = UnreleasedFlag(1111)



    // 1200 - predictive back

    @JvmField

    @Keep

    @JvmField

    val WM_ENABLE_PREDICTIVE_BACK =

        SysPropBooleanFlag(1200, "persist.wm.debug.predictive_back", true)



    @JvmField

    @Keep

    @JvmField

    val WM_ENABLE_PREDICTIVE_BACK_ANIM =

        SysPropBooleanFlag(1201, "persist.wm.debug.predictive_back_anim", false)



    @JvmField

    @Keep

    @JvmField

    val WM_ALWAYS_ENFORCE_PREDICTIVE_BACK =

        SysPropBooleanFlag(1202, "persist.wm.debug.predictive_back_always_enforce", false)
@@ -313,7 +319,7 @@ object Flags { 


    // 1300 - screenshots

    // TODO(b/254512719): Tracking Bug

    @JvmField val SCREENSHOT_REQUEST_PROCESSOR = UnreleasedFlag(1300, true)

    @JvmField val SCREENSHOT_REQUEST_PROCESSOR = UnreleasedFlag(1300, teamfood = true)



    // TODO(b/254513155): Tracking Bug

    @JvmField val SCREENSHOT_WORK_PROFILE_POLICY = UnreleasedFlag(1301)
@@ -327,7 +333,7 @@ object Flags { 
    val CHOOSER_UNBUNDLED = UnreleasedFlag(1500, teamfood = true)



    // 1700 - clipboard

    @JvmField val CLIPBOARD_OVERLAY_REFACTOR = UnreleasedFlag(1700, true)

    @JvmField val CLIPBOARD_OVERLAY_REFACTOR = UnreleasedFlag(1700, teamfood = true)

    @JvmField val CLIPBOARD_REMOTE_BEHAVIOR = UnreleasedFlag(1701)



    // 1800 - shade container
@@ -337,7 +343,7 @@ object Flags { 
    @JvmField val NOTE_TASKS = SysPropBooleanFlag(1900, "persist.sysui.debug.note_tasks")



    // 2000 - device controls

    @Keep val USE_APP_PANELS = UnreleasedFlag(2000, true)

    @Keep @JvmField val USE_APP_PANELS = UnreleasedFlag(2000, teamfood = true)



    // 2100 - Falsing Manager

    @JvmField val FALSING_FOR_LONG_TAPS = ReleasedFlag(2100)
@@ -348,23 +354,47 @@ object Flags { 
    // |  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  |

    @JvmStatic

    fun collectFlags(): Map<Int, Flag<*>> {

        return flagFields

            .map { field ->

                // field[null] returns the current value of the field.

                // See java.lang.Field#get

                val flag = field[null] as Flag<*>

                flag.id to flag

            }

            .toMap()

        return flagFields.mapKeys { field -> field.value.id }

    }



    // |  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  |

    @JvmStatic

    val flagFields: List<Field>

        get() {

            return Flags::class.java.fields.filter { f ->

                Flag::class.java.isAssignableFrom(f.type)

    val flagFields: Map<String, Flag<*>>

        get() = collectFlagsInClass(Flags)



    @VisibleForTesting

    fun collectFlagsInClass(instance: Any): Map<String, Flag<*>> {

        val cls = instance::class

        val javaPropNames = cls.java.fields.map { it.name }

        val props = cls.declaredMembers

        val staticProps = cls.staticProperties

        val staticPropNames = staticProps.map { it.name }

        return props

            .mapNotNull { property ->

                if ((property.returnType.classifier as KClass<*>).isSubclassOf(Flag::class)) {

                    // Fields with @JvmStatic should be accessed via java mechanisms

                    if (javaPropNames.contains(property.name)) {

                        property.name to cls.java.getField(property.name)[null] as Flag<*>

                        // Fields with @Keep but not @JvmField. Don't do this.

                    } else if (staticPropNames.contains(property.name)) {

                        // The below code causes access violation exceptions. I don't know why.

                        // property.name to (property.call() as Flag<*>)

                        // property.name to (staticProps.find { it.name == property.name }!!

                        // .getter.call() as Flag<*>)

                        throw java.lang.RuntimeException(

                            "The {$property.name} flag needs @JvmField"

                        )

                        // Everything else. Skip the `get` prefixed fields that kotlin adds.

                    } else if (property.name.subSequence(0, 3) != "get") {

                        property.name to (property.call(instance) as Flag<*>)

                    } else {

                        null

                    }

                } else {

                    null

                }

            }

            .toMap()

    }

    // |                                                           |

    // \_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/\_/

packages/SystemUI/tests/src/com/android/systemui/flags/FlagsTest.java

deleted100644 → 0
+0 −119
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2021 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.systemui.flags;



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



import android.util.Pair;



import androidx.test.filters.SmallTest;



import com.android.systemui.SysuiTestCase;



import org.junit.Test;



import java.lang.reflect.Field;

import java.util.ArrayList;

import java.util.HashMap;

import java.util.List;

import java.util.Map;



@SmallTest

public class FlagsTest extends SysuiTestCase {



    @Test

    public void testDuplicateFlagIdCheckWorks() {

        List<Pair<String, Flag<?>>> flags = collectFlags(DuplicateFlagContainer.class);

        Map<Integer, List<String>> duplicates = groupDuplicateFlags(flags);



        assertWithMessage(generateAssertionMessage(duplicates))

                .that(duplicates.size()).isEqualTo(2);

    }



    @Test

    public void testNoDuplicateFlagIds() {

        List<Pair<String, Flag<?>>> flags = collectFlags(Flags.class);

        Map<Integer, List<String>> duplicates = groupDuplicateFlags(flags);



        assertWithMessage(generateAssertionMessage(duplicates))

                .that(duplicates.size()).isEqualTo(0);

    }



    private String generateAssertionMessage(Map<Integer, List<String>> duplicates) {

        StringBuilder stringBuilder = new StringBuilder();

        stringBuilder.append("Duplicate flag keys found: {");

        for (int id : duplicates.keySet()) {

            stringBuilder

                    .append(" ")

                    .append(id)

                    .append(": [")

                    .append(String.join(", ", duplicates.get(id)))

                    .append("]");

        }

        stringBuilder.append(" }");



        return stringBuilder.toString();

    }



    private List<Pair<String, Flag<?>>> collectFlags(Class<?> clz) {

        List<Pair<String, Flag<?>>> flags = new ArrayList<>();



        Field[] fields = clz.getFields();



        for (Field field : fields) {

            Class<?> t = field.getType();

            if (Flag.class.isAssignableFrom(t)) {

                try {

                    flags.add(Pair.create(field.getName(), (Flag<?>) field.get(null)));

                } catch (IllegalAccessException e) {

                    // no-op

                }

            }

        }



        return flags;

    }



    private Map<Integer, List<String>> groupDuplicateFlags(List<Pair<String, Flag<?>>> flags) {

        Map<Integer, List<String>> grouping = new HashMap<>();



        for (Pair<String, Flag<?>> flag : flags) {

            grouping.putIfAbsent(flag.second.getId(), new ArrayList<>());

            grouping.get(flag.second.getId()).add(flag.first);

        }



        Map<Integer, List<String>> result = new HashMap<>();

        for (Integer id : grouping.keySet()) {

            if (grouping.get(id).size() > 1) {

                result.put(id, grouping.get(id));

            }

        }



        return result;

    }



    private static class DuplicateFlagContainer {

        public static final BooleanFlag A_FLAG = new UnreleasedFlag(0);

        public static final BooleanFlag B_FLAG = new UnreleasedFlag(0);

        public static final StringFlag C_FLAG = new StringFlag(0);



        public static final BooleanFlag D_FLAG = new UnreleasedFlag(1);



        public static final DoubleFlag E_FLAG = new DoubleFlag(3);

        public static final DoubleFlag F_FLAG = new DoubleFlag(3);

    }

}