Always unconditionally keep id resources
Add regression test that we always keep ids.
We can consider if we want to relax this for optimized shrinking (but
there are libraries relying on these)
Bug: 307987906
Change-Id: Ie1070d1e3cb59dc16e7139dfcae53c26a9ece8f5
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/ResourceShrinkerImpl.kt b/src/resourceshrinker/java/com/android/build/shrinker/ResourceShrinkerImpl.kt
index d8f6fa2..d404e2e6 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/ResourceShrinkerImpl.kt
+++ b/src/resourceshrinker/java/com/android/build/shrinker/ResourceShrinkerImpl.kt
@@ -171,7 +171,9 @@
private fun removeResourceUnusedTableEntries(zis: InputStream,
zos: JarOutputStream,
srcEntry: ZipEntry) {
- val resourceIdsToRemove = unused.map { resource -> resource.value }
+ val resourceIdsToRemove = unused
+ .filterNot { it.type == ResourceType.ID }
+ .map { resource -> resource.value }
val shrunkenResourceTable = Resources.ResourceTable.parseFrom(zis)
.nullOutEntriesWithIds(resourceIdsToRemove)
val bytes = shrunkenResourceTable.toByteArray()
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
index ab97ce2..f37d381 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
@@ -24,6 +24,7 @@
import com.android.ide.common.resources.ResourcesUtil;
import com.android.ide.common.resources.usage.ResourceStore;
import com.android.ide.common.resources.usage.ResourceUsageModel.Resource;
+import com.android.resources.ResourceType;
import com.android.tools.r8.FeatureSplit;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -210,10 +211,14 @@
R8ResourceShrinkerModel model,
boolean exactMatchingOfStyleablesAndAttr) {
if (!exactMatchingOfStyleablesAndAttr) {
- return unusedResources.stream().map(resource -> resource.value).collect(Collectors.toList());
+ return unusedResources.stream()
+ .filter(s -> s.type != ResourceType.ID)
+ .map(resource -> resource.value)
+ .collect(Collectors.toList());
}
return model.getResourceStore().getResources().stream()
.filter(r -> !r.isReachable())
+ .filter(r -> r.type != ResourceType.ID)
.map(r -> r.value)
.collect(Collectors.toList());
}
diff --git a/src/test/java/com/android/tools/r8/androidresources/AlwaysKeepIDsInLegacyMode.java b/src/test/java/com/android/tools/r8/androidresources/AlwaysKeepIDsInLegacyMode.java
new file mode 100644
index 0000000..605a771
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/AlwaysKeepIDsInLegacyMode.java
@@ -0,0 +1,79 @@
+// 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 com.android.tools.r8.R8TestBuilder;
+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.BooleanUtils;
+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 AlwaysKeepIDsInLegacyMode extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameter(1)
+ public boolean optimized;
+
+ @Parameters(name = "{0}, optimized: {1}")
+ public static List<Object[]> data() {
+ return buildParameters(
+ getTestParameters().withDefaultDexRuntime().withAllApiLevels().build(),
+ BooleanUtils.values());
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.string.class, R.id.class)
+ .build(temp);
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(FooBar.class)
+ .addAndroidResources(getTestResources(temp))
+ .addKeepMainRule(FooBar.class)
+ .applyIf(optimized, R8TestBuilder::enableOptimizedShrinking)
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("string", "foo");
+ // The id resource should not be removed in any mode, even though there are no
+ // references to it.
+ resourceTableInspector.assertContainsResourceWithName("id", "the_id");
+ })
+ .run(parameters.getRuntime(), FooBar.class)
+ .assertSuccess();
+ }
+
+ public static class FooBar {
+
+ public static void main(String[] args) {
+ System.out.println(R.string.foo);
+ }
+ }
+
+ public static class R {
+ public static class string {
+ public static int foo;
+ }
+
+ public static class id {
+ public static int the_id;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java b/src/test/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
index e0a3a59..5040ed3 100644
--- a/src/test/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
+++ b/src/test/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
@@ -52,7 +52,8 @@
STRING,
DRAWABLE,
STYLEABLE,
- XML;
+ XML,
+ ID;
public static RClassType fromClass(Class clazz) {
String type = rClassWithoutNamespaceAndOuter(clazz).substring(2);
@@ -119,6 +120,13 @@
+ " path=\"let/it/be\" />\n"
+ "</paths>";
+ public static String XML_FILE_WITH_ID =
+ "<menu xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " xmlns:app=\"http://schemas.android.com/apk/res-auto\"\n"
+ + " xmlns:tools=\"http://schemas.android.com/tools\">\n"
+ + " <item android:id=\"@+id/%s\"/>\n"
+ + "</menu>";
+
public static class AndroidTestRClass {
// The original aapt2 generated R.java class
private final Path javaFilePath;
@@ -256,6 +264,7 @@
public static class AndroidTestResourceBuilder {
private String manifest;
private final Map<String, String> stringValues = new TreeMap<>();
+ private final Set<String> idValues = new TreeSet<>();
private final Set<String> stringValuesWithExtraLanguage = new TreeSet<>();
private final Map<String, String> overlayableValues = new TreeMap<>();
private final Map<String, Integer> styleables = new TreeMap<>();
@@ -285,6 +294,9 @@
// Add 4 different values, i.e., the array will be 4 integers.
addStyleable(name, 4);
}
+ if (rClassType == RClassType.ID) {
+ addIdValue(name);
+ }
}
}
return this;
@@ -317,6 +329,11 @@
return this;
}
+ AndroidTestResourceBuilder addIdValue(String name) {
+ xmlFiles.put(name + ".xml", String.format(XML_FILE_WITH_ID, name));
+ return this;
+ }
+
AndroidTestResourceBuilder addExtraLanguageString(String name) {
stringValuesWithExtraLanguage.add(name);
return this;
