diff options
18 files changed, 745 insertions, 17 deletions
diff --git a/apct-tests/perftests/aconfig/Android.bp b/apct-tests/perftests/aconfig/Android.bp new file mode 100644 index 000000000000..715923de1eb7 --- /dev/null +++ b/apct-tests/perftests/aconfig/Android.bp @@ -0,0 +1,39 @@ +// 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 { + default_team: "trendy_team_android_core_experiments", + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_base_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_base_license"], +} + +android_test { + name: "AconfigPerfTests", + srcs: ["src/**/*.java"], + static_libs: [ + "aconfig_device_paths_java_util", + "androidx.test.rules", + "apct-perftests-utils", + "collector-device-lib", + "truth", + ], + platform_apis: true, + certificate: "platform", + test_suites: ["device-tests"], + data: [":perfetto_artifacts"], +} diff --git a/apct-tests/perftests/aconfig/AndroidManifest.xml b/apct-tests/perftests/aconfig/AndroidManifest.xml new file mode 100644 index 000000000000..e9d7c176303f --- /dev/null +++ b/apct-tests/perftests/aconfig/AndroidManifest.xml @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2020 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. +--> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.perftests.aconfig"> + + <application> + <uses-library android:name="android.test.runner" /> + </application> + + <instrumentation android:name="androidx.test.runner.AndroidJUnitRunner" + android:targetPackage="com.android.perftests.aconfig"/> + +</manifest>
\ No newline at end of file diff --git a/apct-tests/perftests/aconfig/AndroidTest.xml b/apct-tests/perftests/aconfig/AndroidTest.xml new file mode 100644 index 000000000000..036e0310def2 --- /dev/null +++ b/apct-tests/perftests/aconfig/AndroidTest.xml @@ -0,0 +1,63 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright (C) 2018 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. +--> +<configuration description="Runs AconfigPerfTests metric instrumentation."> + <option name="test-suite-tag" value="apct" /> + <option name="test-suite-tag" value="apct-metric-instrumentation" /> + + <target_preparer class="com.android.tradefed.targetprep.suite.SuiteApkInstaller"> + <option name="cleanup-apks" value="true" /> + <option name="test-file-name" value="AconfigPerfTests.apk" /> + </target_preparer> + + <!-- Needed for pushing the trace config file --> + <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer"/> + <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer"> + <option name="push-file" key="trace_config_detailed.textproto" value="/data/misc/perfetto-traces/trace_config.textproto" /> + </target_preparer> + + + <!-- Needed for pulling the collected trace config on to the host --> + <metrics_collector class="com.android.tradefed.device.metric.FilePullerLogCollector"> + <option name="pull-pattern-keys" value="perfetto_file_path" /> + </metrics_collector> + + <!-- Needed for storing the perfetto trace files in the sdcard/test_results--> + <option name="isolated-storage" value="false" /> + + <test class="com.android.tradefed.testtype.AndroidJUnitTest" > + <option name="package" value="com.android.perftests.aconfig" /> + <option name="hidden-api-checks" value="false"/> + + <!-- Listener related args for collecting the traces and waiting for the device to stabilize. --> + <option name="device-listeners" value="android.device.collectors.ProcLoadListener,android.device.collectors.PerfettoListener" /> + <!-- Guarantee that user defined RunListeners will be running before any of the default listeners defined in this runner. --> + <option name="instrumentation-arg" key="newRunListenerMode" value="true" /> + + <!-- ProcLoadListener related arguments --> + <!-- Wait for device last minute threshold to reach 3 with 2 minute timeout before starting the test run --> + <option name="instrumentation-arg" key="procload-collector:per_run" value="true" /> + <option name="instrumentation-arg" key="proc-loadavg-threshold" value="3" /> + <option name="instrumentation-arg" key="proc-loadavg-timeout" value="120000" /> + <option name="instrumentation-arg" key="proc-loadavg-interval" value="10000" /> + + <!-- PerfettoListener related arguments --> + <option name="instrumentation-arg" key="perfetto_config_text_proto" value="true" /> + <option name="instrumentation-arg" key="perfetto_config_file" value="trace_config.textproto" /> + + <option name="instrumentation-arg" key="newRunListenerMode" value="true" /> + + </test> +</configuration>
\ No newline at end of file diff --git a/apct-tests/perftests/aconfig/OWNERS b/apct-tests/perftests/aconfig/OWNERS new file mode 100644 index 000000000000..2202076593fb --- /dev/null +++ b/apct-tests/perftests/aconfig/OWNERS @@ -0,0 +1 @@ +file:platform/packages/modules/ConfigInfrastructure:/OWNERS
\ No newline at end of file diff --git a/apct-tests/perftests/aconfig/src/android/os/flagging/AconfigPackagePerfTest.java b/apct-tests/perftests/aconfig/src/android/os/flagging/AconfigPackagePerfTest.java new file mode 100644 index 000000000000..df6e3c836256 --- /dev/null +++ b/apct-tests/perftests/aconfig/src/android/os/flagging/AconfigPackagePerfTest.java @@ -0,0 +1,139 @@ +/* + * Copyright 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.os.flagging; + +import static com.google.common.truth.Truth.assertThat; + +import android.aconfig.DeviceProtosTestUtil; +import android.aconfig.nano.Aconfig.parsed_flag; +import android.perftests.utils.BenchmarkState; +import android.perftests.utils.PerfStatusReporter; + +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +@RunWith(Parameterized.class) +public class AconfigPackagePerfTest { + + @Rule public PerfStatusReporter mPerfStatusReporter = new PerfStatusReporter(); + + @Parameterized.Parameters(name = "isPlatform={0}") + public static Collection<Object[]> data() { + return Arrays.asList(new Object[][] {{false}, {true}}); + } + + private static final Set<String> PLATFORM_CONTAINERS = Set.of("system", "vendor", "product"); + private static List<parsed_flag> sFlags; + + @BeforeClass + public static void init() { + try { + sFlags = DeviceProtosTestUtil.loadAndParseFlagProtos(); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @Parameterized.Parameter(0) + + // if this variable is true, then the test query flags from system/product/vendor + // if this variable is false, then the test query flags from updatable partitions + public boolean mIsPlatform; + + @Test + public void timeAconfigPackageLoadOnePackage() { + String packageName = ""; + for (parsed_flag flag : sFlags) { + if (mIsPlatform == PLATFORM_CONTAINERS.contains(flag.container)) { + packageName = flag.package_; + break; + } + } + BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + while (state.keepRunning()) { + AconfigPackage.load(packageName); + } + } + + @Test + public void timeAconfigPackageLoadMultiplePackages() { + // load num packages + int packageNum = 25; + Set<String> packageSet = new HashSet<>(); + for (parsed_flag flag : sFlags) { + if (mIsPlatform == PLATFORM_CONTAINERS.contains(flag.container)) { + packageSet.add(flag.package_); + } + if (packageSet.size() >= packageNum) { + break; + } + } + List<String> packageList = new ArrayList(packageSet); + assertThat(packageList.size()).isAtLeast(packageNum); + BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + for (int i = 0; state.keepRunning(); i++) { + AconfigPackage.load(packageList.get(i % packageNum)); + } + } + + @Test + public void timeAconfigPackageGetBooleanFlagValue() { + // get one package contains num of flags + int flagNum = 20; + List<parsed_flag> l = findNumFlagsInSamePackage(flagNum, mIsPlatform); + List<String> flagName = new ArrayList<>(); + String packageName = l.get(0).package_; + for (parsed_flag flag : l) { + flagName.add(flag.name); + } + assertThat(flagName.size()).isAtLeast(flagNum); + BenchmarkState state = mPerfStatusReporter.getBenchmarkState(); + AconfigPackage ap = AconfigPackage.load(packageName); + for (int i = 0; state.keepRunning(); i++) { + ap.getBooleanFlagValue(flagName.get(i % flagNum), false); + } + } + + private static List<parsed_flag> findNumFlagsInSamePackage(int num, boolean isPlatform) { + Map<String, List<parsed_flag>> packageToFlag = new HashMap<>(); + List<parsed_flag> ret = new ArrayList<parsed_flag>(); + for (parsed_flag flag : sFlags) { + if (isPlatform == PLATFORM_CONTAINERS.contains(flag.container)) { + ret = + packageToFlag.computeIfAbsent( + flag.package_, k -> new ArrayList<parsed_flag>()); + ret.add(flag); + if (ret.size() >= num) { + break; + } + } + } + return ret; + } +} diff --git a/packages/CrashRecovery/services/module/java/com/android/server/PackageWatchdog.java b/packages/CrashRecovery/services/module/java/com/android/server/PackageWatchdog.java index ef46906f9cf4..e4f07f9fc213 100644 --- a/packages/CrashRecovery/services/module/java/com/android/server/PackageWatchdog.java +++ b/packages/CrashRecovery/services/module/java/com/android/server/PackageWatchdog.java @@ -1364,8 +1364,6 @@ public class PackageWatchdog { Slog.w(TAG, "Failed to save monitored packages, restoring backup", e); mPolicyFile.failWrite(stream); return false; - } finally { - IoUtils.closeQuietly(stream); } } } diff --git a/packages/CrashRecovery/services/platform/java/com/android/server/PackageWatchdog.java b/packages/CrashRecovery/services/platform/java/com/android/server/PackageWatchdog.java index 4a00ed3d1d46..5297c8b61aad 100644 --- a/packages/CrashRecovery/services/platform/java/com/android/server/PackageWatchdog.java +++ b/packages/CrashRecovery/services/platform/java/com/android/server/PackageWatchdog.java @@ -1372,8 +1372,6 @@ public class PackageWatchdog { Slog.w(TAG, "Failed to save monitored packages, restoring backup", e); mPolicyFile.failWrite(stream); return false; - } finally { - IoUtils.closeQuietly(stream); } } } diff --git a/tools/systemfeatures/Android.bp b/tools/systemfeatures/Android.bp index 2ebede39504e..87ea5db57db4 100644 --- a/tools/systemfeatures/Android.bp +++ b/tools/systemfeatures/Android.bp @@ -100,3 +100,72 @@ sh_test_host { unit_test: true, }, } + +java_library_host { + name: "systemfeatures-errorprone-lib", + srcs: [ + ":systemfeatures-gen-metadata-srcs", + "errorprone/java/**/*.java", + ], + static_libs: [ + "//external/error_prone:error_prone_core", + "guava", + "jsr305", + ], + libs: [ + "//external/auto:auto_service_annotations", + ], + javacflags: [ + // These exports are needed because this errorprone plugin access some private classes + // of the java compiler. + "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + ], + plugins: [ + "//external/auto:auto_service_plugin", + ], +} + +java_plugin { + name: "systemfeatures-errorprone", + static_libs: ["systemfeatures-errorprone-lib"], +} + +java_test_host { + name: "systemfeatures-errorprone-tests", + srcs: [ + "errorprone/tests/java/**/*.java", + ], + java_resource_dirs: ["tests/src"], + java_resources: [ + ":systemfeatures-errorprone-tests-data", + ], + static_libs: [ + "compile-testing-prebuilt", + "error_prone_test_helpers", + "framework-annotations-lib", + "hamcrest", + "hamcrest-library", + "junit", + "systemfeatures-errorprone-lib", + "truth", + ], + test_options: { + unit_test: true, + }, +} + +java_system_features_srcs { + name: "systemfeatures-gen-metadata-srcs", + full_class_name: "com.android.systemfeatures.RoSystemFeaturesMetadata", + metadata_only: true, + visibility: ["//visibility:private"], +} + +filegroup { + name: "systemfeatures-errorprone-tests-data", + path: "tests/src", + srcs: ["tests/src/android/**/*.java"], + visibility: ["//visibility:private"], +} diff --git a/tools/systemfeatures/README.md b/tools/systemfeatures/README.md index 5836f81e5fd3..b1fec1a34723 100644 --- a/tools/systemfeatures/README.md +++ b/tools/systemfeatures/README.md @@ -4,8 +4,110 @@ System features exposed from `PackageManager` are defined and aggregated as `<feature>` xml attributes across various partitions, and are currently queried -at runtime through the framework. This directory contains tooling that will -support *build-time* queries of select system features, enabling optimizations +at runtime through the framework. This directory contains tooling that supports +*build-time* queries of select system features, enabling optimizations like code stripping and conditionally dependencies when so configured. -### TODO(b/203143243): Expand readme after landing codegen. +### System Feature Codegen + +As not all system features can be fully specified or defined at build time (e.g. +updatable partitisions and apex modules can change/remove such features), we +use a conditional, build flag approach that allows a given device to customize +the subset of build-time defined system features that are immutable and cannot +be updated. + +#### Build Flags + +System features that can be fixed at build-time are declared in a common +location, `build/release/flag_declarations/`. These have the form +`RELEASE_SYSTEM_FEATURE_${X}`, where `${X}` corresponds to a feature defined in +`PackageManager`, e.g., `TELEVISION` or `WATCH`. + +Build flag values can then be defined per device (or form factor), where such +values either indicate the existence/version of the system feature, or that the +feature is unavailable, e.g., for TV, we could define these build flag values: +``` +name: "RELEASE_SYSTEM_FEATURE_TELEVISION" +value: { + string_value: "0" # Feature version = 0 +} +``` +``` +name: "RELEASE_SYSTEM_FEATURE_WATCH" +value: { + string_value: "UNAVAILABLE" +} +``` + +See also [SystemFeaturesGenerator](src/com/android/systemfeatures/SystemFeaturesGenerator.kt) +for more details. + +#### Runtime Queries + +Each declared build flag system feature is routed into codegen, generating a +getter API in the internal class, `com.android.internal.pm.RoSystemFeatures`: +``` +class RoSystemFeatures { + ... + public static boolean hasFeatureX(Context context); + ... +} +``` +By default, these queries simply fall back to the usual +`PackageManager.hasSystemFeature(...)` runtime queries. However, if a device +defines these features via build flags, the generated code will add annotations +indicating fixed value for this query, and adjust the generated code to return +the value directly. This in turn enables build-time stripping and optimization. + +> **_NOTE:_** Any build-time defined system features will also be implicitly +used to accelerate calls to `PackageManager.hasSystemFeature(...)` for the +feature, avoiding binder calls when possible. + +#### Lint + +A new `ErrorProne` rule is introduced to assist with migration and maintenance +of codegen APIs for build-time defined system features. This is defined in the +`systemfeatures-errorprone` build rule, which can be added to any Java target's +`plugins` list. + +// TODO(b/203143243): Add plugin to key system targets after initial migration. + +1) Add the plugin dependency to a given `${TARGET}`: +``` +java_library { + name: "${TARGET}", + plugins: ["systemfeatures-errorprone"], +} +``` +2) Run locally: +``` +RUN_ERROR_PRONE=true m ${TARGET} +``` +3) (Optional) Update the target rule to generate in-place patch files: +``` +java_library { + name: "${TARGET}", + plugins: ["systemfeatures-errorprone"], + // DO NOT SUBMIT: GENERATE IN-PLACE PATCH FILES + errorprone: { + javacflags: [ + "-XepPatchChecks:RoSystemFeaturesChecker", + "-XepPatchLocation:IN_PLACE", + ], + } + ... +} +``` +``` +RUN_ERROR_PRONE=true m ${TARGET} +``` + +See also [RoSystemFeaturesChecker](errorprone/java/com/android/systemfeatures/errorprone/RoSystemFeaturesChecker.java) +for more details. + +> **_NOTE:_** Not all system feature queries or targets need or should be +migrated. Only system features that are explicitly declared with build flags, +and only targets that are built with the platform (i.e., not updatable), are +candidates for this linting and migration, e.g., SystemUI, System Server, etc... + +// TODO(b/203143243): Wrap the in-place lint updates with a simple script for convenience. diff --git a/tools/systemfeatures/errorprone/java/com/android/systemfeatures/errorprone/RoSystemFeaturesChecker.java b/tools/systemfeatures/errorprone/java/com/android/systemfeatures/errorprone/RoSystemFeaturesChecker.java new file mode 100644 index 000000000000..78123774205a --- /dev/null +++ b/tools/systemfeatures/errorprone/java/com/android/systemfeatures/errorprone/RoSystemFeaturesChecker.java @@ -0,0 +1,119 @@ +/* + * 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.systemfeatures.errorprone; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; + +import com.android.systemfeatures.RoSystemFeaturesMetadata; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.tools.javac.code.Symbol; + +@AutoService(BugChecker.class) +@BugPattern( + name = "RoSystemFeaturesChecker", + summary = "Use RoSystemFeature instead of PackageManager.hasSystemFeature", + explanation = + "Directly invoking `PackageManager.hasSystemFeature` is less efficient than using" + + " the `RoSystemFeatures` helper class. This check flags invocations like" + + " `context.getPackageManager().hasSystemFeature(PackageManager.FEATURE_FOO)`" + + " and suggests replacing them with" + + " `com.android.internal.pm.RoSystemFeatures.hasFeatureFoo(context)`.", + severity = WARNING) +public class RoSystemFeaturesChecker extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher { + + private static final String PACKAGE_MANAGER_CLASS = "android.content.pm.PackageManager"; + private static final String CONTEXT_CLASS = "android.content.Context"; + private static final String RO_SYSTEM_FEATURE_SIMPLE_CLASS = "RoSystemFeatures"; + private static final String RO_SYSTEM_FEATURE_CLASS = + "com.android.internal.pm." + RO_SYSTEM_FEATURE_SIMPLE_CLASS; + private static final String GET_PACKAGE_MANAGER_METHOD = "getPackageManager"; + private static final String HAS_SYSTEM_FEATURE_METHOD = "hasSystemFeature"; + private static final String FEATURE_PREFIX = "FEATURE_"; + + private static final Matcher<ExpressionTree> HAS_SYSTEM_FEATURE_MATCHER = + Matchers.instanceMethod() + .onDescendantOf(PACKAGE_MANAGER_CLASS) + .named(HAS_SYSTEM_FEATURE_METHOD) + .withParameters(String.class.getName()); + + private static final Matcher<ExpressionTree> GET_PACKAGE_MANAGER_MATCHER = + Matchers.instanceMethod() + .onDescendantOf(CONTEXT_CLASS) + .named(GET_PACKAGE_MANAGER_METHOD); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!HAS_SYSTEM_FEATURE_MATCHER.matches(tree, state)) { + return Description.NO_MATCH; + } + + // Check if the PackageManager was obtained from a Context instance. + ExpressionTree packageManager = ASTHelpers.getReceiver(tree); + if (!GET_PACKAGE_MANAGER_MATCHER.matches(packageManager, state)) { + return Description.NO_MATCH; + } + + // Get the feature argument and check if it's a PackageManager.FEATURE_X constant. + ExpressionTree feature = tree.getArguments().isEmpty() ? null : tree.getArguments().get(0); + Symbol featureSymbol = ASTHelpers.getSymbol(feature); + if (featureSymbol == null + || !featureSymbol.isStatic() + || !featureSymbol.getSimpleName().toString().startsWith(FEATURE_PREFIX) + || ASTHelpers.enclosingClass(featureSymbol) == null + || !ASTHelpers.enclosingClass(featureSymbol) + .getQualifiedName() + .contentEquals(PACKAGE_MANAGER_CLASS)) { + return Description.NO_MATCH; + } + + // Check if the feature argument is part of the RoSystemFeatures API surface. + String featureName = featureSymbol.getSimpleName().toString(); + String methodName = RoSystemFeaturesMetadata.getMethodNameForFeatureName(featureName); + if (methodName == null) { + return Description.NO_MATCH; + } + + // Generate the appropriate fix. + String replacement = + String.format( + "%s.%s(%s)", + RO_SYSTEM_FEATURE_SIMPLE_CLASS, + methodName, + state.getSourceForNode(ASTHelpers.getReceiver(packageManager))); + // Note that ErrorProne doesn't offer a seamless way of removing the `PackageManager` import + // if unused after fix application, so for now we only offer best effort import suggestions. + SuggestedFix fix = + SuggestedFix.builder() + .replace(tree, replacement) + .addImport(RO_SYSTEM_FEATURE_CLASS) + .removeStaticImport(PACKAGE_MANAGER_CLASS + "." + featureName) + .build(); + return describeMatch(tree, fix); + } +} diff --git a/tools/systemfeatures/errorprone/tests/java/com/android/systemfeatures/errorprone/RoSystemFeaturesCheckerTest.java b/tools/systemfeatures/errorprone/tests/java/com/android/systemfeatures/errorprone/RoSystemFeaturesCheckerTest.java new file mode 100644 index 000000000000..c517b2495ee4 --- /dev/null +++ b/tools/systemfeatures/errorprone/tests/java/com/android/systemfeatures/errorprone/RoSystemFeaturesCheckerTest.java @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2020 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.systemfeatures.errorprone; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.CompilationTestHelper; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class RoSystemFeaturesCheckerTest { + private BugCheckerRefactoringTestHelper mRefactoringHelper; + private CompilationTestHelper mCompilationHelper; + + @Before + public void setUp() { + mCompilationHelper = + CompilationTestHelper.newInstance(RoSystemFeaturesChecker.class, getClass()); + mRefactoringHelper = + BugCheckerRefactoringTestHelper.newInstance( + RoSystemFeaturesChecker.class, getClass()); + } + + @Test + public void testNoDiagnostic() { + mCompilationHelper + .addSourceFile("/android/content/Context.java") + .addSourceFile("/android/content/pm/PackageManager.java") + .addSourceLines("Example.java", + """ + import android.content.Context; + import android.content.pm.PackageManager; + public class Example { + void test(Context context) { + boolean hasCustomFeature = context.getPackageManager() + .hasSystemFeature("my.custom.feature"); + boolean hasNonAnnotatedFeature = context.getPackageManager() + .hasSystemFeature(PackageManager.FEATURE_NOT_ANNOTATED); + boolean hasNonRoApiFeature = context.getPackageManager() + .hasSystemFeature(PackageManager.FEATURE_NOT_IN_RO_FEATURE_API); + } + } + """) + .doTest(); + } + + @Test + public void testDiagnostic() { + mCompilationHelper + .addSourceFile("/android/content/Context.java") + .addSourceFile("/android/content/pm/PackageManager.java") + .addSourceLines("Example.java", + """ + import android.content.Context; + import android.content.pm.PackageManager; + public class Example { + void test(Context context) { + boolean hasFeature = context.getPackageManager() + // BUG: Diagnostic contains: + .hasSystemFeature(PackageManager.FEATURE_PC); + } + } + """) + .doTest(); + } + + @Test + public void testFix() { + mRefactoringHelper + .addInputLines("Example.java", + """ + import static android.content.pm.PackageManager.FEATURE_WATCH; + + import android.content.Context; + import android.content.pm.PackageManager; + public class Example { + static class CustomContext extends Context {}; + private CustomContext mContext; + void test(Context context) { + boolean hasPc = mContext.getPackageManager() + .hasSystemFeature(PackageManager.FEATURE_PC); + boolean hasWatch = context.getPackageManager() + .hasSystemFeature(FEATURE_WATCH); + } + } + """) + .addOutputLines("Example.java", + """ + import android.content.Context; + import android.content.pm.PackageManager; + import com.android.internal.pm.RoSystemFeatures; + public class Example { + static class CustomContext extends Context {}; + private CustomContext mContext; + void test(Context context) { + boolean hasPc = RoSystemFeatures.hasFeaturePc(mContext); + boolean hasWatch = RoSystemFeatures.hasFeatureWatch(context); + } + } + """) + // Don't try compiling the output, as it requires pulling in the full set of code + // dependencies. + .allowBreakingChanges() + .doTest(); + } +} diff --git a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt index f260e2733843..ea660b013893 100644 --- a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt +++ b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt @@ -53,11 +53,20 @@ import javax.lang.model.element.Modifier * public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures(); * } * </pre> + * + * <p> If `--metadata-only=true` is set, the resulting class would simply be: + * <pre> + * package com.foo; + * public final class RoSystemFeatures { + * public static String getMethodNameForFeatureName(String featureName); + * } + * </pre> */ object SystemFeaturesGenerator { private const val FEATURE_ARG = "--feature=" private const val FEATURE_APIS_ARG = "--feature-apis=" private const val READONLY_ARG = "--readonly=" + private const val METADATA_ONLY_ARG = "--metadata-only=" 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") @@ -84,6 +93,8 @@ object SystemFeaturesGenerator { 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.") + println(" --metadata-only=true|false Whether to simply output metadata about the") + println(" generated API surface.") } /** Main entrypoint for build-time system feature codegen. */ @@ -106,6 +117,7 @@ object SystemFeaturesGenerator { } var readonly = false + var metadataOnly = false var outputClassName: ClassName? = null val featureArgs = mutableListOf<FeatureInfo>() // We could just as easily hardcode this list, as the static API surface should change @@ -115,6 +127,8 @@ object SystemFeaturesGenerator { when { arg.startsWith(READONLY_ARG) -> readonly = arg.substring(READONLY_ARG.length).toBoolean() + arg.startsWith(METADATA_ONLY_ARG) -> + metadataOnly = arg.substring(METADATA_ONLY_ARG.length).toBoolean() arg.startsWith(FEATURE_ARG) -> { featureArgs.add(parseFeatureArg(arg)) } @@ -155,9 +169,13 @@ object SystemFeaturesGenerator { .addModifiers(Modifier.PUBLIC, Modifier.FINAL) .addJavadoc("@hide") - addFeatureMethodsToClass(classBuilder, features.values) - addMaybeFeatureMethodToClass(classBuilder, features.values) - addGetFeaturesMethodToClass(classBuilder, features.values) + if (metadataOnly) { + addMetadataMethodToClass(classBuilder, features.values) + } else { + addFeatureMethodsToClass(classBuilder, features.values) + addMaybeFeatureMethodToClass(classBuilder, features.values) + addGetFeaturesMethodToClass(classBuilder, features.values) + } // TODO(b/203143243): Add validation of build vs runtime values to ensure consistency. JavaFile.builder(outputClassName.packageName(), classBuilder.build()) @@ -214,11 +232,8 @@ object SystemFeaturesGenerator { features: Collection<FeatureInfo>, ) { for (feature in features) { - // Turn "FEATURE_FOO" into "hasFeatureFoo". - val methodName = - "has" + CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, feature.name) val methodBuilder = - MethodSpec.methodBuilder(methodName) + MethodSpec.methodBuilder(feature.methodName) .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addJavadoc("Check for ${feature.name}.\n\n@hide") .returns(Boolean::class.java) @@ -341,5 +356,32 @@ object SystemFeaturesGenerator { builder.addMethod(methodBuilder.build()) } - private data class FeatureInfo(val name: String, val version: Int?, val readonly: Boolean) + /* + * Adds a metadata helper method that maps FEATURE_FOO names to their generated hasFeatureFoo() + * API counterpart, if defined. + */ + private fun addMetadataMethodToClass( + builder: TypeSpec.Builder, + features: Collection<FeatureInfo>, + ) { + val methodBuilder = + MethodSpec.methodBuilder("getMethodNameForFeatureName") + .addModifiers(Modifier.PUBLIC, Modifier.STATIC) + .addJavadoc("@return \"hasFeatureFoo\" if FEATURE_FOO is in the API, else null") + .returns(String::class.java) + .addParameter(String::class.java, "featureVarName") + + methodBuilder.beginControlFlow("switch (featureVarName)") + for (feature in features) { + methodBuilder.addStatement("case \$S: return \$S", feature.name, feature.methodName) + } + methodBuilder.addStatement("default: return null").endControlFlow() + + builder.addMethod(methodBuilder.build()) + } + + private data class FeatureInfo(val name: String, val version: Int?, val readonly: Boolean) { + // Turn "FEATURE_FOO" into "hasFeatureFoo". + val methodName get() = "has" + CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, name) + } } diff --git a/tools/systemfeatures/tests/src/SystemFeaturesMetadataProcessorTest.java b/tools/systemfeatures/tests/src/SystemFeaturesMetadataProcessorTest.java index 74ce6daaffc4..560454b65b7e 100644 --- a/tools/systemfeatures/tests/src/SystemFeaturesMetadataProcessorTest.java +++ b/tools/systemfeatures/tests/src/SystemFeaturesMetadataProcessorTest.java @@ -36,8 +36,8 @@ public class SystemFeaturesMetadataProcessorTest { @Test public void testSdkFeatureCount() { // See the fake PackageManager definition in this directory. - // It defines 5 annotated features, and any/all other constants should be ignored. - assertThat(SystemFeaturesMetadata.SDK_FEATURE_COUNT).isEqualTo(5); + // It defines 6 annotated features, and any/all other constants should be ignored. + assertThat(SystemFeaturesMetadata.SDK_FEATURE_COUNT).isEqualTo(6); } @Test diff --git a/tools/systemfeatures/tests/src/Context.java b/tools/systemfeatures/tests/src/android/content/Context.java index 630bc0771a01..630bc0771a01 100644 --- a/tools/systemfeatures/tests/src/Context.java +++ b/tools/systemfeatures/tests/src/android/content/Context.java diff --git a/tools/systemfeatures/tests/src/FeatureInfo.java b/tools/systemfeatures/tests/src/android/content/pm/FeatureInfo.java index 9d57edc64ca5..9d57edc64ca5 100644 --- a/tools/systemfeatures/tests/src/FeatureInfo.java +++ b/tools/systemfeatures/tests/src/android/content/pm/FeatureInfo.java diff --git a/tools/systemfeatures/tests/src/PackageManager.java b/tools/systemfeatures/tests/src/android/content/pm/PackageManager.java index 839a9377476d..4a9edd61b55b 100644 --- a/tools/systemfeatures/tests/src/PackageManager.java +++ b/tools/systemfeatures/tests/src/android/content/pm/PackageManager.java @@ -36,6 +36,9 @@ public class PackageManager { @SdkConstant(SdkConstantType.FEATURE) public static final String FEATURE_WIFI = "wifi"; + @SdkConstant(SdkConstantType.FEATURE) + public static final String FEATURE_NOT_IN_RO_FEATURE_API = "not_in_ro_feature_api"; + @SdkConstant(SdkConstantType.INTENT_CATEGORY) public static final String FEATURE_INTENT_CATEGORY = "intent_category_with_feature_name_prefix"; @@ -47,4 +50,9 @@ public class PackageManager { public boolean hasSystemFeature(String featureName, int version) { return false; } + + /** @hide */ + public boolean hasSystemFeature(String featureName) { + return hasSystemFeature(featureName, 0); + } } diff --git a/tools/systemfeatures/tests/src/ArrayMap.java b/tools/systemfeatures/tests/src/android/util/ArrayMap.java index a5ed9b088896..a5ed9b088896 100644 --- a/tools/systemfeatures/tests/src/ArrayMap.java +++ b/tools/systemfeatures/tests/src/android/util/ArrayMap.java diff --git a/tools/systemfeatures/tests/src/ArraySet.java b/tools/systemfeatures/tests/src/android/util/ArraySet.java index 0eb8f298bd89..0eb8f298bd89 100644 --- a/tools/systemfeatures/tests/src/ArraySet.java +++ b/tools/systemfeatures/tests/src/android/util/ArraySet.java |