Integrate system feature codegen into SystemConfig
Add initial hooks from SystemConfig to query against system features
that are compiled into the framework via build flags. The behavior
is as follows:
* System features compiled as available will always be exposed
* System features compiled as unavailable will never be exposed
* Runtime parsing of system features won't override compiled features
Currently, we log a warning if there's a conflict between the set of
compiled and runtime system features, but this will likely be elevated
to an exception after additional hardening and development.
The current codegen is a no-op with respect to this behavior, as
RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS is flagged as off.
A follow-up change will integrate RoSystemFeature queries into:
* ApplicationPackageManager - To bypass binder calls when possible
* SystemServer - For various hasSystemFeature checks
Bug: 203143243
Test: atest FrameworksServicesTests
Flag: build.RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS
Change-Id: I1fcd2f2f86dc5605f33182e88fc98a21b57bf812
diff --git a/core/java/Android.bp b/core/java/Android.bp
index fae411d..ee71a9d 100644
--- a/core/java/Android.bp
+++ b/core/java/Android.bp
@@ -19,6 +19,7 @@
srcs: [
"**/*.java",
"**/*.aidl",
+ ":systemfeatures-gen-srcs",
":framework-nfc-non-updatable-sources",
],
visibility: ["//frameworks/base"],
@@ -597,3 +598,29 @@
}
// protolog end
+
+// Whether to enable read-only system feature codegen.
+gen_readonly_feature_apis = select(release_flag("RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS"), {
+ true: "true",
+ false: "false",
+ default: "false",
+})
+
+// Generates com.android.internal.pm.RoSystemFeatures, optionally compiling in
+// details about fixed system features defined by build flags. When disabled,
+// the APIs are simply passthrough stubs with no meaningful side effects.
+genrule {
+ name: "systemfeatures-gen-srcs",
+ cmd: "$(location systemfeatures-gen-tool) com.android.internal.pm.RoSystemFeatures " +
+ // --readonly=false (default) makes the codegen an effective no-op passthrough API.
+ " --readonly=" + gen_readonly_feature_apis +
+ // For now, only export "android.hardware.type.*" system features APIs.
+ // TODO(b/203143243): Use an intermediate soong var that aggregates all declared
+ // RELEASE_SYSTEM_FEATURE_* declarations into a single arg.
+ " --feature-apis=AUTOMOTIVE,WATCH,TELEVISION,EMBEDDED,PC" +
+ " > $(out)",
+ out: [
+ "RoSystemFeatures.java",
+ ],
+ tools: ["systemfeatures-gen-tool"],
+}
diff --git a/services/core/java/com/android/server/SystemConfig.java b/services/core/java/com/android/server/SystemConfig.java
index d80e40c..8b619a4 100644
--- a/services/core/java/com/android/server/SystemConfig.java
+++ b/services/core/java/com/android/server/SystemConfig.java
@@ -46,6 +46,7 @@
import android.util.Xml;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.pm.RoSystemFeatures;
import com.android.internal.util.XmlUtils;
import com.android.modules.utils.build.UnboundedSdkLevel;
import com.android.server.pm.permission.PermissionAllowlist;
@@ -212,6 +213,30 @@
}
}
+ /**
+ * Utility class for testing interaction with compile-time defined system features.
+ * @hide
+ */
+ @VisibleForTesting
+ public static class Injector {
+ /** Whether a system feature is defined as enabled and available at compile-time. */
+ public boolean isReadOnlySystemEnabledFeature(String featureName, int version) {
+ return Boolean.TRUE.equals(RoSystemFeatures.maybeHasFeature(featureName, version));
+ }
+
+ /** Whether a system feature is defined as disabled and unavailable at compile-time. */
+ public boolean isReadOnlySystemDisabledFeature(String featureName, int version) {
+ return Boolean.FALSE.equals(RoSystemFeatures.maybeHasFeature(featureName, version));
+ }
+
+ /** The full set of system features defined as compile-time enabled and available. */
+ public ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() {
+ return RoSystemFeatures.getReadOnlySystemEnabledFeatures();
+ }
+ }
+
+ private final Injector mInjector;
+
// These are the built-in shared libraries that were read from the
// system configuration files. Keys are the library names; values are
// the individual entries that contain information such as filename
@@ -220,7 +245,7 @@
// These are the features this devices supports that were read from the
// system configuration files.
- final ArrayMap<String, FeatureInfo> mAvailableFeatures = new ArrayMap<>();
+ final ArrayMap<String, FeatureInfo> mAvailableFeatures;
// These are the features which this device doesn't support; the OEM
// partition uses these to opt-out of features from the system image.
@@ -602,12 +627,26 @@
public ArrayMap<String, Integer> getOemDefinedUids() {
return mOemDefinedUids;
}
+
/**
* Only use for testing. Do NOT use in production code.
* @param readPermissions false to create an empty SystemConfig; true to read the permissions.
*/
@VisibleForTesting
public SystemConfig(boolean readPermissions) {
+ this(readPermissions, new Injector());
+ }
+
+ /**
+ * Only use for testing. Do NOT use in production code.
+ * @param readPermissions false to create an empty SystemConfig; true to read the permissions.
+ * @param injector Additional dependency injection for testing.
+ */
+ @VisibleForTesting
+ public SystemConfig(boolean readPermissions, Injector injector) {
+ mInjector = injector;
+ mAvailableFeatures = mInjector.getReadOnlySystemEnabledFeatures();
+
if (readPermissions) {
Slog.w(TAG, "Constructing a test SystemConfig");
readAllPermissions();
@@ -617,6 +656,9 @@
}
SystemConfig() {
+ mInjector = new Injector();
+ mAvailableFeatures = mInjector.getReadOnlySystemEnabledFeatures();
+
TimingsTraceLog log = new TimingsTraceLog(TAG, Trace.TRACE_TAG_SYSTEM_SERVER);
log.traceBegin("readAllPermissions");
try {
@@ -1777,6 +1819,10 @@
}
private void addFeature(String name, int version) {
+ if (mInjector.isReadOnlySystemDisabledFeature(name, version)) {
+ Slog.w(TAG, "Skipping feature addition for compile-time disabled feature: " + name);
+ return;
+ }
FeatureInfo fi = mAvailableFeatures.get(name);
if (fi == null) {
fi = new FeatureInfo();
@@ -1789,6 +1835,10 @@
}
private void removeFeature(String name) {
+ if (mInjector.isReadOnlySystemEnabledFeature(name, /*version=*/0)) {
+ Slog.w(TAG, "Skipping feature removal for compile-time enabled feature: " + name);
+ return;
+ }
if (mAvailableFeatures.remove(name) != null) {
Slog.d(TAG, "Removed unavailable feature " + name);
}
diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt
new file mode 100644
index 0000000..22d894a
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt
@@ -0,0 +1,167 @@
+/*
+ * 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 com.android.server.systemconfig
+
+import android.content.Context
+import android.content.pm.FeatureInfo
+import android.util.ArrayMap
+import android.util.Xml
+
+import androidx.test.filters.SmallTest
+import androidx.test.platform.app.InstrumentationRegistry
+import com.android.server.SystemConfig
+import com.google.common.truth.Truth.assertThat
+import org.junit.runner.RunWith
+import org.junit.Rule
+
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import org.junit.runners.JUnit4
+
+@SmallTest
+@RunWith(JUnit4::class)
+class SystemConfigReadOnlyFeaturesTest {
+
+ companion object {
+ private const val FEATURE_ONE = "feature.test.1"
+ private const val FEATURE_TWO = "feature.test.2"
+ private const val FEATURE_RUNTIME_AVAILABLE_TEMPLATE =
+ """
+ <permissions>
+ <feature name="%s" />
+ </permissions>
+ """
+ private const val FEATURE_RUNTIME_DISABLED_TEMPLATE =
+ """
+ <permissions>
+ <Disabled-feature name="%s" />
+ </permissions>
+ """
+
+ fun featureInfo(featureName: String) = FeatureInfo().apply { name = featureName }
+ }
+
+ private val context: Context = InstrumentationRegistry.getInstrumentation().context
+
+ @get:Rule
+ val tempFolder = TemporaryFolder(context.filesDir)
+
+ private val injector = TestInjector()
+
+ private var uniqueCounter = 0
+
+ @Test
+ fun empty() {
+ assertFeatures().isEmpty()
+ }
+
+ @Test
+ fun readOnlyEnabled() {
+ addReadOnlyEnabledFeature(FEATURE_ONE)
+ addReadOnlyEnabledFeature(FEATURE_TWO)
+
+ assertFeatures().containsAtLeast(FEATURE_ONE, FEATURE_TWO)
+ }
+
+ @Test
+ fun readOnlyAndRuntimeEnabled() {
+ addReadOnlyEnabledFeature(FEATURE_ONE)
+ addRuntimeEnabledFeature(FEATURE_TWO)
+
+ // No issues with matching availability.
+ assertFeatures().containsAtLeast(FEATURE_ONE, FEATURE_TWO)
+ }
+
+ @Test
+ fun readOnlyEnabledRuntimeDisabled() {
+ addReadOnlyEnabledFeature(FEATURE_ONE)
+ addRuntimeDisabledFeature(FEATURE_ONE)
+
+ // Read-only feature availability should take precedence.
+ assertFeatures().contains(FEATURE_ONE)
+ }
+
+ @Test
+ fun readOnlyDisabled() {
+ addReadOnlyDisabledFeature(FEATURE_ONE)
+
+ assertFeatures().doesNotContain(FEATURE_ONE)
+ }
+
+ @Test
+ fun readOnlyAndRuntimeDisabled() {
+ addReadOnlyDisabledFeature(FEATURE_ONE)
+ addRuntimeDisabledFeature(FEATURE_ONE)
+
+ // No issues with matching (un)availability.
+ assertFeatures().doesNotContain(FEATURE_ONE)
+ }
+
+ @Test
+ fun readOnlyDisabledRuntimeEnabled() {
+ addReadOnlyDisabledFeature(FEATURE_ONE)
+ addRuntimeEnabledFeature(FEATURE_ONE)
+ addRuntimeEnabledFeature(FEATURE_TWO)
+
+ // Read-only feature (un)availability should take precedence.
+ assertFeatures().doesNotContain(FEATURE_ONE)
+ assertFeatures().contains(FEATURE_TWO)
+ }
+
+ fun addReadOnlyEnabledFeature(featureName: String) {
+ injector.readOnlyEnabledFeatures[featureName] = featureInfo(featureName)
+ }
+
+ fun addReadOnlyDisabledFeature(featureName: String) {
+ injector.readOnlyDisabledFeatures.add(featureName)
+ }
+
+ fun addRuntimeEnabledFeature(featureName: String) {
+ FEATURE_RUNTIME_AVAILABLE_TEMPLATE.format(featureName).write()
+ }
+
+ fun addRuntimeDisabledFeature(featureName: String) {
+ FEATURE_RUNTIME_DISABLED_TEMPLATE.format(featureName).write()
+ }
+
+ private fun String.write() = tempFolder.root.resolve("${uniqueCounter++}.xml")
+ .writeText(this.trimIndent())
+
+ private fun assertFeatures() = assertThat(availableFeatures().keys)
+
+ private fun availableFeatures() = SystemConfig(false, injector).apply {
+ val parser = Xml.newPullParser()
+ readPermissions(parser, tempFolder.root, /*Grant all permission flags*/ 0.inv())
+ }.let { it.availableFeatures }
+
+ internal class TestInjector() : SystemConfig.Injector() {
+ val readOnlyEnabledFeatures = ArrayMap<String, FeatureInfo>()
+ val readOnlyDisabledFeatures = mutableSetOf<String>()
+
+ override fun isReadOnlySystemEnabledFeature(featureName: String, version: Int): Boolean {
+ return readOnlyEnabledFeatures.containsKey(featureName)
+ }
+
+ override fun isReadOnlySystemDisabledFeature(featureName: String, version: Int): Boolean {
+ return readOnlyDisabledFeatures.contains(featureName)
+ }
+
+ override fun getReadOnlySystemEnabledFeatures(): ArrayMap<String, FeatureInfo> {
+ return readOnlyEnabledFeatures
+ }
+ }
+}
diff --git a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
index cba521e..196b5e7 100644
--- a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
+++ b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt
@@ -22,8 +22,6 @@
import com.squareup.javapoet.MethodSpec
import com.squareup.javapoet.ParameterizedTypeName
import com.squareup.javapoet.TypeSpec
-import java.util.HashMap
-import java.util.Map
import javax.lang.model.element.Modifier
/*
@@ -52,7 +50,7 @@
* public static boolean hasFeatureAutomotive(Context context);
* public static boolean hasFeatureLeanback(Context context);
* public static Boolean maybeHasFeature(String feature, int version);
- * public static ArrayMap<String, FeatureInfo> getCompileTimeAvailableFeatures();
+ * public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures();
* }
* </pre>
*/
@@ -63,6 +61,7 @@
private val PACKAGEMANAGER_CLASS = ClassName.get("android.content.pm", "PackageManager")
private val CONTEXT_CLASS = ClassName.get("android.content", "Context")
private val FEATUREINFO_CLASS = ClassName.get("android.content.pm", "FeatureInfo")
+ private val ARRAYMAP_CLASS = ClassName.get("android.util", "ArrayMap")
private val ASSUME_TRUE_CLASS =
ClassName.get("com.android.aconfig.annotations", "AssumeTrueForR8")
private val ASSUME_FALSE_CLASS =
@@ -291,19 +290,19 @@
features: Collection<FeatureInfo>,
) {
val methodBuilder =
- MethodSpec.methodBuilder("getCompileTimeAvailableFeatures")
+ MethodSpec.methodBuilder("getReadOnlySystemEnabledFeatures")
.addModifiers(Modifier.PUBLIC, Modifier.STATIC)
.addAnnotation(ClassName.get("android.annotation", "NonNull"))
.addJavadoc("Gets features marked as available at compile-time, keyed by name." +
"\n\n@hide")
.returns(ParameterizedTypeName.get(
- ClassName.get(Map::class.java),
+ ARRAYMAP_CLASS,
ClassName.get(String::class.java),
FEATUREINFO_CLASS))
val availableFeatures = features.filter { it.readonly && it.version != null }
- methodBuilder.addStatement("Map<String, FeatureInfo> features = new \$T<>(\$L)",
- HashMap::class.java, availableFeatures.size)
+ methodBuilder.addStatement("\$T<String, FeatureInfo> features = new \$T<>(\$L)",
+ ARRAYMAP_CLASS, ARRAYMAP_CLASS, availableFeatures.size)
if (!availableFeatures.isEmpty()) {
methodBuilder.addStatement("FeatureInfo fi = new FeatureInfo()")
}
diff --git a/tools/systemfeatures/tests/golden/RoFeatures.java.gen b/tools/systemfeatures/tests/golden/RoFeatures.java.gen
index edbfc42..ee97b26 100644
--- a/tools/systemfeatures/tests/golden/RoFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RoFeatures.java.gen
@@ -13,10 +13,9 @@
import android.content.Context;
import android.content.pm.FeatureInfo;
import android.content.pm.PackageManager;
+import android.util.ArrayMap;
import com.android.aconfig.annotations.AssumeFalseForR8;
import com.android.aconfig.annotations.AssumeTrueForR8;
-import java.util.HashMap;
-import java.util.Map;
/**
* @hide
@@ -94,8 +93,8 @@
* @hide
*/
@NonNull
- public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() {
- Map<String, FeatureInfo> features = new HashMap<>(2);
+ public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() {
+ ArrayMap<String, FeatureInfo> features = new ArrayMap<>(2);
FeatureInfo fi = new FeatureInfo();
fi.name = PackageManager.FEATURE_WATCH;
fi.version = 1;
diff --git a/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen b/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen
index bf7a006..40c7db7 100644
--- a/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen
@@ -9,8 +9,7 @@
import android.content.Context;
import android.content.pm.FeatureInfo;
import android.content.pm.PackageManager;
-import java.util.HashMap;
-import java.util.Map;
+import android.util.ArrayMap;
/**
* @hide
@@ -43,8 +42,8 @@
* @hide
*/
@NonNull
- public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() {
- Map<String, FeatureInfo> features = new HashMap<>(0);
+ public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() {
+ ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0);
return features;
}
}
diff --git a/tools/systemfeatures/tests/golden/RwFeatures.java.gen b/tools/systemfeatures/tests/golden/RwFeatures.java.gen
index b20b228..7bf8961 100644
--- a/tools/systemfeatures/tests/golden/RwFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RwFeatures.java.gen
@@ -12,8 +12,7 @@
import android.content.Context;
import android.content.pm.FeatureInfo;
import android.content.pm.PackageManager;
-import java.util.HashMap;
-import java.util.Map;
+import android.util.ArrayMap;
/**
* @hide
@@ -73,8 +72,8 @@
* @hide
*/
@NonNull
- public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() {
- Map<String, FeatureInfo> features = new HashMap<>(0);
+ public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() {
+ ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0);
return features;
}
}
diff --git a/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen b/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen
index d91f5b6..eb7ec63 100644
--- a/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen
+++ b/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen
@@ -7,8 +7,7 @@
import android.annotation.Nullable;
import android.content.Context;
import android.content.pm.FeatureInfo;
-import java.util.HashMap;
-import java.util.Map;
+import android.util.ArrayMap;
/**
* @hide
@@ -32,8 +31,8 @@
* @hide
*/
@NonNull
- public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() {
- Map<String, FeatureInfo> features = new HashMap<>(0);
+ public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() {
+ ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0);
return features;
}
}
diff --git a/tools/systemfeatures/tests/src/ArrayMap.java b/tools/systemfeatures/tests/src/ArrayMap.java
new file mode 100644
index 0000000..a5ed9b0
--- /dev/null
+++ b/tools/systemfeatures/tests/src/ArrayMap.java
@@ -0,0 +1,26 @@
+/*
+ * 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.util;
+
+import java.util.HashMap;
+
+/** Stub for testing. */
+public final class ArrayMap<K, V> extends HashMap<K, V> {
+ public ArrayMap(int capacity) {
+ super(capacity);
+ }
+}
diff --git a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
index 39f8fc4..ed3f5c9 100644
--- a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
+++ b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java
@@ -60,7 +60,7 @@
assertThat(RwNoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull();
assertThat(RwNoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();
assertThat(RwNoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();
- assertThat(RwNoFeatures.getCompileTimeAvailableFeatures()).isEmpty();
+ assertThat(RwNoFeatures.getReadOnlySystemEnabledFeatures()).isEmpty();
}
@Test
@@ -72,7 +72,7 @@
assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull();
assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();
assertThat(RoNoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();
- assertThat(RoNoFeatures.getCompileTimeAvailableFeatures()).isEmpty();
+ assertThat(RoNoFeatures.getReadOnlySystemEnabledFeatures()).isEmpty();
// Also ensure we fall back to the PackageManager for feature APIs without an accompanying
// versioned feature definition.
@@ -106,7 +106,7 @@
assertThat(RwFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull();
assertThat(RwFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();
assertThat(RwFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();
- assertThat(RwFeatures.getCompileTimeAvailableFeatures()).isEmpty();
+ assertThat(RwFeatures.getReadOnlySystemEnabledFeatures()).isEmpty();
}
@Test
@@ -163,7 +163,7 @@
assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 100)).isNull();
assertThat(RoFeatures.maybeHasFeature("", 0)).isNull();
- Map<String, FeatureInfo> compiledFeatures = RoFeatures.getCompileTimeAvailableFeatures();
+ Map<String, FeatureInfo> compiledFeatures = RoFeatures.getReadOnlySystemEnabledFeatures();
assertThat(compiledFeatures.keySet())
.containsExactly(PackageManager.FEATURE_WATCH, PackageManager.FEATURE_WIFI);
assertThat(compiledFeatures.get(PackageManager.FEATURE_WATCH).version).isEqualTo(1);