Don't restart SystemUI in TestHarness mode.

Log the name of the flag that is causing restarts.

Also, we weren't copying old overrriden-id's into the new
overridden-names. Fix that.

Fixes: 267753212
Test: atest SystemUITests and some manual flag flipping.
Change-Id: I7df265a54e657453c664c3d77987ee498c1c97cd

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

@@ -40,7 +40,6 @@ import androidx.annotation.Nullable;
 import com.android.systemui.dagger.SysUISingleton;
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.util.settings.GlobalSettings;
-import com.android.systemui.util.settings.SecureSettings;
 
 import org.jetbrains.annotations.NotNull;
 
@@ -74,7 +73,6 @@ public class FeatureFlagsDebug implements FeatureFlags {
     private final FlagManager mFlagManager;
     private final Context mContext;
     private final GlobalSettings mGlobalSettings;
-    private final SecureSettings mSecureSettings;
     private final Resources mResources;
     private final SystemPropertiesHelper mSystemProperties;
     private final ServerFlagReader mServerFlagReader;
@@ -87,8 +85,9 @@ public class FeatureFlagsDebug implements FeatureFlags {
     private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
             new ServerFlagReader.ChangeListener() {
                 @Override
-                public void onChange() {
-                    mRestarter.restartSystemUI();
+                public void onChange(Flag<?> flag) {
+                    mRestarter.restartSystemUI(
+                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
                 }
             };
 
@@ -97,7 +96,6 @@ public class FeatureFlagsDebug implements FeatureFlags {
             FlagManager flagManager,
             Context context,
             GlobalSettings globalSettings,
-            SecureSettings secureSettings,
             SystemPropertiesHelper systemProperties,
             @Main Resources resources,
             ServerFlagReader serverFlagReader,
@@ -106,7 +104,6 @@ public class FeatureFlagsDebug implements FeatureFlags {
         mFlagManager = flagManager;
         mContext = context;
         mGlobalSettings = globalSettings;
-        mSecureSettings = secureSettings;
         mResources = resources;
         mSystemProperties = systemProperties;
         mServerFlagReader = serverFlagReader;
@@ -119,7 +116,8 @@ public class FeatureFlagsDebug implements FeatureFlags {
         IntentFilter filter = new IntentFilter();
         filter.addAction(ACTION_SET_FLAG);
         filter.addAction(ACTION_GET_FLAGS);
-        mFlagManager.setOnSettingsChangedAction(this::restartSystemUI);
+        mFlagManager.setOnSettingsChangedAction(
+                suppressRestart -> restartSystemUI(suppressRestart, "Settings changed"));
         mFlagManager.setClearCacheAction(this::removeFromCache);
         mContext.registerReceiver(mReceiver, filter, null, null,
                 Context.RECEIVER_EXPORTED_UNAUDITED);
@@ -233,6 +231,10 @@ public class FeatureFlagsDebug implements FeatureFlags {
         Boolean result = readBooleanFlagOverride(flag.getName());
         if (result == null) {
             result = readBooleanFlagOverride(flag.getId());
+            if (result != null) {
+                // Move overrides from id to name
+                setFlagValueInternal(flag.getName(), result, BooleanFlagSerializer.INSTANCE);
+            }
         }
         boolean hasServerOverride = mServerFlagReader.hasOverride(
                 flag.getNamespace(), flag.getName());
@@ -305,25 +307,38 @@ public class FeatureFlagsDebug implements FeatureFlags {
         requireNonNull(value, "Cannot set a null value");
         T currentValue = readFlagValueInternal(name, serializer);
         if (Objects.equals(currentValue, value)) {
-            Log.i(TAG, "Flag id " + name + " is already " + value);
+            Log.i(TAG, "Flag \"" + name + "\" is already " + value);
             return;
         }
+        setFlagValueInternal(name, value, serializer);
+        Log.i(TAG, "Set flag \"" + name + "\" to " + value);
+        removeFromCache(name);
+        mFlagManager.dispatchListenersAndMaybeRestart(
+                name,
+                suppressRestart -> restartSystemUI(
+                        suppressRestart, "Flag \"" + name + "\" changed to " + value));
+    }
+
+    private <T> void setFlagValueInternal(
+            String name, @NonNull T value, FlagSerializer<T> serializer) {
         final String data = serializer.toSettingsData(value);
         if (data == null) {
-            Log.w(TAG, "Failed to set id " + name + " to " + value);
+            Log.w(TAG, "Failed to set flag " + name + " to " + value);
             return;
         }
         mGlobalSettings.putStringForUser(mFlagManager.nameToSettingsKey(name), data,
                 UserHandle.USER_CURRENT);
-        Log.i(TAG, "Set id " + name + " to " + value);
-        removeFromCache(name);
-        mFlagManager.dispatchListenersAndMaybeRestart(name, this::restartSystemUI);
     }
 
     <T> void eraseFlag(Flag<T> flag) {
         if (flag instanceof SysPropFlag) {
-            mSystemProperties.erase(((SysPropFlag<T>) flag).getName());
-            dispatchListenersAndMaybeRestart(flag.getName(), this::restartAndroid);
+            mSystemProperties.erase(flag.getName());
+            dispatchListenersAndMaybeRestart(
+                    flag.getName(),
+                    suppressRestart -> restartSystemUI(
+                            suppressRestart,
+                            "SysProp Flag \"" + flag.getNamespace() + "."
+                                    + flag.getName() + "\" reset to default."));
         } else {
             eraseFlag(flag.getName());
         }
@@ -333,7 +348,10 @@ public class FeatureFlagsDebug implements FeatureFlags {
     private void eraseFlag(String name) {
         eraseInternal(name);
         removeFromCache(name);
-        dispatchListenersAndMaybeRestart(name, this::restartSystemUI);
+        dispatchListenersAndMaybeRestart(
+                name,
+                suppressRestart -> restartSystemUI(
+                        suppressRestart, "Flag \"" + name + "\" reset to default"));
     }
 
     private void dispatchListenersAndMaybeRestart(String name, Consumer<Boolean> restartAction) {
@@ -367,20 +385,20 @@ public class FeatureFlagsDebug implements FeatureFlags {
         mFlagManager.removeListener(listener);
     }
 
-    private void restartSystemUI(boolean requestSuppress) {
+    private void restartSystemUI(boolean requestSuppress, String reason) {
         if (requestSuppress) {
             Log.i(TAG, "SystemUI Restart Suppressed");
             return;
         }
-        mRestarter.restartSystemUI();
+        mRestarter.restartSystemUI(reason);
     }
 
-    private void restartAndroid(boolean requestSuppress) {
+    private void restartAndroid(boolean requestSuppress, String reason) {
         if (requestSuppress) {
             Log.i(TAG, "Android Restart Suppressed");
             return;
         }
-        mRestarter.restartAndroid();
+        mRestarter.restartAndroid(reason);
     }
 
     void setBooleanFlagInternal(Flag<?> flag, boolean value) {
@@ -391,8 +409,11 @@ public class FeatureFlagsDebug implements FeatureFlags {
         } else if (flag instanceof SysPropBooleanFlag) {
             // Store SysProp flags in SystemProperties where they can read by outside parties.
             mSystemProperties.setBoolean(((SysPropBooleanFlag) flag).getName(), value);
-            dispatchListenersAndMaybeRestart(flag.getName(),
-                    FeatureFlagsDebug.this::restartAndroid);
+            dispatchListenersAndMaybeRestart(
+                    flag.getName(),
+                    suppressRestart -> restartSystemUI(
+                            suppressRestart,
+                            "Flag \"" + flag.getName() + "\" changed to " + value));
         } else {
             throw new IllegalArgumentException("Unknown flag type");
         }

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

@@ -29,6 +29,7 @@ constructor(
 ) : Restarter {
 
     private var androidRestartRequested = false
+    private var pendingReason = ""
 
     val observer =
         object : WakefulnessLifecycle.Observer {
@@ -38,18 +39,20 @@ constructor(
             }
         }
 
-    override fun restartSystemUI() {
+    override fun restartSystemUI(reason: String) {
         Log.d(FeatureFlagsDebug.TAG, "SystemUI Restart requested. Restarting on next screen off.")
-        scheduleRestart()
+        Log.i(FeatureFlagsDebug.TAG, reason)
+        scheduleRestart(reason)
     }
 
-    override fun restartAndroid() {
+    override fun restartAndroid(reason: String) {
         Log.d(FeatureFlagsDebug.TAG, "Android Restart requested. Restarting on next screen off.")
         androidRestartRequested = true
-        scheduleRestart()
+        scheduleRestart(reason)
     }
 
-    fun scheduleRestart() {
+    fun scheduleRestart(reason: String) {
+        pendingReason = reason
         if (wakefulnessLifecycle.wakefulness == WakefulnessLifecycle.WAKEFULNESS_ASLEEP) {
             restartNow()
         } else {
@@ -59,9 +62,9 @@ constructor(
 
     private fun restartNow() {
         if (androidRestartRequested) {
-            systemExitRestarter.restartAndroid()
+            systemExitRestarter.restartAndroid(pendingReason)
         } else {
-            systemExitRestarter.restartSystemUI()
+            systemExitRestarter.restartSystemUI(pendingReason)
         }
     }
 }

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

@@ -57,8 +57,9 @@ public class FeatureFlagsRelease implements FeatureFlags {
     private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
             new ServerFlagReader.ChangeListener() {
                 @Override
-                public void onChange() {
-                    mRestarter.restartSystemUI();
+                public void onChange(Flag<?> flag) {
+                    mRestarter.restartSystemUI(
+                            "Server flag change: " + flag.getNamespace() + "." + flag.getName());
                 }
             };
 

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

@@ -36,41 +36,43 @@ constructor(
 ) : Restarter {
     var listenersAdded = false
     var pendingRestart: Runnable? = null
+    private var pendingReason = ""
     var androidRestartRequested = false
 
     val observer =
         object : WakefulnessLifecycle.Observer {
             override fun onFinishedGoingToSleep() {
-                scheduleRestart()
+                scheduleRestart(pendingReason)
             }
         }
 
     val batteryCallback =
         object : BatteryController.BatteryStateChangeCallback {
             override fun onBatteryLevelChanged(level: Int, pluggedIn: Boolean, charging: Boolean) {
-                scheduleRestart()
+                scheduleRestart(pendingReason)
             }
         }
 
-    override fun restartSystemUI() {
+    override fun restartSystemUI(reason: String) {
         Log.d(
             FeatureFlagsDebug.TAG,
             "SystemUI Restart requested. Restarting when plugged in and idle."
         )
-        scheduleRestart()
+        scheduleRestart(reason)
     }
 
-    override fun restartAndroid() {
+    override fun restartAndroid(reason: String) {
         Log.d(
             FeatureFlagsDebug.TAG,
             "Android Restart requested. Restarting when plugged in and idle."
         )
         androidRestartRequested = true
-        scheduleRestart()
+        scheduleRestart(reason)
     }
 
-    private fun scheduleRestart() {
+    private fun scheduleRestart(reason: String) {
         // Don't bother adding listeners twice.
+        pendingReason = reason
         if (!listenersAdded) {
             listenersAdded = true
             wakefulnessLifecycle.addObserver(observer)
@@ -91,9 +93,9 @@ constructor(
     private fun restartNow() {
         Log.d(FeatureFlagsRelease.TAG, "Restarting due to systemui flag change")
         if (androidRestartRequested) {
-            systemExitRestarter.restartAndroid()
+            systemExitRestarter.restartAndroid(pendingReason)
         } else {
-            systemExitRestarter.restartSystemUI()
+            systemExitRestarter.restartSystemUI(pendingReason)
         }
     }
 }

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

@@ -16,7 +16,7 @@
 package com.android.systemui.flags
 
 interface Restarter {
-    fun restartSystemUI()
+    fun restartSystemUI(reason: String)
 
-    fun restartAndroid()
+    fun restartAndroid(reason: String)
 }

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

@@ -17,8 +17,10 @@
 package com.android.systemui.flags
 
 import android.provider.DeviceConfig
+import android.util.Log
 import com.android.systemui.dagger.SysUISingleton
 import com.android.systemui.dagger.qualifiers.Background
+import com.android.systemui.dagger.qualifiers.TestHarness
 import com.android.systemui.util.DeviceConfigProxy
 import dagger.Module
 import dagger.Provides
@@ -35,21 +37,27 @@ interface ServerFlagReader {
     fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener)
 
     interface ChangeListener {
-        fun onChange()
+        fun onChange(flag: Flag<*>)
     }
 }
 
 class ServerFlagReaderImpl @Inject constructor(
     private val namespace: String,
     private val deviceConfig: DeviceConfigProxy,
-    @Background private val executor: Executor
+    @Background private val executor: Executor,
+    @TestHarness private val isTestHarness: Boolean
 ) : ServerFlagReader {
 
+    private val TAG = "ServerFlagReader"
+
     private val listeners =
         mutableListOf<Pair<ServerFlagReader.ChangeListener, Collection<Flag<*>>>>()
 
     private val onPropertiesChangedListener = object : DeviceConfig.OnPropertiesChangedListener {
         override fun onPropertiesChanged(properties: DeviceConfig.Properties) {
+            if (isTestHarness) {
+                Log.w(TAG, "Ignore server flag changes in Test Harness mode.")
+            }
             if (properties.namespace != namespace) {
                 return
             }
@@ -59,7 +67,7 @@ class ServerFlagReaderImpl @Inject constructor(
                 propLoop@ for (propName in properties.keyset) {
                     for (flag in flags) {
                         if (propName == getServerOverrideName(flag.id) || propName == flag.name) {
-                            listener.onChange()
+                            listener.onChange(flag)
                             break@propLoop
                         }
                     }
@@ -111,10 +119,11 @@ interface ServerFlagReaderModule {
         @SysUISingleton
         fun bindsReader(
             deviceConfig: DeviceConfigProxy,
-            @Background executor: Executor
+            @Background executor: Executor,
+            @TestHarness isTestHarness: Boolean
         ): ServerFlagReader {
             return ServerFlagReaderImpl(
-                SYSUI_NAMESPACE, deviceConfig, executor
+                SYSUI_NAMESPACE, deviceConfig, executor, isTestHarness
             )
         }
     }
@@ -139,7 +148,7 @@ class ServerFlagReaderFake : ServerFlagReader {
         for ((listener, flags) in listeners) {
             flagLoop@ for (flag in flags) {
                 if (name == flag.name) {
-                    listener.onChange()
+                    listener.onChange(flag)
                     break@flagLoop
                 }
             }

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

@@ -16,6 +16,7 @@
 
 package com.android.systemui.flags
 
+import android.util.Log
 import com.android.internal.statusbar.IStatusBarService
 import javax.inject.Inject
 
@@ -24,11 +25,13 @@ class SystemExitRestarter
 constructor(
     private val barService: IStatusBarService,
 ) : Restarter {
-    override fun restartAndroid() {
+    override fun restartAndroid(reason: String) {
+        Log.d(FeatureFlagsDebug.TAG, "Restarting Android: " + reason)
         barService.restart()
     }
 
-    override fun restartSystemUI() {
+    override fun restartSystemUI(reason: String) {
+        Log.d(FeatureFlagsDebug.TAG, "Restarting SystemUI: " + reason)
         System.exit(0)
     }
 }

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

@@ -20,6 +20,7 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.keyguard.WakefulnessLifecycle
 import com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_ASLEEP
 import com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_AWAKE
+import com.android.systemui.util.mockito.any
 import org.junit.Before
 import org.junit.Test
 import org.mockito.ArgumentCaptor
@@ -48,22 +49,22 @@ class FeatureFlagsDebugRestarterTest : SysuiTestCase() {
     @Test
     fun testRestart_ImmediateWhenAsleep() {
         whenever(wakefulnessLifecycle.wakefulness).thenReturn(WAKEFULNESS_ASLEEP)
-        restarter.restartSystemUI()
-        verify(systemExitRestarter).restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
+        verify(systemExitRestarter).restartSystemUI(any())
     }
 
     @Test
     fun testRestart_WaitsForSceenOff() {
         whenever(wakefulnessLifecycle.wakefulness).thenReturn(WAKEFULNESS_AWAKE)
 
-        restarter.restartSystemUI()
-        verify(systemExitRestarter, never()).restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
+        verify(systemExitRestarter, never()).restartSystemUI(any())
 
         val captor = ArgumentCaptor.forClass(WakefulnessLifecycle.Observer::class.java)
         verify(wakefulnessLifecycle).addObserver(captor.capture())
 
         captor.value.onFinishedGoingToSleep()
 
-        verify(systemExitRestarter).restartSystemUI()
+        verify(systemExitRestarter).restartSystemUI(any())
     }
 }

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

@@ -28,7 +28,6 @@ import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.nullable
 import com.android.systemui.util.mockito.withArgCaptor
 import com.android.systemui.util.settings.GlobalSettings
-import com.android.systemui.util.settings.SecureSettings
 import com.google.common.truth.Truth.assertThat
 import org.junit.Assert
 import org.junit.Before
@@ -63,8 +62,6 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
     @Mock
     private lateinit var globalSettings: GlobalSettings
     @Mock
-    private lateinit var secureSettings: SecureSettings
-    @Mock
     private lateinit var systemProperties: SystemPropertiesHelper
     @Mock
     private lateinit var resources: Resources
@@ -92,7 +89,6 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
             flagManager,
             mockContext,
             globalSettings,
-            secureSettings,
             systemProperties,
             resources,
             serverFlagReader,

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

@@ -22,6 +22,7 @@ import com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_ASLEEP
 import com.android.systemui.keyguard.WakefulnessLifecycle.WAKEFULNESS_AWAKE
 import com.android.systemui.statusbar.policy.BatteryController
 import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.mockito.any
 import com.android.systemui.util.time.FakeSystemClock
 import com.google.common.truth.Truth.assertThat
 import org.junit.Before
@@ -63,7 +64,7 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(batteryController.isPluggedIn).thenReturn(true)
 
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
         assertThat(executor.numPending()).isEqualTo(1)
     }
 
@@ -72,11 +73,11 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(wakefulnessLifecycle.wakefulness).thenReturn(WAKEFULNESS_ASLEEP)
         whenever(batteryController.isPluggedIn).thenReturn(true)
 
-        restarter.restartSystemUI()
-        verify(systemExitRestarter, never()).restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
+        verify(systemExitRestarter, never()).restartSystemUI("Restart for test")
         executor.advanceClockToLast()
         executor.runAllReady()
-        verify(systemExitRestarter).restartSystemUI()
+        verify(systemExitRestarter).restartSystemUI(any())
     }
 
     @Test
@@ -85,7 +86,7 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(batteryController.isPluggedIn).thenReturn(true)
 
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
         assertThat(executor.numPending()).isEqualTo(0)
     }
 
@@ -95,7 +96,7 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(batteryController.isPluggedIn).thenReturn(false)
 
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
         assertThat(executor.numPending()).isEqualTo(0)
     }
 
@@ -105,8 +106,8 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(batteryController.isPluggedIn).thenReturn(true)
 
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
+        restarter.restartSystemUI("Restart for test")
         assertThat(executor.numPending()).isEqualTo(1)
     }
 
@@ -115,7 +116,7 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(wakefulnessLifecycle.wakefulness).thenReturn(WAKEFULNESS_AWAKE)
         whenever(batteryController.isPluggedIn).thenReturn(true)
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
 
         val captor = ArgumentCaptor.forClass(WakefulnessLifecycle.Observer::class.java)
         verify(wakefulnessLifecycle).addObserver(captor.capture())
@@ -131,7 +132,7 @@ class FeatureFlagsReleaseRestarterTest : SysuiTestCase() {
         whenever(wakefulnessLifecycle.wakefulness).thenReturn(WAKEFULNESS_ASLEEP)
         whenever(batteryController.isPluggedIn).thenReturn(false)
         assertThat(executor.numPending()).isEqualTo(0)
-        restarter.restartSystemUI()
+        restarter.restartSystemUI("Restart for test")
 
         val captor =
             ArgumentCaptor.forClass(BatteryController.BatteryStateChangeCallback::class.java)

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

@@ -45,7 +45,7 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
     fun setup() {
         MockitoAnnotations.initMocks(this)
 
-        serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor)
+        serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor, false)
     }
 
     @Test
@@ -56,6 +56,6 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
         deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false)
         executor.runAllReady()
 
-        verify(changeListener).onChange()
+        verify(changeListener).onChange(flag)
     }
 }