Merge "Decouple system feature API codegen from feature definitions" into main (ea87f325) · Commits · e / os / android_frameworks_base

tools/systemfeatures/Android.bp

+2 −2

Original line number	Diff line number	Diff line
		@@ -30,9 +30,9 @@ java_binary_host {
		genrule {
		name: "systemfeatures-gen-tests-srcs",
		cmd: "$(location systemfeatures-gen-tool) com.android.systemfeatures.RwNoFeatures --readonly=false > $(location RwNoFeatures.java) && " +
		"$(location systemfeatures-gen-tool) com.android.systemfeatures.RoNoFeatures --readonly=true > $(location RoNoFeatures.java) && " +
		"$(location systemfeatures-gen-tool) com.android.systemfeatures.RoNoFeatures --readonly=true --feature-apis=WATCH > $(location RoNoFeatures.java) && " +
		"$(location systemfeatures-gen-tool) com.android.systemfeatures.RwFeatures --readonly=false --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: > $(location RwFeatures.java) && " +
		"$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: > $(location RoFeatures.java)",
		"$(location systemfeatures-gen-tool) com.android.systemfeatures.RoFeatures --readonly=true --feature=WATCH:1 --feature=WIFI:0 --feature=VULKAN:-1 --feature=AUTO: --feature-apis=WATCH,PC > $(location RoFeatures.java)",
		out: [
		"RwNoFeatures.java",
		"RoNoFeatures.java",

tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt

+84 −35

Original line number	Diff line number	Diff line
		@@ -32,6 +32,7 @@ import javax.lang.model.element.Modifier
		* <pre>
		* <cmd> com.foo.RoSystemFeatures --readonly=true \
		* --feature=WATCH:0 --feature=AUTOMOTIVE: --feature=VULKAN:9348
		* --feature-apis=WATCH,PC,LEANBACK
		* </pre>
		*
		* This generates a class that has the following signature:
		@@ -45,12 +46,15 @@ import javax.lang.model.element.Modifier
		* public static boolean hasFeatureAutomotive(Context context);
		* @AssumeTrueForR8
		* public static boolean hasFeatureVulkan(Context context);
		* public static boolean hasFeaturePc(Context context);
		* public static boolean hasFeatureLeanback(Context context);
		* public static Boolean maybeHasFeature(String feature, int version);
		* }
		* </pre>
		*/
		object SystemFeaturesGenerator {
		private const val FEATURE_ARG = "--feature="
		private const val FEATURE_APIS_ARG = "--feature-apis="
		private const val READONLY_ARG = "--readonly="
		private val PACKAGEMANAGER_CLASS = ClassName.get("android.content.pm", "PackageManager")
		private val CONTEXT_CLASS = ClassName.get("android.content", "Context")
		@@ -64,6 +68,15 @@ object SystemFeaturesGenerator {
		println(" Options:")
		println(" --readonly=true\|false Whether to encode features as build-time constants")
		println(" --feature=\$NAME:\$VER A feature+version pair (blank version == disabled)")
		println(" This will always generate associated query APIs,")
		println(" adding to or replacing those from `--feature-apis=`.")
		println(" --feature-apis=\$NAME_1,\$NAME_2")
		println(" A comma-separated set of features for which to always")
		println(" generate named query APIs. If a feature in this set is")
		println(" not explicitly defined via `--feature=`, then a simple")
		println(" runtime passthrough API will be generated, regardless")
		println(" of the `--readonly` flag. This allows decoupling the")
		println(" API surface from variations in device feature sets.")
		}

		/** Main entrypoint for build-time system feature codegen. */
		@@ -76,18 +89,42 @@ object SystemFeaturesGenerator {

		var readonly = false
		var outputClassName: ClassName? = null
		val features = mutableListOf<FeatureInfo>()
		val featureArgs = mutableListOf<FeatureArg>()
		// We could just as easily hardcode this list, as the static API surface should change
		// somewhat infrequently, but this decouples the codegen from the framework completely.
		val featureApiArgs = mutableSetOf<String>()
		for (arg in args) {
		when {
		arg.startsWith(READONLY_ARG) ->
		readonly = arg.substring(READONLY_ARG.length).toBoolean()
		arg.startsWith(FEATURE_ARG) -> {
		features.add(parseFeatureArg(arg))
		featureArgs.add(parseFeatureArg(arg))
		}
		arg.startsWith(FEATURE_APIS_ARG) -> {
		featureApiArgs.addAll(
		arg.substring(FEATURE_APIS_ARG.length).split(",").map {
		parseFeatureName(it)
		}
		)
		}
		else -> outputClassName = ClassName.bestGuess(arg)
		}
		}

		// First load in all of the feature APIs we want to generate. Explicit feature definitions
		// will then override this set with the appropriate readonly and version value.
		val features = mutableMapOf<String, FeatureInfo>()
		featureApiArgs.associateByTo(
		features,
		{ it },
		{ FeatureInfo(it, version = null, readonly = false) },
		)
		featureArgs.associateByTo(
		features,
		{ it.name },
		{ FeatureInfo(it.name, it.version, readonly) },
		)

		outputClassName
		?: run {
		println("Output class name must be provided.")
		@@ -100,8 +137,8 @@ object SystemFeaturesGenerator {
		.addModifiers(Modifier.PUBLIC, Modifier.FINAL)
		.addJavadoc("@hide")

		addFeatureMethodsToClass(classBuilder, readonly, features)
		addMaybeFeatureMethodToClass(classBuilder, readonly, features)
		addFeatureMethodsToClass(classBuilder, features.values)
		addMaybeFeatureMethodToClass(classBuilder, features.values)

		// TODO(b/203143243): Add validation of build vs runtime values to ensure consistency.
		JavaFile.builder(outputClassName.packageName(), classBuilder.build())
		@@ -115,13 +152,16 @@ object SystemFeaturesGenerator {
		* * "--feature=WATCH:7" -> Feature enabled w/ version 7
		* * "--feature=WATCH:" -> Feature disabled
		*/
		private fun parseFeatureArg(arg: String): FeatureInfo {
		private fun parseFeatureArg(arg: String): FeatureArg {
		val featureArgs = arg.substring(FEATURE_ARG.length).split(":")
		val name = featureArgs[0].let { if (!it.startsWith("FEATURE_")) "FEATURE_$it" else it }
		val name = parseFeatureName(featureArgs[0])
		val version = featureArgs.getOrNull(1)?.toIntOrNull()
		return FeatureInfo(name, version)
		return FeatureArg(name, version)
		}

		private fun parseFeatureName(name: String): String =
		if (name.startsWith("FEATURE_")) name else "FEATURE_$name"

		/*
		* Adds per-feature query methods to the class with the form:
		* {@code public static boolean hasFeatureX(Context context)},
		@@ -129,8 +169,7 @@ object SystemFeaturesGenerator {
		*/
		private fun addFeatureMethodsToClass(
		builder: TypeSpec.Builder,
		readonly: Boolean,
		features: List<FeatureInfo>
		features: Collection<FeatureInfo>,
		) {
		for (feature in features) {
		// Turn "FEATURE_FOO" into "hasFeatureFoo".
		@@ -142,7 +181,7 @@ object SystemFeaturesGenerator {
		.returns(Boolean::class.java)
		.addParameter(CONTEXT_CLASS, "context")

		if (readonly) {
		if (feature.readonly) {
		val featureEnabled = compareValues(feature.version, 0) >= 0
		methodBuilder.addAnnotation(
		if (featureEnabled) ASSUME_TRUE_CLASS else ASSUME_FALSE_CLASS
		@@ -158,20 +197,18 @@ object SystemFeaturesGenerator {
		builder.addMethod(methodBuilder.build())
		}

		if (!readonly) {
		// This is a trivial method, even if unused based on readonly-codegen, it does little harm
		// to always include it.
		builder.addMethod(
		MethodSpec.methodBuilder("hasFeatureFallback")
		.addModifiers(Modifier.PRIVATE, Modifier.STATIC)
		.returns(Boolean::class.java)
		.addParameter(CONTEXT_CLASS, "context")
		.addParameter(String::class.java, "featureName")
		.addStatement(
		"return context.getPackageManager().hasSystemFeature(featureName, 0)"
		)
		.addStatement("return context.getPackageManager().hasSystemFeature(featureName, 0)")
		.build()
		)
		}
		}

		/*
		* Adds a generic query method to the class with the form: {@code public static boolean
		@@ -185,8 +222,7 @@ object SystemFeaturesGenerator {
		*/
		private fun addMaybeFeatureMethodToClass(
		builder: TypeSpec.Builder,
		readonly: Boolean,
		features: List<FeatureInfo>
		features: Collection<FeatureInfo>,
		) {
		val methodBuilder =
		MethodSpec.methodBuilder("maybeHasFeature")
		@@ -196,9 +232,19 @@ object SystemFeaturesGenerator {
		.addParameter(String::class.java, "featureName")
		.addParameter(Int::class.java, "version")

		if (readonly) {
		methodBuilder.beginControlFlow("switch (featureName)")
		var hasSwitchBlock = false
		for (feature in features) {
		// We only return non-null results for queries against readonly-defined features.
		if (!feature.readonly) {
		continue
		}
		if (!hasSwitchBlock) {
		// As an optimization, only create the switch block if needed. Even an empty
		// switch-on-string block can induce a hash, which we can avoid if readonly
		// support is completely disabled.
		hasSwitchBlock = true
		methodBuilder.beginControlFlow("switch (featureName)")
		}
		methodBuilder.addCode("case \$T.\$N: ", PACKAGEMANAGER_CLASS, feature.name)
		if (feature.version != null) {
		methodBuilder.addStatement("return \$L >= version", feature.version)
		@@ -206,6 +252,7 @@ object SystemFeaturesGenerator {
		methodBuilder.addStatement("return false")
		}
		}
		if (hasSwitchBlock) {
		methodBuilder.addCode("default: ")
		methodBuilder.addStatement("break")
		methodBuilder.endControlFlow()
		@@ -214,5 +261,7 @@ object SystemFeaturesGenerator {
		builder.addMethod(methodBuilder.build())
		}

		private data class FeatureInfo(val name: String, val version: Int?)
		private data class FeatureArg(val name: String, val version: Int?)

		private data class FeatureInfo(val name: String, val version: Int?, val readonly: Boolean)
		}

tools/systemfeatures/tests/PackageManager.java

+1 −0

Original line number	Diff line number	Diff line
		@@ -19,6 +19,7 @@ package android.content.pm;
		/** Stub for testing */
		public class PackageManager {
		public static final String FEATURE_AUTO = "automotive";
		public static final String FEATURE_PC = "pc";
		public static final String FEATURE_VULKAN = "vulkan";
		public static final String FEATURE_WATCH = "watch";
		public static final String FEATURE_WIFI = "wifi";

tools/systemfeatures/tests/SystemFeaturesGeneratorTest.java

+17 −0

Original line number	Diff line number	Diff line
		@@ -68,6 +68,13 @@ public class SystemFeaturesGeneratorTest {
		assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull();
		assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull();
		assertThat(RoNoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();

		// Also ensure we fall back to the PackageManager for feature APIs without an accompanying
		// versioned feature definition.
		when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_WATCH, 0)).thenReturn(true);
		assertThat(RwFeatures.hasFeatureWatch(mContext)).isTrue();
		when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_WATCH, 0)).thenReturn(false);
		assertThat(RwFeatures.hasFeatureWatch(mContext)).isFalse();
		}

		@Test
		@@ -127,6 +134,16 @@ public class SystemFeaturesGeneratorTest {
		assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isFalse();
		assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 100)).isFalse();

		// For feature APIs without an associated feature definition, conditional queries should
		// report null, and explicit queries should report runtime-defined versions.
		when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_PC, 0)).thenReturn(true);
		assertThat(RoFeatures.hasFeaturePc(mContext)).isTrue();
		when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_PC, 0)).thenReturn(false);
		assertThat(RoFeatures.hasFeaturePc(mContext)).isFalse();
		assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_PC, -1)).isNull();
		assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_PC, 0)).isNull();
		assertThat(RoFeatures.maybeHasFeature(PackageManager.FEATURE_PC, 100)).isNull();

		// For undefined types, conditional queries should report null (unknown).
		assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", -1)).isNull();
		assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull();