Fix usage of Immutable list in resource shrinker
The returned list is mutable unless it is empty, allways create a copy.
Bug: b/401546693
Change-Id: I9ea50d23c365ea59e1f6ae5e13a02048daa8b5f8
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 a64a3fd..baf1f72 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/LegacyResourceShrinker.java
@@ -37,7 +37,6 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
@@ -211,12 +210,13 @@
// Finds unused resources in provided resources collection.
// Marks all used resources as 'reachable' in original collection.
List<Resource> unusedResources =
- ResourcesUtil.findUnusedResources(
- model.getResourceStore().getResources(),
- roots -> {
- debugReporter.debug(() -> "The root reachable resources are:");
- roots.forEach(root -> debugReporter.debug(() -> " " + root));
- });
+ new ArrayList<>(
+ ResourcesUtil.findUnusedResources(
+ model.getResourceStore().getResources(),
+ roots -> {
+ debugReporter.debug(() -> "The root reachable resources are:");
+ roots.forEach(root -> debugReporter.debug(() -> " " + root));
+ }));
ImmutableSet.Builder<String> resEntriesToKeepBuilder = new ImmutableSet.Builder<>();
for (PathAndBytes xmlInput : Iterables.concat(xmlInputs, resFolderInputs)) {
if (ResourceShrinkerImplKt.isJarPathReachable(resourceStore, xmlInput.path.toString())) {
diff --git a/src/test/java/com/android/tools/r8/androidresources/DuplicatedEntriesEmptyUnusedTest.java b/src/test/java/com/android/tools/r8/androidresources/DuplicatedEntriesEmptyUnusedTest.java
index 50ed17d..8b7e50b 100644
--- a/src/test/java/com/android/tools/r8/androidresources/DuplicatedEntriesEmptyUnusedTest.java
+++ b/src/test/java/com/android/tools/r8/androidresources/DuplicatedEntriesEmptyUnusedTest.java
@@ -3,10 +3,6 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.androidresources;
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.MatcherAssert.assertThat;
-
-import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -14,7 +10,6 @@
import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
import com.android.tools.r8.androidresources.DuplicatedEntriesEmptyUnusedTest.FeatureSplit.FeatureSplitMain;
-import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.utils.BooleanUtils;
import java.io.IOException;
import java.util.List;
@@ -64,47 +59,31 @@
public void testR8() throws Exception {
TemporaryFolder featureSplitTemp = ToolHelper.getTemporaryFolderForTest();
featureSplitTemp.create();
- R8FullTestBuilder testBuilder =
- testForR8(parameters.getBackend())
- .setMinApi(parameters)
- .addProgramClasses(Base.class)
- .addFeatureSplit(FeatureSplitMain.class)
- .addAndroidResources(getTestResources(temp))
- .addFeatureSplitAndroidResources(
- getFeatureSplitTestResources(featureSplitTemp), FeatureSplit.class.getName())
- .applyIf(optimized, R8FullTestBuilder::enableOptimizedShrinking)
- .addKeepMainRule(Base.class)
- .addKeepMainRule(FeatureSplitMain.class);
- if (optimized) {
- testBuilder
- .compile()
- .inspectShrunkenResources(
- resourceTableInspector -> {
- resourceTableInspector.assertContainsResourceWithName("xml", "duplicated_xml");
- resourceTableInspector.assertContainsResourceWithName(
- "string", "duplicated_string");
- })
- .inspectShrunkenResourcesForFeature(
- resourceTableInspector -> {
- resourceTableInspector.assertContainsResourceWithName("xml", "duplicated_xml");
- resourceTableInspector.assertContainsResourceWithName(
- "string", "duplicated_string");
- },
- FeatureSplit.class.getName())
- .run(parameters.getRuntime(), Base.class)
- .assertSuccess();
- } else {
- // TODO(b/401546693): This should compile without failure (and keep all resources)
- try {
- testBuilder.compile();
- } catch (CompilationFailedException e) {
- assertThat(
- e.getCause().getMessage(),
- containsString("Operation is not supported for read-only collection"));
- return;
- }
- throw new Unreachable("Problem fixed?");
- }
+
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(Base.class)
+ .addFeatureSplit(FeatureSplitMain.class)
+ .addAndroidResources(getTestResources(temp))
+ .addFeatureSplitAndroidResources(
+ getFeatureSplitTestResources(featureSplitTemp), FeatureSplit.class.getName())
+ .applyIf(optimized, R8FullTestBuilder::enableOptimizedShrinking)
+ .addKeepMainRule(Base.class)
+ .addKeepMainRule(FeatureSplitMain.class)
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("xml", "duplicated_xml");
+ resourceTableInspector.assertContainsResourceWithName("string", "duplicated_string");
+ })
+ .inspectShrunkenResourcesForFeature(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("xml", "duplicated_xml");
+ resourceTableInspector.assertContainsResourceWithName("string", "duplicated_string");
+ },
+ FeatureSplit.class.getName())
+ .run(parameters.getRuntime(), Base.class)
+ .assertSuccess();
}
public static class Base {
