Correctly handle manifest unqualified class names
There is a range of specific attributes that can use the package name of the manifest.
Bug: b/287398085
Change-Id: I90bdf2c563558b2521deb2575cac9943f25a120d
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
index 5ac9755..875288c 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
@@ -64,6 +64,19 @@
private final Set<String> seenNoneClassValues = new HashSet<>();
private final Set<Integer> seenResourceIds = new HashSet<>();
+ private static final Set<String> SPECIAL_MANIFEST_ELEMENTS =
+ ImmutableSet.of(
+ "provider",
+ "activity",
+ "service",
+ "receiver",
+ "instrumentation",
+ "process",
+ "application");
+
+ private static final Set<String> SPECIAL_APPLICATION_ATTRIBUTES =
+ ImmutableSet.of("backupAgent", "appComponentFactory", "zygotePreloadName");
+
@FunctionalInterface
public interface ClassReferenceCallback {
boolean tryClass(String possibleClass, Origin xmlFileOrigin);
@@ -226,7 +239,7 @@
private void traceXml(String xmlFile, InputStream inputStream) {
try {
XmlNode xmlNode = XmlNode.parseFrom(inputStream);
- visitNode(xmlNode, xmlFile);
+ visitNode(xmlNode, xmlFile, null);
// Ensure that we trace the transitive reachable ids, without us having to iterate all
// resources for the reachable marker.
ProtoAndroidManifestUsageRecorderKt.recordUsagesFromNode(xmlNode, r8ResourceShrinkerModel)
@@ -237,7 +250,6 @@
}
}
-
private void tryEnqueuerOnString(String possibleClass, String xmlName) {
// There are a lot of xml tags and attributes that are evaluated over and over, if it is
// not a class, ignore it.
@@ -249,18 +261,51 @@
}
}
- private void visitNode(XmlNode xmlNode, String xmlName) {
+ private void visitNode(XmlNode xmlNode, String xmlName, String manifestPackageName) {
XmlElement element = xmlNode.getElement();
tryEnqueuerOnString(element.getName(), xmlName);
+
for (XmlAttribute xmlAttribute : element.getAttributeList()) {
+ if (xmlAttribute.getName().equals("package") && element.getName().equals("manifest")) {
+ // We are traversing a manifest, record the package name if we see it.
+ manifestPackageName = xmlAttribute.getValue();
+ }
String value = xmlAttribute.getValue();
tryEnqueuerOnString(value, xmlName);
if (value.startsWith(".")) {
// package specific names, e.g. context
getPackageNames().forEach(s -> tryEnqueuerOnString(s + value, xmlName));
}
+ if (manifestPackageName != null) {
+ // Manifest case
+ traceManifestSpecificValues(xmlName, manifestPackageName, xmlAttribute, element);
+ }
}
- element.getChildList().forEach(e -> visitNode(e, xmlName));
+ for (XmlNode node : element.getChildList()) {
+ visitNode(node, xmlName, manifestPackageName);
+ }
+ }
+
+ private void traceManifestSpecificValues(
+ String xmlName, String packageName, XmlAttribute xmlAttribute, XmlElement element) {
+ if (!SPECIAL_MANIFEST_ELEMENTS.contains(element.getName())) {
+ return;
+ }
+ // All elements can have package specific name attributes pointing at classes.
+ if (xmlAttribute.getName().equals("name")) {
+ tryEnqueuerOnString(getFullyQualifiedName(packageName, xmlAttribute), xmlName);
+ }
+ // Application elements have multiple special case attributes, where the value is potentially
+ // a class name (unqualified).
+ if (element.getName().equals("application")) {
+ if (SPECIAL_APPLICATION_ATTRIBUTES.contains(xmlAttribute.getName())) {
+ tryEnqueuerOnString(getFullyQualifiedName(packageName, xmlAttribute), xmlName);
+ }
+ }
+ }
+
+ private static String getFullyQualifiedName(String packageName, XmlAttribute xmlAttribute) {
+ return packageName + "." + xmlAttribute.getValue();
}
public Map<Integer, List<String>> getResourceIdToXmlFiles() {
diff --git a/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationAttributesTest.java b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationAttributesTest.java
new file mode 100644
index 0000000..2770b77
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationAttributesTest.java
@@ -0,0 +1,99 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AndroidManifestWithApplicationAttributesTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public String attributeName;
+
+ @Parameters(name = "{0}, attributeName: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ ImmutableList.of("backupAgent", "appComponentFactory", "zygotePreloadName"));
+ }
+
+ private static final String INNER_CLASS_NAME =
+ AndroidManifestWithApplicationAttributesTest.class.getSimpleName()
+ + "$"
+ + Bar.class.getSimpleName();
+
+ public static String MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION =
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ + "<manifest xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " xmlns:tools=\"http://schemas.android.com/tools\""
+ + " package=\""
+ + AndroidManifestWithApplicationAttributesTest.class.getPackage().getName()
+ + "\""
+ + ">\n"
+ + " <application android:%s=\""
+ + INNER_CLASS_NAME
+ + "\"/>"
+ + "</manifest>";
+
+ public AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withManifest(
+ String.format(MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION, attributeName))
+ .build(temp);
+ }
+
+ @Test
+ public void testManifestReferences() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(Bar.class)
+ .addAndroidResources(getTestResources(temp))
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresentAndNotRenamed());
+ // We should have two and only two methods, the two constructors.
+ assertEquals(barClass.allMethods(MethodSubject::isInstanceInitializer).size(), 2);
+ assertEquals(barClass.allMethods().size(), 2);
+ });
+ }
+
+ // Only referenced from Manifest file
+ public static class Bar {
+ // We should keep this since this is a provider
+ public Bar() {
+ System.out.println("init");
+ }
+
+ public Bar(String x) {
+ System.out.println("init with string");
+ }
+
+ public void bar() {
+ System.out.println("never kept");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationElementTest.java b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationElementTest.java
new file mode 100644
index 0000000..5e3ea38
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithApplicationElementTest.java
@@ -0,0 +1,101 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AndroidManifestWithApplicationElementTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public String elementType;
+
+ @Parameters(name = "{0}, elementType: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ ImmutableList.of("provider", "activity", "service", "receiver"));
+ }
+
+ private static final String INNER_CLASS_NAME =
+ AndroidManifestWithApplicationElementTest.class.getSimpleName()
+ + "$"
+ + Bar.class.getSimpleName();
+
+ public static String MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION =
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ + "<manifest xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " xmlns:tools=\"http://schemas.android.com/tools\""
+ + " package=\""
+ + AndroidManifestWithApplicationElementTest.class.getPackage().getName()
+ + "\""
+ + ">\n"
+ + " <application>\n"
+ + " <%s android:name=\""
+ + INNER_CLASS_NAME
+ + "\"/>"
+ + " </application>\n"
+ + "</manifest>";
+
+ public AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withManifest(
+ String.format(MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION, elementType))
+ .build(temp);
+ }
+
+ @Test
+ public void testManifestReferences() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(Bar.class)
+ .addAndroidResources(getTestResources(temp))
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresentAndNotRenamed());
+ // We should have two and only two methods, the two constructors.
+ assertEquals(barClass.allMethods(MethodSubject::isInstanceInitializer).size(), 2);
+ assertEquals(barClass.allMethods().size(), 2);
+ });
+ }
+
+ // Only referenced from Manifest file
+ public static class Bar {
+ // We should keep this since this is a provider
+ public Bar() {
+ System.out.println("init");
+ }
+
+ public Bar(String x) {
+ System.out.println("init with string");
+ }
+
+ public void bar() {
+ System.out.println("never kept");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithManifestEntriesTest.java b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithManifestEntriesTest.java
new file mode 100644
index 0000000..6037077
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithManifestEntriesTest.java
@@ -0,0 +1,99 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.util.List;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AndroidManifestWithManifestEntriesTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public String elementType;
+
+ @Parameters(name = "{0}, elementType: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ ImmutableList.of("application", "instrumentation"));
+ }
+
+ private static final String INNER_CLASS_NAME =
+ AndroidManifestWithManifestEntriesTest.class.getSimpleName()
+ + "$"
+ + Bar.class.getSimpleName();
+
+ public static String MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION =
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ + "<manifest xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " xmlns:tools=\"http://schemas.android.com/tools\""
+ + " package=\""
+ + AndroidManifestWithManifestEntriesTest.class.getPackage().getName()
+ + "\""
+ + ">\n"
+ + " <%s android:name=\""
+ + INNER_CLASS_NAME
+ + "\"/>\n"
+ + "</manifest>";
+
+ public AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withManifest(
+ String.format(MANIFEST_WITH_MANIFEST_PACKAGE_USAGE_SUB_APPLICATION, elementType))
+ .build(temp);
+ }
+
+ @Test
+ public void testManifestReferences() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(Bar.class)
+ .addAndroidResources(getTestResources(temp))
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresentAndNotRenamed());
+ // We should have two and only two methods, the two constructors.
+ assertEquals(barClass.allMethods(MethodSubject::isInstanceInitializer).size(), 2);
+ assertEquals(barClass.allMethods().size(), 2);
+ });
+ }
+
+ // Only referenced from Manifest file
+ public static class Bar {
+ // We should keep this since this is a provider
+ public Bar() {
+ System.out.println("init");
+ }
+
+ public Bar(String x) {
+ System.out.println("init with string");
+ }
+
+ public void bar() {
+ System.out.println("never kept");
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithProcessesTest.java b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithProcessesTest.java
new file mode 100644
index 0000000..e4f8583
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/AndroidManifestWithProcessesTest.java
@@ -0,0 +1,92 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class AndroidManifestWithProcessesTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withDefaultDexRuntime().withAllApiLevels().build();
+ }
+
+ private static final String INNER_CLASS_NAME =
+ AndroidManifestWithProcessesTest.class.getSimpleName() + "$" + Bar.class.getSimpleName();
+
+ public static String MANIFEST_WITH_PROCESS =
+ "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+ + "<manifest xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " xmlns:tools=\"http://schemas.android.com/tools\""
+ + " package=\""
+ + AndroidManifestWithProcessesTest.class.getPackage().getName()
+ + "\""
+ + ">\n"
+ + " <application>\n"
+ + " <processes>\n"
+ + " <process android:process=\":sub\" android:name=\""
+ + INNER_CLASS_NAME
+ + "\"/>\n"
+ + " </processes>\n"
+ + " </application>\n"
+ + "</manifest>";
+
+ public AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder().withManifest(MANIFEST_WITH_PROCESS).build(temp);
+ }
+
+ @Test
+ public void testManifestReferences() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(Bar.class)
+ .addAndroidResources(getTestResources(temp))
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresentAndNotRenamed());
+ // We should have two and only two methods, the two constructors.
+ assertEquals(barClass.allMethods(MethodSubject::isInstanceInitializer).size(), 2);
+ assertEquals(barClass.allMethods().size(), 2);
+ });
+ }
+
+ // Only referenced from Manifest file
+ public static class Bar {
+ // We should keep this since this is a provider
+ public Bar() {
+ System.out.println("init");
+ }
+
+ public Bar(String x) {
+ System.out.println("init with string");
+ }
+
+ public void bar() {
+ System.out.println("never kept");
+ }
+ }
+}