Only restart SystemUI when Server Flags actually change.

DeviceConfig is alerting us to all server pushes (pulls?). Even if
the flags is checking on aren't actually changing.

This CL ensures we only restart if the flag value has actually
changed.

Fixes: 272780732
Test: atest SystemUITests
Change-Id: I3571d210885472dc69f50c76922a89733f1a5d83

6 files changed, 247 insertions, 104 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
index 58fe0940b7fb..367a9b9bd4a0 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsDebug.java
+++ b/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) {
-                    mRestarter.restartSystemUI(
-                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+                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());
+
+                    }
                 }
             };
 
diff --git a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java b/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
index 9859ff6b4917..9d19a7dc4604 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/FeatureFlagsRelease.java
+++ b/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) {
-                    mRestarter.restartSystemUI(
-                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
+                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());
+                    }
                 }
             };
 
@@ -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())) {
-            return isEnabled(flag.getName(), mResources.getBoolean(flag.getResourceId()));
-        }
-
-        return mBooleanCache.get(flag.getName());
+        // Fill the cache.
+        return isEnabledInternal(flag.getName(), mResources.getBoolean(flag.getResourceId()));
     }
 
     @Override
     public boolean isEnabled(SysPropBooleanFlag flag) {
-        if (!mBooleanCache.containsKey(flag.getName())) {
-            return isEnabled(
-                    flag.getName(),
-                    mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
-        }
-
-        return mBooleanCache.get(flag.getName());
+        // Fill the cache.
+        return isEnabledInternal(
+                flag.getName(),
+                mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
     }
 
-    private boolean isEnabled(String name, boolean defaultValue) {
-        mBooleanCache.put(name, defaultValue);
-        return defaultValue;
+    /**
+     * 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);
+        }
+
+        return mBooleanCache.get(name);
     }
 
     @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())) {
-            return getString(flag.getName(),
-                    requireNonNull(mResources.getString(flag.getResourceId())));
-        }
-
-        return mStringCache.get(flag.getName());
+        // Fill the cache.
+        return getStringInternal(flag.getName(),
+                requireNonNull(mResources.getString(flag.getResourceId())));
     }
 
-    private String getString(String name, String defaultValue) {
-        mStringCache.put(name, defaultValue);
-        return defaultValue;
+    /**
+     * 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);
+        }
+
+        return mStringCache.get(name);
     }
 
     @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");
diff --git a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt b/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt
index 9b748d0a0eb2..eaf5eac155ef 100644
--- a/packages/SystemUI/src/com/android/systemui/flags/ServerFlagReader.kt
+++ b/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))
     }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt
index 2bcd75b7c707..18f7db18b1f4 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsDebugTest.kt
+++ b/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(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
+        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(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).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(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
+        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(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).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(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
+        assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).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(mFeatureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
-        assertThat(mFeatureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
+        assertThat(featureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
+        assertThat(featureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
+        assertThat(featureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
+        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(mFeatureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
-        assertThat(mFeatureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
+        assertThat(featureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
+        assertThat(featureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
+        assertThat(featureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
+        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(mFeatureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
-        assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
+        assertThat(featureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
+        assertThat(featureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
+        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(mFeatureFlagsDebug.isEnabled(flag2)).isTrue()
-        assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse()
-        assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty()
-        assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
-        assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
-        assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7")
+        assertThat(featureFlagsDebug.isEnabled(flag1)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(flag2)).isTrue()
+        assertThat(featureFlagsDebug.isEnabled(flag3)).isFalse()
+        assertThat(featureFlagsDebug.getString(flag4)).isEmpty()
+        assertThat(featureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
+        assertThat(featureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
+        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()
     }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt
index 4c6028c4c9b7..917147b17517 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagsReleaseTest.kt
+++ b/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())
     }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt
index 2e9800606edf..953b7fb32d56 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/flags/ServerFlagReaderImplTest.kt
+++ b/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,18 +59,18 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
         deviceConfig.setProperty(NAMESPACE, "flag_1", "1", false)
         executor.runAllReady()
 
-        verify(changeListener).onChange(flag)
+        verify(changeListener).onChange(flag, "1")
     }
 
     @Test
     fun testChange_ignoresListenersDuringTest() {
         val serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor, true)
-        val flag = ReleasedFlag(1, "1", "test")
+        val flag = ReleasedFlag(1, "1", "   test")
         serverFlagReader.listenForChanges(listOf(flag), changeListener)
 
         deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false)
         executor.runAllReady()
 
-        verify(changeListener, never()).onChange(flag)
+        verify(changeListener, never()).onChange(any(), anyString())
     }
 }