Merge "Only restart SystemUI when Server Flags actually change." into tm-qpr-dev am: d7c9694969

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

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

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

@@ -85,9 +85,34 @@ public class FeatureFlagsDebug implements FeatureFlags {
     private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
             new ServerFlagReader.ChangeListener() {
                 @Override
-                public void onChange(Flag<?> flag) {
+                public void onChange(Flag<?> flag, String value) {
+                    boolean shouldRestart = false;
+                    if (mBooleanFlagCache.containsKey(flag.getName())) {
+                        boolean newValue = value == null ? false : Boolean.parseBoolean(value);
+                        if (mBooleanFlagCache.get(flag.getName()) != newValue) {
+                            shouldRestart = true;
+                        }
+                    } else if (mStringFlagCache.containsKey(flag.getName())) {
+                        String newValue = value == null ? "" : value;
+                        if (mStringFlagCache.get(flag.getName()) != value) {
+                            shouldRestart = true;
+                        }
+                    } else if (mIntFlagCache.containsKey(flag.getName())) {
+                        int newValue = 0;
+                        try {
+                            newValue = value == null ? 0 : Integer.parseInt(value);
+                        } catch (NumberFormatException e) {
+                        }
+                        if (mIntFlagCache.get(flag.getName()) != newValue) {
+                            shouldRestart = true;
+                        }
+                    }
+                    if (shouldRestart) {
                         mRestarter.restartSystemUI(
-                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+                                "Server flag change: " + flag.getNamespace() + "."
+                                        + flag.getName());
+
+                    }
                 }
             };
 

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

@@ -53,13 +53,38 @@ public class FeatureFlagsRelease implements FeatureFlags {
     private final Map<String, Flag<?>> mAllFlags;
     private final Map<String, Boolean> mBooleanCache = new HashMap<>();
     private final Map<String, String> mStringCache = new HashMap<>();
+    private final Map<String, Integer> mIntCache = new HashMap<>();
 
     private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
             new ServerFlagReader.ChangeListener() {
                 @Override
-                public void onChange(Flag<?> flag) {
+                public void onChange(Flag<?> flag, String value) {
+                    boolean shouldRestart = false;
+                    if (mBooleanCache.containsKey(flag.getName())) {
+                        boolean newValue = value == null ? false : Boolean.parseBoolean(value);
+                        if (mBooleanCache.get(flag.getName()) != newValue) {
+                            shouldRestart = true;
+                        }
+                    } else if (mStringCache.containsKey(flag.getName())) {
+                        String newValue = value == null ? "" : value;
+                        if (mStringCache.get(flag.getName()) != newValue) {
+                            shouldRestart = true;
+                        }
+                    } else if (mIntCache.containsKey(flag.getName())) {
+                        int newValue = 0;
+                        try {
+                            newValue = value == null ? 0 : Integer.parseInt(value);
+                        } catch (NumberFormatException e) {
+                        }
+                        if (mIntCache.get(flag.getName()) != newValue) {
+                            shouldRestart = true;
+                        }
+                    }
+                    if (shouldRestart) {
                         mRestarter.restartSystemUI(
-                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+                                "Server flag change: " + flag.getNamespace() + "."
+                                        + flag.getName());
+                    }
                 }
             };
 
@@ -97,68 +122,97 @@ public class FeatureFlagsRelease implements FeatureFlags {
 
     @Override
     public boolean isEnabled(@NotNull ReleasedFlag flag) {
-        return mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true);
+        // Fill the cache.
+        return isEnabledInternal(flag.getName(),
+                mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true));
     }
 
     @Override
     public boolean isEnabled(ResourceBooleanFlag flag) {
-        if (!mBooleanCache.containsKey(flag.getName())) {
+        // Fill the cache.
-            return isEnabled(flag.getName(), mResources.getBoolean(flag.getResourceId()));
+        return isEnabledInternal(flag.getName(), mResources.getBoolean(flag.getResourceId()));
-        }
-
-        return mBooleanCache.get(flag.getName());
     }
 
     @Override
     public boolean isEnabled(SysPropBooleanFlag flag) {
-        if (!mBooleanCache.containsKey(flag.getName())) {
+        // Fill the cache.
-            return isEnabled(
+        return isEnabledInternal(
                 flag.getName(),
                 mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
     }
 
-        return mBooleanCache.get(flag.getName());
+    /**
+     * Checks and fills the boolean cache. This is important, Always call through to this method!
+     *
+     * We use the cache as a way to decide if we need to restart the process when server-side
+     * changes occur.
+     */
+    private boolean isEnabledInternal(String name, boolean defaultValue) {
+        // Fill the cache.
+        if (!mBooleanCache.containsKey(name)) {
+            mBooleanCache.put(name, defaultValue);
         }
 
-    private boolean isEnabled(String name, boolean defaultValue) {
+        return mBooleanCache.get(name);
-        mBooleanCache.put(name, defaultValue);
-        return defaultValue;
     }
 
     @NonNull
     @Override
     public String getString(@NonNull StringFlag flag) {
-        return getString(flag.getName(), flag.getDefault());
+        // Fill the cache.
+        return getStringInternal(flag.getName(), flag.getDefault());
     }
 
     @NonNull
     @Override
     public String getString(@NonNull ResourceStringFlag flag) {
-        if (!mStringCache.containsKey(flag.getName())) {
+        // Fill the cache.
-            return getString(flag.getName(),
+        return getStringInternal(flag.getName(),
                 requireNonNull(mResources.getString(flag.getResourceId())));
     }
 
-        return mStringCache.get(flag.getName());
+    /**
+     * Checks and fills the String cache. This is important, Always call through to this method!
+     *
+     * We use the cache as a way to decide if we need to restart the process when server-side
+     * changes occur.
+     */
+    private String getStringInternal(String name, String defaultValue) {
+        if (!mStringCache.containsKey(name)) {
+            mStringCache.put(name, defaultValue);
         }
 
-    private String getString(String name, String defaultValue) {
+        return mStringCache.get(name);
-        mStringCache.put(name, defaultValue);
-        return defaultValue;
     }
 
     @NonNull
     @Override
     public int getInt(@NonNull IntFlag flag) {
-        return flag.getDefault();
+        // Fill the cache.
+        return getIntInternal(flag.getName(), flag.getDefault());
     }
 
     @NonNull
     @Override
     public int getInt(@NonNull ResourceIntFlag flag) {
+        // Fill the cache.
         return mResources.getInteger(flag.getResourceId());
     }
 
+    /**
+     * Checks and fills the integer cache. This is important, Always call through to this method!
+     *
+     * We use the cache as a way to decide if we need to restart the process when server-side
+     * changes occur.
+     */
+    private int getIntInternal(String name, int defaultValue) {
+        if (!mIntCache.containsKey(name)) {
+            mIntCache.put(name, defaultValue);
+        }
+
+        return mIntCache.get(name);
+    }
+
     @Override
     public void dump(@NonNull PrintWriter pw, @NonNull String[] args) {
         pw.println("can override: false");

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

@@ -37,7 +37,7 @@ interface ServerFlagReader {
     fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener)
 
     interface ChangeListener {
-        fun onChange(flag: Flag<*>)
+        fun onChange(flag: Flag<*>, value: String?)
     }
 }
 
@@ -67,7 +67,7 @@ class ServerFlagReaderImpl @Inject constructor(
                 propLoop@ for (propName in properties.keyset) {
                     for (flag in flags) {
                         if (propName == flag.name) {
-                            listener.onChange(flag)
+                            listener.onChange(flag, properties.getString(propName, null))
                             break@propLoop
                         }
                     }
@@ -144,7 +144,7 @@ class ServerFlagReaderFake : ServerFlagReader {
         for ((listener, flags) in listeners) {
             flagLoop@ for (flag in flags) {
                 if (name == flag.name) {
-                    listener.onChange(flag)
+                    listener.onChange(flag, if (value) "true" else "false")
                     break@flagLoop
                 }
             }
@@ -159,5 +159,6 @@ class ServerFlagReaderFake : ServerFlagReader {
         flags: Collection<Flag<*>>,
         listener: ServerFlagReader.ChangeListener
     ) {
+        listeners.add(Pair(listener, flags))
     }
 }

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt

@@ -37,6 +37,7 @@ import org.mockito.Mock
 import org.mockito.Mockito.anyBoolean
 import org.mockito.Mockito.anyString
 import org.mockito.Mockito.inOrder
+import org.mockito.Mockito.never
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.verifyNoMoreInteractions
@@ -53,7 +54,7 @@ import org.mockito.Mockito.`when` as whenever
  */
 @SmallTest
 class FeatureFlagsDebugTest : SysuiTestCase() {
-    private lateinit var mFeatureFlagsDebug: FeatureFlagsDebug
+    private lateinit var featureFlagsDebug: FeatureFlagsDebug
 
     @Mock
     private lateinit var flagManager: FlagManager
@@ -85,7 +86,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         flagMap.put(Flags.TEAMFOOD.name, Flags.TEAMFOOD)
         flagMap.put(teamfoodableFlagA.name, teamfoodableFlagA)
         flagMap.put(teamfoodableFlagB.name, teamfoodableFlagB)
-        mFeatureFlagsDebug = FeatureFlagsDebug(
+        featureFlagsDebug = FeatureFlagsDebug(
             flagManager,
             mockContext,
             globalSettings,
@@ -95,7 +96,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             flagMap,
             restarter
         )
-        mFeatureFlagsDebug.init()
+        featureFlagsDebug.init()
         verify(flagManager).onSettingsChangedAction = any()
         broadcastReceiver = withArgCaptor {
             verify(mockContext).registerReceiver(
@@ -116,7 +117,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         whenever(flagManager.readFlagValue<Boolean>(eq("4"), any())).thenReturn(false)
 
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 ReleasedFlag(
                     2,
                     name = "2",
@@ -125,7 +126,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             )
         ).isTrue()
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 UnreleasedFlag(
                     3,
                     name = "3",
@@ -134,7 +135,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             )
         ).isTrue()
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 ReleasedFlag(
                     4,
                     name = "4",
@@ -143,7 +144,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             )
         ).isFalse()
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 UnreleasedFlag(
                     5,
                     name = "5",
@@ -157,8 +158,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
     fun teamFoodFlag_False() {
         whenever(flagManager.readFlagValue<Boolean>(
             eq(Flags.TEAMFOOD.name), any())).thenReturn(false)
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
 
         // Regular boolean flags should still test the same.
         // Only our teamfoodableFlag should change.
@@ -169,8 +170,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
     fun teamFoodFlag_True() {
         whenever(flagManager.readFlagValue<Boolean>(
             eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
 
         // Regular boolean flags should still test the same.
         // Only our teamfoodableFlag should change.
@@ -185,8 +186,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             .thenReturn(false)
         whenever(flagManager.readFlagValue<Boolean>(
             eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
 
         // Regular boolean flags should still test the same.
         // Only our teamfoodableFlag should change.
@@ -205,7 +206,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         whenever(flagManager.readFlagValue<Boolean>(eq("5"), any())).thenReturn(false)
 
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 ResourceBooleanFlag(
                     1,
                     "1",
@@ -214,16 +215,16 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
                 )
             )
         ).isFalse()
-        assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
 
         Assert.assertThrows(NameNotFoundException::class.java) {
-            mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 1004))
+            featureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 1004))
         }
         // Test that resource is loaded (and validated) even when the setting is set.
         //  This prevents developers from not noticing when they reference an invalid resource.
         Assert.assertThrows(NameNotFoundException::class.java) {
-            mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
+            featureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
         }
     }
 
@@ -236,11 +237,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             return@thenAnswer it.getArgument(1)
         }
 
-        assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
-        assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
         assertThat(
-            mFeatureFlagsDebug.isEnabled(
+            featureFlagsDebug.isEnabled(
                 SysPropBooleanFlag(
                     4,
                     "d",
@@ -249,17 +250,17 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
                 )
             )
         ).isFalse()
-        assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
     }
 
     @Test
     fun readStringFlag() {
         whenever(flagManager.readFlagValue<String>(eq("3"), any())).thenReturn("foo")
         whenever(flagManager.readFlagValue<String>(eq("4"), any())).thenReturn("bar")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
+        assertThat(featureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
+        assertThat(featureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
+        assertThat(featureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
+        assertThat(featureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
     }
 
     @Test
@@ -276,7 +277,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         whenever(flagManager.readFlagValue<String>(eq("6"), any())).thenReturn("override6")
 
         assertThat(
-            mFeatureFlagsDebug.getString(
+            featureFlagsDebug.getString(
                 ResourceStringFlag(
                     1,
                     "1",
@@ -286,7 +287,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             )
         ).isEqualTo("")
         assertThat(
-            mFeatureFlagsDebug.getString(
+            featureFlagsDebug.getString(
                 ResourceStringFlag(
                     2,
                     "2",
@@ -296,7 +297,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             )
         ).isEqualTo("resource2")
         assertThat(
-            mFeatureFlagsDebug.getString(
+            featureFlagsDebug.getString(
                 ResourceStringFlag(
                     3,
                     "3",
@@ -307,15 +308,15 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         ).isEqualTo("override3")
 
         Assert.assertThrows(NullPointerException::class.java) {
-            mFeatureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
+            featureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
         }
         Assert.assertThrows(NameNotFoundException::class.java) {
-            mFeatureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
+            featureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
         }
         // Test that resource is loaded (and validated) even when the setting is set.
         //  This prevents developers from not noticing when they reference an invalid resource.
         Assert.assertThrows(NameNotFoundException::class.java) {
-            mFeatureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
+            featureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
         }
     }
 
@@ -323,10 +324,10 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
     fun readIntFlag() {
         whenever(flagManager.readFlagValue<Int>(eq("3"), any())).thenReturn(22)
         whenever(flagManager.readFlagValue<Int>(eq("4"), any())).thenReturn(48)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
+        assertThat(featureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
+        assertThat(featureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
+        assertThat(featureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
+        assertThat(featureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
     }
 
     @Test
@@ -342,17 +343,17 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         whenever(flagManager.readFlagValue<Int>(eq(4), any())).thenReturn(500)
         whenever(flagManager.readFlagValue<Int>(eq(5), any())).thenReturn(9519)
 
-        assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
+        assertThat(featureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
-        assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
+        assertThat(featureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
-        assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
+        assertThat(featureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
 
         Assert.assertThrows(NotFoundException::class.java) {
-            mFeatureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 1004))
+            featureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 1004))
         }
         // Test that resource is loaded (and validated) even when the setting is set.
         //  This prevents developers from not noticing when they reference an invalid resource.
         Assert.assertThrows(NotFoundException::class.java) {
-            mFeatureFlagsDebug.getInt(ResourceIntFlag(5, "5", "test", 1005))
+            featureFlagsDebug.getInt(ResourceIntFlag(5, "5", "test", 1005))
         }
     }
 
@@ -432,11 +433,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("original")
 
         // gets the flag & cache it
-        assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
+        assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
         verify(flagManager, times(1)).readFlagValue(eq("1"), eq(StringFlagSerializer))
 
         // hit the cache
-        assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
+        assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
         verifyNoMoreInteractions(flagManager)
 
         // set the flag
@@ -444,7 +445,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         verifyPutData("1", "{\"type\":\"string\",\"value\":\"new\"}", numReads = 2)
         whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("new")
 
-        assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("new")
+        assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("new")
         verify(flagManager, times(3)).readFlagValue(eq("1"), eq(StringFlagSerializer))
     }
 
@@ -454,7 +455,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
 
         serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
 
-        assertThat(mFeatureFlagsDebug.isEnabled(flag)).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(flag)).isFalse()
     }
 
     @Test
@@ -462,7 +463,33 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
         val flag = UnreleasedFlag(100, name = "100", namespace = "test")
 
         serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
-        assertThat(mFeatureFlagsDebug.isEnabled(flag)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(flag)).isTrue()
+    }
+
+    @Test
+    fun serverSide_OverrideUncached_NoRestart() {
+        // No one has read the flag, so it's not in the cache.
+        serverFlagReader.setFlagValue(
+            teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
+        verify(restarter, never()).restartSystemUI(anyString())
+    }
+
+    @Test
+    fun serverSide_Override_Restarts() {
+        // Read it to put it in the cache.
+        featureFlagsDebug.isEnabled(teamfoodableFlagA)
+        serverFlagReader.setFlagValue(
+            teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
+        verify(restarter).restartSystemUI(anyString())
+    }
+
+    @Test
+    fun serverSide_RedundantOverride_NoRestart() {
+        // Read it to put it in the cache.
+        featureFlagsDebug.isEnabled(teamfoodableFlagA)
+        serverFlagReader.setFlagValue(
+            teamfoodableFlagA.namespace, teamfoodableFlagA.name, teamfoodableFlagA.default)
+        verify(restarter, never()).restartSystemUI(anyString())
     }
 
     @Test
@@ -482,13 +509,13 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             .thenReturn("override7")
 
         // WHEN the flags have been accessed
-        assertThat(mFeatureFlagsDebug.isEnabled(flag1)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(flag1)).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(flag2)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(flag2)).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(flag3)).isFalse()
-        assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty()
+        assertThat(featureFlagsDebug.getString(flag4)).isEmpty()
-        assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
+        assertThat(featureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
-        assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
+        assertThat(featureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
-        assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7")
+        assertThat(featureFlagsDebug.getString(flag7)).isEqualTo("override7")
 
         // THEN the dump contains the flags and the default values
         val dump = dumpToString()
@@ -527,7 +554,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
     private fun dumpToString(): String {
         val sw = StringWriter()
         val pw = PrintWriter(sw)
-        mFeatureFlagsDebug.dump(pw, emptyArray<String>())
+        featureFlagsDebug.dump(pw, emptyArray<String>())
         pw.flush()
         return sw.toString()
     }

packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt

@@ -24,6 +24,8 @@ import org.junit.Assert.assertThrows
 import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
+import org.mockito.Mockito
+import org.mockito.Mockito.never
 import org.mockito.MockitoAnnotations
 import org.mockito.Mockito.`when` as whenever
 
@@ -33,7 +35,7 @@ import org.mockito.Mockito.`when` as whenever
  */
 @SmallTest
 class FeatureFlagsReleaseTest : SysuiTestCase() {
-    private lateinit var mFeatureFlagsRelease: FeatureFlagsRelease
+    private lateinit var featureFlagsRelease: FeatureFlagsRelease
 
     @Mock private lateinit var mResources: Resources
     @Mock private lateinit var mSystemProperties: SystemPropertiesHelper
@@ -41,15 +43,21 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
     private val flagMap = mutableMapOf<String, Flag<*>>()
     private val serverFlagReader = ServerFlagReaderFake()
 
+
+    private val flagA = ReleasedFlag(501, name = "a", namespace = "test")
+
     @Before
     fun setup() {
         MockitoAnnotations.initMocks(this)
-        mFeatureFlagsRelease = FeatureFlagsRelease(
+        flagMap.put(flagA.name, flagA)
+        featureFlagsRelease = FeatureFlagsRelease(
             mResources,
             mSystemProperties,
             serverFlagReader,
             flagMap,
             restarter)
+
+        featureFlagsRelease.init()
     }
 
     @Test
@@ -60,7 +68,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
         val flagNamespace = "test"
         val flag = ResourceBooleanFlag(flagId, flagName, flagNamespace, flagResourceId)
         whenever(mResources.getBoolean(flagResourceId)).thenReturn(true)
-        assertThat(mFeatureFlagsRelease.isEnabled(flag)).isTrue()
+        assertThat(featureFlagsRelease.isEnabled(flag)).isTrue()
     }
 
     @Test
@@ -70,16 +78,16 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
         whenever(mResources.getString(1003)).thenReturn(null)
         whenever(mResources.getString(1004)).thenAnswer { throw NameNotFoundException() }
 
-        assertThat(mFeatureFlagsRelease.getString(
+        assertThat(featureFlagsRelease.getString(
             ResourceStringFlag(1, "1", "test", 1001))).isEqualTo("")
-        assertThat(mFeatureFlagsRelease.getString(
+        assertThat(featureFlagsRelease.getString(
             ResourceStringFlag(2, "2", "test", 1002))).isEqualTo("res2")
 
         assertThrows(NullPointerException::class.java) {
-            mFeatureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
+            featureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
         }
         assertThrows(NameNotFoundException::class.java) {
-            mFeatureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
+            featureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
         }
     }
 
@@ -92,7 +100,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
 
         val flag = SysPropBooleanFlag(flagId, flagName, flagNamespace, flagDefault)
         whenever(mSystemProperties.getBoolean(flagName, flagDefault)).thenReturn(flagDefault)
-        assertThat(mFeatureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
+        assertThat(featureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
     }
 
     @Test
@@ -101,7 +109,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
 
         serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
 
-        assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
+        assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
     }
 
     @Test
@@ -110,6 +118,32 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
 
         serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
 
-        assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
+        assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
+    }
+
+    @Test
+    fun serverSide_OverrideUncached_NoRestart() {
+        // No one has read the flag, so it's not in the cache.
+        serverFlagReader.setFlagValue(
+            flagA.namespace, flagA.name, !flagA.default)
+        Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
+    }
+
+    @Test
+    fun serverSide_Override_Restarts() {
+        // Read it to put it in the cache.
+        featureFlagsRelease.isEnabled(flagA)
+        serverFlagReader.setFlagValue(
+            flagA.namespace, flagA.name, !flagA.default)
+        Mockito.verify(restarter).restartSystemUI(Mockito.anyString())
+    }
+
+    @Test
+    fun serverSide_RedundantOverride_NoRestart() {
+        // Read it to put it in the cache.
+        featureFlagsRelease.isEnabled(flagA)
+        serverFlagReader.setFlagValue(
+            flagA.namespace, flagA.name, flagA.default)
+        Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
     }
 }

packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt

@@ -21,11 +21,13 @@ import android.testing.AndroidTestingRunner
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.util.DeviceConfigProxyFake
 import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.mockito.any
 import com.android.systemui.util.time.FakeSystemClock
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
+import org.mockito.Mockito.anyString
 import org.mockito.Mockito.never
 import org.mockito.Mockito.verify
 import org.mockito.MockitoAnnotations
@@ -57,7 +59,7 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
         deviceConfig.setProperty(NAMESPACE, "flag_1", "1", false)
         executor.runAllReady()
 
-        verify(changeListener).onChange(flag)
+        verify(changeListener).onChange(flag, "1")
     }
 
     @Test
@@ -69,6 +71,6 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
         deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false)
         executor.runAllReady()
 
-        verify(changeListener, never()).onChange(flag)
+        verify(changeListener, never()).onChange(any(), anyString())
     }
 }