[Ravenwood] Update system property handling

- Split out "core" system properties (the default properties we set
  during the global initialization) and "test" system properties (the
  properties defined and set through RavenwoodRule)
- Reset only the "test" system properties before each test execution
- Update the implementation to support nested RavenwoodRule in the
  future

Flag: EXEMPT host test change only
Bug: 377765941
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh
Change-Id: Ia9a2ee217aa89e0f2565d14fb26e3842947e9dc7

3 files changed, 279 insertions, 27 deletions

diff --git a/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRunnerState.java b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRunnerState.java
index 110de989b524..6dfcf4ce03cf 100644
--- a/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRunnerState.java
+++ b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRunnerState.java
@@ -15,12 +15,23 @@
  */
 package android.platform.test.ravenwood;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 import android.util.Log;
+import android.util.Pair;
+
+import com.android.ravenwood.RavenwoodRuntimeNative;
 
 import org.junit.runner.Description;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
 /**
- * Used to store various states associated with the current test runner that's inly needed
+ * Used to store various states associated with the current test runner that's only needed
  * in junit-impl.
  *
  * We don't want to put it in junit-src to avoid having to recompile all the downstream
@@ -30,6 +41,11 @@ import org.junit.runner.Description;
  */
 public final class RavenwoodRunnerState {
     private static final String TAG = "RavenwoodRunnerState";
+    private static final String RAVENWOOD_RULE_ERROR =
+            "RavenwoodRule(s) are not executed in the correct order";
+
+    private static final List<Pair<RavenwoodRule, RavenwoodPropertyState>> sActiveProperties =
+            new ArrayList<>();
 
     private final RavenwoodAwareTestRunner mRunner;
 
@@ -53,6 +69,7 @@ public final class RavenwoodRunnerState {
 
     public void exitTestClass() {
         Log.i(TAG, "exitTestClass: " + mRunner.mTestJavaClass.getName());
+        assertTrue(RAVENWOOD_RULE_ERROR, sActiveProperties.isEmpty());
         RavenwoodRuntimeEnvironmentController.exitTestClass();
     }
 
@@ -66,9 +83,68 @@ public final class RavenwoodRunnerState {
     }
 
     public void enterRavenwoodRule(RavenwoodRule rule) {
-        RavenwoodRuntimeEnvironmentController.setSystemProperties(rule.mSystemProperties);
+        pushTestProperties(rule);
     }
 
     public void exitRavenwoodRule(RavenwoodRule rule) {
+        popTestProperties(rule);
+    }
+
+    static class RavenwoodPropertyState {
+
+        final List<Pair<String, String>> mBackup;
+        final Set<String> mKeyReadable;
+        final Set<String> mKeyWritable;
+
+        RavenwoodPropertyState(RavenwoodTestProperties props) {
+            mBackup = props.mValues.keySet().stream()
+                    .map(key -> Pair.create(key, RavenwoodRuntimeNative.getSystemProperty(key)))
+                    .toList();
+            mKeyReadable = Set.copyOf(props.mKeyReadable);
+            mKeyWritable = Set.copyOf(props.mKeyWritable);
+        }
+
+        boolean isKeyAccessible(String key, boolean write) {
+            return write ? mKeyWritable.contains(key) : mKeyReadable.contains(key);
+        }
+
+        void restore() {
+            mBackup.forEach(pair -> {
+                if (pair.second == null) {
+                    RavenwoodRuntimeNative.removeSystemProperty(pair.first);
+                } else {
+                    RavenwoodRuntimeNative.setSystemProperty(pair.first, pair.second);
+                }
+            });
+        }
+    }
+
+    private static void pushTestProperties(RavenwoodRule rule) {
+        sActiveProperties.add(Pair.create(rule, new RavenwoodPropertyState(rule.mProperties)));
+        rule.mProperties.mValues.forEach(RavenwoodRuntimeNative::setSystemProperty);
+    }
+
+    private static void popTestProperties(RavenwoodRule rule) {
+        var pair = sActiveProperties.removeLast();
+        assertNotNull(RAVENWOOD_RULE_ERROR, pair);
+        assertEquals(RAVENWOOD_RULE_ERROR, rule, pair.first);
+        pair.second.restore();
+    }
+
+    @SuppressWarnings("unused")  // Called from native code (ravenwood_sysprop.cpp)
+    private static void checkSystemPropertyAccess(String key, boolean write) {
+        if (write && RavenwoodSystemProperties.sDefaultValues.containsKey(key)) {
+            // The default core values should never be modified
+            throw new IllegalArgumentException(
+                    "Setting core system property '" + key + "' is not allowed");
+        }
+
+        final boolean result = RavenwoodSystemProperties.isKeyAccessible(key, write)
+                || sActiveProperties.stream().anyMatch(p -> p.second.isKeyAccessible(key, write));
+
+        if (!result) {
+            throw new IllegalArgumentException((write ? "Write" : "Read")
+                    + " access to system property '" + key + "' denied via RavenwoodRule");
+        }
     }
 }
diff --git a/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRuntimeEnvironmentController.java b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRuntimeEnvironmentController.java
index c2ed45d8f427..e730a292a9da 100644
--- a/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRuntimeEnvironmentController.java
+++ b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodRuntimeEnvironmentController.java
@@ -163,8 +163,6 @@ public class RavenwoodRuntimeEnvironmentController {
     @GuardedBy("sInitializationLock")
     private static Throwable sExceptionFromGlobalInit;
 
-    private static RavenwoodSystemProperties sProps;
-
     private static final int DEFAULT_TARGET_SDK_LEVEL = VERSION_CODES.CUR_DEVELOPMENT;
     private static final String DEFAULT_PACKAGE_NAME = "com.android.ravenwoodtests.defaultname";
 
@@ -234,7 +232,6 @@ public class RavenwoodRuntimeEnvironmentController {
 
         // Do the basic set up for the android sysprops.
         RavenwoodSystemProperties.initialize();
-        setSystemProperties(null);
 
         // Do this after loading RAVENWOOD_NATIVE_RUNTIME_NAME (which backs Os.setenv()),
         // before loadFrameworkNativeCode() (which uses $ANDROID_LOG_TAGS).
@@ -356,10 +353,13 @@ public class RavenwoodRuntimeEnvironmentController {
         // will call Mockito.framework().clearInlineMocks() after execution.
         sInstrumentation.basicInit(instContext, targetContext, createMockUiAutomation());
 
+        // Reset some global state
         Process_ravenwood.reset();
         DeviceConfig_host.reset();
         Binder.restoreCallingIdentity(sCallingIdentity);
 
+        SystemProperties.clearChangeCallbacksForTest();
+
         if (ENABLE_TIMEOUT_STACKS) {
             sPendingTimeout = sTimeoutExecutor.schedule(
                     RavenwoodRuntimeEnvironmentController::dumpStacks,
@@ -484,19 +484,6 @@ public class RavenwoodRuntimeEnvironmentController {
         }
     }
 
-    /**
-     * Set the current configuration to the actual SystemProperties.
-     */
-    public static void setSystemProperties(@Nullable RavenwoodSystemProperties systemProperties) {
-        SystemProperties.clearChangeCallbacksForTest();
-        RavenwoodRuntimeNative.clearSystemProperties();
-        if (systemProperties == null) systemProperties = new RavenwoodSystemProperties();
-        sProps = new RavenwoodSystemProperties(systemProperties, true);
-        for (var entry : systemProperties.getValues().entrySet()) {
-            RavenwoodRuntimeNative.setSystemProperty(entry.getKey(), entry.getValue());
-        }
-    }
-
     private static final String MOCKITO_ERROR = "FATAL: Unsupported Mockito detected!"
             + " Your test or its dependencies use one of the \"mockito-target-*\""
             + " modules as static library, which is unusable on host side."
@@ -546,15 +533,6 @@ public class RavenwoodRuntimeEnvironmentController {
         return mock;
     }
 
-    @SuppressWarnings("unused")  // Called from native code (ravenwood_sysprop.cpp)
-    private static void checkSystemPropertyAccess(String key, boolean write) {
-        boolean result = write ? sProps.isKeyWritable(key) : sProps.isKeyReadable(key);
-        if (!result) {
-            throw new IllegalArgumentException((write ? "Write" : "Read")
-                    + " access to system property '" + key + "' denied via RavenwoodConfig");
-        }
-    }
-
     private static void dumpCommandLineArgs() {
         Log.i(TAG, "JVM arguments:");
 
diff --git a/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodSystemProperties.java b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodSystemProperties.java
new file mode 100644
index 000000000000..c545baacdf3e
--- /dev/null
+++ b/ravenwood/junit-impl-src/android/platform/test/ravenwood/RavenwoodSystemProperties.java
@@ -0,0 +1,198 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package android.platform.test.ravenwood;
+
+import static com.android.ravenwood.common.RavenwoodCommonUtils.RAVENWOOD_VERBOSE_LOGGING;
+import static com.android.ravenwood.common.RavenwoodCommonUtils.getRavenwoodRuntimePath;
+
+import android.util.Log;
+
+import com.android.ravenwood.RavenwoodRuntimeNative;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * A class to manage the core default system properties of the Ravenwood environment.
+ */
+public class RavenwoodSystemProperties {
+    private static final String TAG = "RavenwoodSystemProperties";
+
+    /** We pull in properties from this file. */
+    private static final String RAVENWOOD_BUILD_PROP = "ravenwood-data/ravenwood-build.prop";
+
+    /** This is the actual build.prop we use to build the device (contents depends on lunch). */
+    private static final String DEVICE_BUILD_PROP = "ravenwood-data/build.prop";
+
+    /** The default values. */
+    static final Map<String, String> sDefaultValues = new HashMap<>();
+
+    private static final String[] PARTITIONS = {
+            "bootimage",
+            "odm",
+            "product",
+            "system",
+            "system_ext",
+            "vendor",
+            "vendor_dlkm",
+    };
+
+    static Map<String, String> readProperties(String propFile) {
+        // Use an ordered map just for cleaner dump log.
+        final Map<String, String> ret = new LinkedHashMap<>();
+        try {
+            Files.readAllLines(Path.of(propFile)).stream()
+                    .map(String::trim)
+                    .filter(s -> !s.startsWith("#"))
+                    .map(s -> s.split("\\s*=\\s*", 2))
+                    .filter(a -> a.length == 2 && a[1].length() > 0)
+                    .forEach(a -> ret.put(a[0], a[1]));
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        return ret;
+    }
+
+    /**
+     * Load default sysprops from {@link #RAVENWOOD_BUILD_PROP}. We also pull in
+     * certain properties from the acutual device's build.prop {@link #DEVICE_BUILD_PROP} too.
+     *
+     * More info about property file loading: system/core/init/property_service.cpp
+     * In the following logic, the only partition we would need to consider is "system",
+     * since we only read from system-build.prop
+     */
+    static void initialize() {
+        var path = getRavenwoodRuntimePath();
+        var ravenwoodProps = readProperties(path + RAVENWOOD_BUILD_PROP);
+        var deviceProps = readProperties(path + DEVICE_BUILD_PROP);
+
+        Log.i(TAG, "Default system properties:");
+        ravenwoodProps.forEach((key, origValue) -> {
+            final String value;
+
+            // If a value starts with "$$$", then this is a reference to the device-side value.
+            if (origValue.startsWith("$$$")) {
+                var deviceKey = origValue.substring(3);
+                var deviceValue = deviceProps.get(deviceKey);
+                if (deviceValue == null) {
+                    throw new RuntimeException("Failed to initialize system properties. Key '"
+                            + deviceKey + "' doesn't exist in the device side build.prop");
+                }
+                value = deviceValue;
+            } else {
+                value = origValue;
+            }
+            Log.i(TAG, key + "=" + value);
+            sDefaultValues.put(key, value);
+        });
+
+        // Copy ro.product.* and ro.build.* to all partitions, just in case
+        // We don't want to log these because these are just a lot of duplicate values
+        for (var entry : Set.copyOf(sDefaultValues.entrySet())) {
+            var key = entry.getKey();
+            if (key.startsWith("ro.product.") || key.startsWith("ro.build.")) {
+                var name = key.substring(3);
+                for (String partition : PARTITIONS) {
+                    var newKey = "ro." + partition + "." + name;
+                    if (!sDefaultValues.containsKey(newKey)) {
+                        sDefaultValues.put(newKey, entry.getValue());
+                    }
+                }
+            }
+        }
+
+        if (RAVENWOOD_VERBOSE_LOGGING) {
+            // Dump all properties for local debugging.
+            Log.v(TAG, "All system properties:");
+            for (var key : sDefaultValues.keySet().stream().sorted().toList()) {
+                Log.v(TAG, "" + key + "=" + sDefaultValues.get(key));
+            }
+        }
+
+        // Actually set the system properties
+        sDefaultValues.forEach(RavenwoodRuntimeNative::setSystemProperty);
+    }
+
+    private static boolean isKeyReadable(String key) {
+        final String root = getKeyRoot(key);
+
+        if (root.startsWith("debug.")) return true;
+
+        // This set is carefully curated to help identify situations where a test may
+        // accidentally depend on a default value of an obscure property whose owner hasn't
+        // decided how Ravenwood should behave.
+        if (root.startsWith("boot.")) return true;
+        if (root.startsWith("build.")) return true;
+        if (root.startsWith("product.")) return true;
+        if (root.startsWith("soc.")) return true;
+        if (root.startsWith("system.")) return true;
+
+        // For PropertyInvalidatedCache
+        if (root.startsWith("cache_key.")) return true;
+
+        switch (key) {
+            case "gsm.version.baseband":
+            case "no.such.thing":
+            case "qemu.sf.lcd_density":
+            case "ro.bootloader":
+            case "ro.debuggable":
+            case "ro.hardware":
+            case "ro.hw_timeout_multiplier":
+            case "ro.odm.build.media_performance_class":
+            case "ro.sf.lcd_density":
+            case "ro.treble.enabled":
+            case "ro.vndk.version":
+            case "ro.icu.data.path":
+                return true;
+        }
+
+        return false;
+    }
+
+    private static boolean isKeyWritable(String key) {
+        final String root = getKeyRoot(key);
+
+        if (root.startsWith("debug.")) return true;
+
+        // For PropertyInvalidatedCache
+        if (root.startsWith("cache_key.")) return true;
+
+        return false;
+    }
+
+    static boolean isKeyAccessible(String key, boolean write) {
+        return write ? isKeyWritable(key) : isKeyReadable(key);
+    }
+
+    /**
+     * Return the "root" of the given property key, stripping away any modifier prefix such as
+     * {@code ro.} or {@code persist.}.
+     */
+    private static String getKeyRoot(String key) {
+        if (key.startsWith("ro.")) {
+            return key.substring(3);
+        } else if (key.startsWith("persist.")) {
+            return key.substring(8);
+        } else {
+            return key;
+        }
+    }
+}