diff options
4 files changed, 104 insertions, 37 deletions
diff --git a/tools/systemfeatures/Android.bp b/tools/systemfeatures/Android.bp index 2cebfe9790d0..aca25eb8f603 100644 --- a/tools/systemfeatures/Android.bp +++ b/tools/systemfeatures/Android.bp @@ -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", diff --git a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt index 9bfda451067f..e537ffcb56bd 100644 --- a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt +++ b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt @@ -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,19 +197,17 @@ object SystemFeaturesGenerator { builder.addMethod(methodBuilder.build()) } - if (!readonly) { - 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)" - ) - .build() - ) - } + // 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)") + .build() + ) } /* @@ -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,16 +232,27 @@ object SystemFeaturesGenerator { .addParameter(String::class.java, "featureName") .addParameter(Int::class.java, "version") - if (readonly) { - methodBuilder.beginControlFlow("switch (featureName)") - for (feature in features) { - methodBuilder.addCode("case \$T.\$N: ", PACKAGEMANAGER_CLASS, feature.name) - if (feature.version != null) { - methodBuilder.addStatement("return \$L >= version", feature.version) - } else { - methodBuilder.addStatement("return false") - } + 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) + } else { + 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) } diff --git a/tools/systemfeatures/tests/PackageManager.java b/tools/systemfeatures/tests/PackageManager.java index 645d500bc762..db670482065a 100644 --- a/tools/systemfeatures/tests/PackageManager.java +++ b/tools/systemfeatures/tests/PackageManager.java @@ -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"; diff --git a/tools/systemfeatures/tests/SystemFeaturesGeneratorTest.java b/tools/systemfeatures/tests/SystemFeaturesGeneratorTest.java index 547d2cbd26f9..6dfd244a807b 100644 --- a/tools/systemfeatures/tests/SystemFeaturesGeneratorTest.java +++ b/tools/systemfeatures/tests/SystemFeaturesGeneratorTest.java @@ -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(); |