Filter local and transitive resources together

Whether a resource is accepted or rejected by density-based resource filtering
is dependent on what other resources are available. When filtering resources in
execution, this was taken into account - resources were filtered after merging.
To replicate this behavior when filtering in analysis, we must look at both
local and transitive resources before we actually filter anything.

This process makes filtering with dynamic configuration extremely inefficient, since the NestedSet of transitive resources must be collapsed at each library target. We can fix this by only looking at the transitive resources at the top-level target, even when using dynamic filtering. I'm not implementing that in this change, however, since dynamic filtering is relatively low priority and this review is already pretty big.

Note that some of the messiness around filtering ResourceDependencies and
NestedSet<ResourceContainer> will go away once those NestedSets are removed.

Also, stop filtering resources in android_test, since android_test can never specify resource filters.

RELNOTES: none
PiperOrigin-RevId: 174474491

6 files changed, 263 insertions, 177 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index 079cc6c496..0dbc9e6e7e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -343,22 +343,10 @@ public final class ApplicationManifest {
       Artifact proguardCfg,
       @Nullable String packageUnderTest)
       throws InterruptedException, RuleErrorException {
-    // Filter the resources during analysis to prevent processing of dependencies on unwanted
-    // resources during execution.
-    ResourceFilterFactory resourceFilterFactory =
-        ResourceFilterFactory.fromRuleContext(ruleContext);
-    resourceDeps = resourceDeps.filter(ruleContext, resourceFilterFactory);
-
     LocalResourceContainer data =
         LocalResourceContainer.forAssetsAndResources(
-                ruleContext,
-                "assets",
-                AndroidCommon.getAssetDir(ruleContext),
-                "local_resource_files")
-            .filter(ruleContext, resourceFilterFactory);
+            ruleContext, "assets", AndroidCommon.getAssetDir(ruleContext), "local_resource_files");
 
-    // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
-    // container from it.
     ResourceContainer resourceContainer =
         checkForInlinedResources(
             ResourceContainer.builderFromRule(ruleContext)
@@ -378,7 +366,6 @@ public final class ApplicationManifest {
         new AndroidResourcesProcessorBuilder(ruleContext)
             .setLibrary(false)
             .setApkOut(resourceContainer.getApk())
-            .setResourceFilterFactory(resourceFilterFactory)
             .setUncompressedExtensions(ImmutableList.of())
             .setCrunchPng(true)
             .setJavaPackage(resourceContainer.getJavaPackage())
@@ -428,10 +415,11 @@ public final class ApplicationManifest {
       throws InterruptedException, RuleErrorException {
     // Filter the resources during analysis to prevent processing of dependencies on unwanted
     // resources during execution.
-    ResourceFilterFactory resourceFilterFactory =
-        ResourceFilterFactory.fromRuleContext(ruleContext);
-    data = data.filter(ruleContext, resourceFilterFactory);
-    resourceDeps = resourceDeps.filter(ruleContext, resourceFilterFactory);
+    ResourceFilter resourceFilter =
+        ResourceFilterFactory.fromRuleContext(ruleContext)
+            .getResourceFilter(ruleContext, resourceDeps, data);
+    data = data.filter(ruleContext, resourceFilter);
+    resourceDeps = resourceDeps.filter(resourceFilter);
 
     // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
     // container from it.
@@ -528,8 +516,10 @@ public final class ApplicationManifest {
     // resources during execution.
     ResourceFilterFactory resourceFilterFactory =
         ResourceFilterFactory.fromRuleContext(ruleContext);
-    data = data.filter(ruleContext, resourceFilterFactory);
-    resourceDeps = resourceDeps.filter(ruleContext, resourceFilterFactory);
+    ResourceFilter resourceFilter =
+        resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, data);
+    data = data.filter(ruleContext, resourceFilter);
+    resourceDeps = resourceDeps.filter(resourceFilter);
 
     // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
     // container from it.
@@ -602,10 +592,12 @@ public final class ApplicationManifest {
       throws InterruptedException, RuleErrorException {
     LocalResourceContainer data =
         LocalResourceContainer.forAssetsAndResources(
-                ruleContext, "assets", AndroidCommon.getAssetDir(ruleContext), "resource_files")
-            .filter(ruleContext, resourceFilterFactory);
+            ruleContext, "assets", AndroidCommon.getAssetDir(ruleContext), "resource_files");
 
-    resourceDeps = resourceDeps.filter(ruleContext, resourceFilterFactory);
+    ResourceFilter resourceFilter =
+        resourceFilterFactory.getResourceFilter(ruleContext, resourceDeps, data);
+    data = data.filter(ruleContext, resourceFilter);
+    resourceDeps = resourceDeps.filter(resourceFilter);
 
     // Now that the LocalResourceContainer has been filtered, we can build a filtered resource
     // container from it.
@@ -679,13 +671,15 @@ public final class ApplicationManifest {
       throws InterruptedException, RuleErrorException {
     // Filter the resources during analysis to prevent processing of dependencies on unwanted
     // resources during execution.
-    ResourceFilterFactory resourceFilterFactory =
-        ResourceFilterFactory.fromRuleContext(ruleContext);
     LocalResourceContainer data =
         LocalResourceContainer.forAssetsAndResources(
-                ruleContext, "assets", AndroidCommon.getAssetDir(ruleContext), "resource_files")
-            .filter(ruleContext, resourceFilterFactory);
-    resourceDeps = resourceDeps.filter(ruleContext, resourceFilterFactory);
+            ruleContext, "assets", AndroidCommon.getAssetDir(ruleContext), "resource_files");
+    ResourceFilter resourceFilter =
+        ResourceFilterFactory.fromRuleContext(ruleContext)
+            .getResourceFilter(ruleContext, resourceDeps, data);
+    data = data.filter(ruleContext, resourceFilter);
+    resourceDeps = resourceDeps.filter(resourceFilter);
+
     ResourceContainer.Builder builder =
         ResourceContainer.builderFromRule(ruleContext)
             .setAssetsAndResourcesFrom(data)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index 89f38bbeee..dbc7c964fc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.rules.android.ResourceContainer.ResourceTyp
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Arrays;
 import java.util.LinkedHashSet;
+import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nullable;
 
@@ -180,9 +181,9 @@ public final class LocalResourceContainer {
    * Creates a {@link LocalResourceContainer} containing the resources included in a {@link
    * FileProvider}.
    *
-   * <p>In general, {@link #forAssetsAndResources(RuleContext, String, String, String)} should be
-   * used instead. No assets or transitive resources will be included in the container produced by
-   * this method.
+   * <p>In general, {@link #forAssetsAndResources(RuleContext, String, PathFragment, String)} should
+   * be used instead. No assets or transitive resources will be included in the container produced
+   * by this method.
    *
    * @param ruleContext the current context
    * @param resourceFileProvider the provider containing resources
@@ -378,22 +379,39 @@ public final class LocalResourceContainer {
    * Filters this object.
    *
    * @return a new {@link LocalResourceContainer} with resources filtered by the passed {@link
-   *     ResourceFilterFactory}, or this object if no resources should be filtered.
+   *     ResourceFilter}, or this object if no resources should be filtered.
    */
   public LocalResourceContainer filter(
-      RuleErrorConsumer ruleErrorConsumer, ResourceFilterFactory resourceFilterFactory)
+      RuleErrorConsumer ruleErrorConsumer, ResourceFilter resourceFilter)
       throws RuleErrorException {
-    ImmutableList<Artifact> filteredResources = resourceFilterFactory
-        .filter(ruleErrorConsumer, resources);
+    Optional<ImmutableList<Artifact>> filtered =
+        resourceFilter.maybeFilter(resources, /* isDependency= */ false);
 
-    if (filteredResources.size() == resources.size()) {
+    if (!filtered.isPresent()) {
       // Nothing was filtered out
       return this;
     }
 
+    return withResources(ruleErrorConsumer, filtered.get(), "resource_files");
+  }
+
+  @VisibleForTesting
+  static LocalResourceContainer forResources(
+      RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> resources)
+      throws RuleErrorException {
+    return new LocalResourceContainer(
+        resources,
+        getResourceRoots(ruleErrorConsumer, resources, "resource_files"),
+        ImmutableList.of(),
+        ImmutableList.of());
+  }
+
+  private LocalResourceContainer withResources(
+      RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> resources, String resourcesAttr)
+      throws RuleErrorException {
     return new LocalResourceContainer(
-        filteredResources,
-        getResourceRoots(ruleErrorConsumer, filteredResources, "resource_files"),
+        resources,
+        getResourceRoots(ruleErrorConsumer, resources, resourcesAttr),
         assets,
         assetRoots);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
index ce218c3236..8f724e6a05 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceContainer.java
@@ -22,12 +22,12 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.rules.java.JavaUtil;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Objects;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /** The resources contributed by a single target. */
@@ -177,11 +177,11 @@ public abstract class ResourceContainer {
    * Returns a copy of this container with filtered resources, or the original if no resources
    * should be filtered. The original container is unchanged.
    */
-  public ResourceContainer filter(
-      RuleErrorConsumer ruleErrorConsumer, ResourceFilterFactory filter) {
-    ImmutableList<Artifact> filteredResources = filter.filter(ruleErrorConsumer, getResources());
+  public ResourceContainer filter(ResourceFilter filter, boolean isDependency) {
+    Optional<ImmutableList<Artifact>> filteredResources =
+        filter.maybeFilter(getResources(), isDependency);
 
-    if (filteredResources.size() == getResources().size()) {
+    if (!filteredResources.isPresent()) {
       // No filtering was done; return this container
       return this;
     }
@@ -189,7 +189,7 @@ public abstract class ResourceContainer {
     // If the resources were filtered, also filter the resource roots
     ImmutableList.Builder<PathFragment> filteredResourcesRootsBuilder = ImmutableList.builder();
     for (PathFragment resourceRoot : getResourcesRoots()) {
-      for (Artifact resource : filteredResources) {
+      for (Artifact resource : filteredResources.get()) {
         if (resource.getRootRelativePath().startsWith(resourceRoot)) {
           filteredResourcesRootsBuilder.add(resourceRoot);
           break;
@@ -198,7 +198,7 @@ public abstract class ResourceContainer {
     }
 
     return toBuilder()
-        .setResources(filteredResources)
+        .setResources(filteredResources.get())
         .setResourcesRoots(filteredResourcesRootsBuilder.build())
         .build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
index cb9a3dd971..b72c44970b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.rules.android;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -25,7 +26,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.RuleErrorConsumer;
+import java.util.Optional;
 import javax.annotation.Nullable;
 
 /**
@@ -352,16 +353,12 @@ public final class ResourceDependencies {
     this.transitiveRTxt = transitiveRTxt;
   }
 
-  /**
-   * Returns a copy of this instance with filtered resources. The original object is unchanged, and
-   * may be returned if no filtering should be done.
-   */
-  public ResourceDependencies filter(
-      RuleErrorConsumer ruleErrorConsumer, ResourceFilterFactory filter) {
-    NestedSet<Artifact> filteredResources =
-        filter.filterDependencies(ruleErrorConsumer, transitiveResources);
+  /** Returns a copy of this instance with filtered resources. The original object is unchanged. */
+  public ResourceDependencies filter(ResourceFilter resourceFilter) {
+    Optional<NestedSet<Artifact>> filteredResources =
+        resourceFilter.maybeFilterDependencies(transitiveResources);
 
-    if (filteredResources == transitiveResources) {
+    if (!filteredResources.isPresent()) {
       // No filtering was done.
       return this;
     }
@@ -369,11 +366,22 @@ public final class ResourceDependencies {
     // Note that this doesn't filter any of the dependent artifacts. This
     // means that if any resource changes, the corresponding actions will get
     // re-executed
+    return withResources(
+        resourceFilter.filterDependencyContainers(transitiveResourceContainers),
+        resourceFilter.filterDependencyContainers(directResourceContainers),
+        filteredResources.get());
+  }
+
+  @VisibleForTesting
+  ResourceDependencies withResources(
+      NestedSet<ResourceContainer> directResourceContainers,
+      NestedSet<ResourceContainer> transitiveResourceContainers,
+      NestedSet<Artifact> transitiveResources) {
     return new ResourceDependencies(
         neverlink,
-        filter.filterDependencyContainers(ruleErrorConsumer, transitiveResourceContainers),
-        filter.filterDependencyContainers(ruleErrorConsumer, directResourceContainers),
-        filteredResources,
+        transitiveResourceContainers,
+        directResourceContainers,
+        transitiveResources,
         transitiveAssets,
         transitiveManifests,
         transitiveAapt2RTxt,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java
new file mode 100644
index 0000000000..74f7b27f67
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilter.java
@@ -0,0 +1,109 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// 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.google.devtools.build.lib.rules.android;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+
+/** Filters containers of android resources. */
+public class ResourceFilter {
+  private final ImmutableSet<Artifact> acceptedResources;
+  private final Consumer<Artifact> filteredDependencyConsumer;
+  private final boolean isEmpty;
+
+  static final ResourceFilter empty() {
+    return new ResourceFilter(ImmutableSet.of(), (artifact -> {}), /* isEmpty= */ true);
+  }
+
+  static final ResourceFilter of(
+      ImmutableSet<Artifact> acceptedResources, Consumer<Artifact> filteredDependencyConsumer) {
+    return new ResourceFilter(acceptedResources, filteredDependencyConsumer, /* isEmpty= */ false);
+  }
+
+  private ResourceFilter(
+      ImmutableSet<Artifact> acceptedResources,
+      Consumer<Artifact> filteredDependencyConsumer,
+      boolean isEmpty) {
+    this.acceptedResources = acceptedResources;
+    this.filteredDependencyConsumer = filteredDependencyConsumer;
+    this.isEmpty = isEmpty;
+  }
+
+  public Optional<NestedSet<Artifact>> maybeFilterDependencies(NestedSet<Artifact> artifacts) {
+    if (isEmpty) {
+      return Optional.empty();
+    }
+
+    List<Artifact> asList = artifacts.toList();
+    List<Artifact> filtered =
+        asList.stream().filter(acceptedResources::contains).collect(Collectors.toList());
+    if (filtered.size() == asList.size()) {
+      // No filtering needs to be done
+      return Optional.empty();
+    }
+
+    return Optional.of(NestedSetBuilder.wrap(artifacts.getOrder(), filtered));
+  }
+
+  public NestedSet<ResourceContainer> filterDependencyContainers(
+      NestedSet<ResourceContainer> resourceContainers) {
+    if (isEmpty) {
+      return resourceContainers;
+    }
+
+    NestedSetBuilder<ResourceContainer> builder =
+        new NestedSetBuilder<>(resourceContainers.getOrder());
+
+    for (ResourceContainer container : resourceContainers) {
+      builder.add(container.filter(this, /* isDependency = */ true));
+    }
+
+    return builder.build();
+  }
+
+  Optional<ImmutableList<Artifact>> maybeFilter(
+      ImmutableList<Artifact> artifacts, boolean isDependency) {
+    if (isEmpty) {
+      return Optional.empty();
+    }
+
+    boolean removedAny = false;
+    ImmutableList.Builder<Artifact> filtered = ImmutableList.builder();
+
+    for (Artifact artifact : artifacts) {
+      if (acceptedResources.contains(artifact)) {
+        filtered.add(artifact);
+      } else {
+        removedAny = true;
+        if (isDependency) {
+          filteredDependencyConsumer.accept(artifact);
+        }
+      }
+    }
+
+    if (!removedAny) {
+      // No filtering was done, return the original
+      return Optional.empty();
+    }
+
+    return Optional.of(filtered.build());
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
index c252c94204..a77946803e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceFilterFactory.java
@@ -22,26 +22,24 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.PatchTransition;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.packages.AttributeMap;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.RuleErrorConsumer;
 import com.google.devtools.build.lib.syntax.Type;
-import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
+import java.util.Collection;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.annotation.Nullable;
@@ -395,75 +393,35 @@ public class ResourceFilterFactory {
   }
 
   /**
-   * Filters a NestedSet of resource containers that contain dependencies of the current rule. This
-   * may be a no-op if this filter is empty or if resource prefiltering is disabled.
-   */
-  NestedSet<ResourceContainer> filterDependencyContainers(
-      RuleErrorConsumer ruleErrorConsumer, NestedSet<ResourceContainer> resources) {
-    if (!shouldFilterDependencies()) {
-      return resources;
-    }
-
-    NestedSetBuilder<ResourceContainer> builder = new NestedSetBuilder<>(resources.getOrder());
-
-    for (ResourceContainer resource : resources) {
-      builder.add(resource.filter(ruleErrorConsumer, this));
-    }
-
-    return builder.build();
-  }
-
-  /**
-   * Filters a NestedSet of artifact dependencies of the current rule. Returns a filtered copy of
-   * the input, or the input itself if no filtering needs to be done.
+   * Gets an {@link ResourceFilter} that can be used to filter collections of artifacts.
+   *
+   * <p>In density-based filtering, the presence of one resource can affect whether another is
+   * accepted or rejected. As such, both local and dependent resources must be passed.
    */
-  NestedSet<Artifact> filterDependencies(
-      RuleErrorConsumer ruleErrorConsumer, NestedSet<Artifact> resources) {
-    if (!shouldFilterDependencies()) {
-      return resources;
-    }
-
-    return NestedSetBuilder.wrap(
-        resources.getOrder(), filter(ruleErrorConsumer, ImmutableList.copyOf(resources)));
-  }
-
-  private boolean shouldFilterDependencies() {
-    if (!isPrefiltering() || usesDynamicConfiguration()) {
-      /*
-       * If the filter is empty, resource prefiltering is disabled, or the resources of dependencies
-       * have already been filtered thanks to dynamic configuration, just return the original,
-       * rather than make a copy.
-       *
-       * Resources should only be prefiltered in top-level android targets (such as android_binary).
-       * The output of resource processing, which includes the input NestedSet<ResourceContainer>
-       * returned by this method, is exposed to other actions via the AndroidResourcesProvider. If
-       * this method did a no-op copy and collapse in those cases, rather than just return the
-       * original NestedSet, we would lose all of the advantages around memory and time that
-       * NestedSets provide: each android_library target would have to copy the resources provided
-       * by its dependencies into a new NestedSet rather than just create a NestedSet pointing at
-       * its dependencies's NestedSets.
-       */
-      return false;
-    }
-
-    return true;
-  }
-
-  ImmutableList<Artifact> filter(
-      RuleErrorConsumer ruleErrorConsumer, ImmutableList<Artifact> artifacts) {
+  ResourceFilter getResourceFilter(
+      RuleErrorConsumer ruleErrorConsumer,
+      ResourceDependencies resourceDeps,
+      LocalResourceContainer localResources)
+      throws RuleErrorException {
     if (!isPrefiltering()) {
-      return artifacts;
+      return ResourceFilter.empty();
     }
 
+    ImmutableList<FolderConfiguration> folderConfigs = getConfigurationFilters(ruleErrorConsumer);
+
+    ImmutableSet.Builder<Artifact> keptArtifacts = ImmutableSet.builder();
     List<BestArtifactsForDensity> bestArtifactsForAllDensities = new ArrayList<>();
     for (Density density : getDensities(ruleErrorConsumer)) {
       bestArtifactsForAllDensities.add(new BestArtifactsForDensity(ruleErrorConsumer, density));
     }
 
-    ImmutableList<FolderConfiguration> folderConfigs = getConfigurationFilters(ruleErrorConsumer);
-
-    Set<Artifact> keptArtifactsNotFilteredByDensity = new HashSet<>();
-    for (Artifact artifact : artifacts) {
+    // Look at the local and transitive resources.
+    // TODO(b/68265485): In FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION, this will collapse the
+    // NestedSet of resources at each node of the build graph. Instead, we should only filter local
+    // resources at each non-binary target, and then filter both local and transitive resources in
+    // the binary.
+    for (Artifact artifact :
+        Iterables.concat(localResources.getResources(), resourceDeps.getTransitiveResources())) {
       FolderConfiguration config = getConfigForArtifact(ruleErrorConsumer, artifact);
 
       // aapt explicitly ignores the version qualifier; duplicate this behavior here.
@@ -474,7 +432,7 @@ public class ResourceFilterFactory {
       }
 
       if (!shouldFilterByDensity(artifact)) {
-        keptArtifactsNotFilteredByDensity.add(artifact);
+        keptArtifacts.add(artifact);
         continue;
       }
 
@@ -483,46 +441,33 @@ public class ResourceFilterFactory {
       }
     }
 
-    // Build the output by iterating through the input so that contents of both have the same order.
-    ImmutableList.Builder<Artifact> builder = ImmutableList.builder();
-    for (Artifact artifact : artifacts) {
-
-      boolean kept = false;
-      if (keptArtifactsNotFilteredByDensity.contains(artifact)) {
-        builder.add(artifact);
-        kept = true;
-      } else {
-        for (BestArtifactsForDensity bestArtifactsForDensity : bestArtifactsForAllDensities) {
-          if (bestArtifactsForDensity.contains(artifact)) {
-            builder.add(artifact);
-            kept = true;
-            break;
-          }
-        }
-      }
-
-      // In FilterBehavior.FILTER_IN_ANALYSIS, this class needs to record any resources that were
-      // filtered out so that resource processing ignores references to them in symbols files of
-      // dependencies.
-      if (!kept && !usesDynamicConfiguration()) {
-        String parentDir = artifact.getPath().getParentDirectory().getBaseName();
-        filteredResources.add(parentDir + "/" + artifact.getFilename());
-      }
+    for (BestArtifactsForDensity bestArtifactsForDensity : bestArtifactsForAllDensities) {
+      keptArtifacts.addAll(bestArtifactsForDensity.get());
     }
 
-    // TODO(asteinb): We should only build a new list if some artifacts were filtered out. If
-    // nothing was filtered, we can be more efficient by returning the original list instead.
-    return builder.build();
+    return ResourceFilter.of(
+        keptArtifacts.build(),
+        (artifact) -> {
+          // This class needs to record any dependent resources that were filtered out so that
+          // resource processing ignores references to them in symbols files of  dependencies.
+          String parentDir = artifact.getPath().getParentDirectory().getBaseName();
+          filteredResources.add(parentDir + "/" + artifact.getFilename());
+        });
   }
 
   /**
-   * Tracks the best artifact for a desired density for each combination of filename and non-density
-   * qualifiers.
+   * Tracks the best artifacts for a desired density for each combination of filename and
+   * non-density qualifiers.
+   *
+   * <p>Filtering resources from multiple targets at once means we may get multiple resources with
+   * the same filename and qualifiers (including density) - we accept all of them with the best
+   * density, and the unwanted ones will be removed during resource merging.
    */
   private class BestArtifactsForDensity {
     private final RuleErrorConsumer ruleErrorConsumer;
     private final Density desiredDensity;
-    private final Map<String, Artifact> nameAndConfigurationToBestArtifact = new HashMap<>();
+    private final Multimap<String, Artifact> nameAndConfigurationToBestArtifacts =
+        HashMultimap.create();
 
     public BestArtifactsForDensity(RuleErrorConsumer ruleErrorConsumer, Density density) {
       this.ruleErrorConsumer = ruleErrorConsumer;
@@ -538,25 +483,38 @@ public class ResourceFilterFactory {
 
       // We want to find a single best artifact for each combination of non-density qualifiers and
       // filename. Combine those two values to create a single unique key.
-      // We also need to include the path to the resource, otherwise resource conflicts (multiple
-      // resources with the same name but different locations) might accidentally get resolved here
-      // (possibly incorrectly). Resource conflicts should be resolve during merging in execution
-      // instead.
+      // We might encounter resource conflicts (multiple resources with the same name but different
+      // locations) - in that case, we might have multiple best artifacts. In that case, keep them
+      // all, and resource conflicts should will resolved during merging in execution.
       config.setDensityQualifier(null);
-      Path qualifierDir = artifact.getPath().getParentDirectory();
-      String resourceDir = qualifierDir.getParentDirectory().toString();
       String nameAndConfiguration =
-          Joiner.on('/').join(resourceDir, config.getUniqueKey(), artifact.getFilename());
+          Joiner.on('/').join(config.getUniqueKey(), artifact.getFilename());
+
+      Collection<Artifact> currentBest =
+          nameAndConfigurationToBestArtifacts.get(nameAndConfiguration);
+
+      if (currentBest.isEmpty()) {
+        nameAndConfigurationToBestArtifacts.put(nameAndConfiguration, artifact);
+        return;
+      }
+
+      double affinity = computeAffinity(artifact);
+      // All of the current best artifacts should have the same density, so get the affinity of an
+      // arbitrary one.
+      double currentAffinity = computeAffinity(currentBest.iterator().next());
 
-      Artifact currentBest = nameAndConfigurationToBestArtifact.get(nameAndConfiguration);
+      if (affinity == currentAffinity) {
+        nameAndConfigurationToBestArtifacts.put(nameAndConfiguration, artifact);
+      }
 
-      if (currentBest == null || computeAffinity(artifact) < computeAffinity(currentBest)) {
-        nameAndConfigurationToBestArtifact.put(nameAndConfiguration, artifact);
+      if (affinity < currentAffinity) {
+        nameAndConfigurationToBestArtifacts.removeAll(nameAndConfiguration);
+        nameAndConfigurationToBestArtifacts.put(nameAndConfiguration, artifact);
       }
     }
 
-    public boolean contains(Artifact artifact) {
-      return nameAndConfigurationToBestArtifact.containsValue(artifact);
+    public Collection<Artifact> get() {
+      return nameAndConfigurationToBestArtifacts.values();
     }
 
     /**
@@ -734,15 +692,14 @@ public class ResourceFilterFactory {
     return filterBehavior == FilterBehavior.FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION;
   }
 
-  /*
-   * TODO: Stop tracking these once {@link FilterBehavior#FILTER_IN_ANALYSIS} is fully replaced by
-   * {@link FilterBehavior#FILTER_IN_ANALYSIS_WITH_DYNAMIC_CONFIGURATION}.
+  /**
+   * Gets a list of resource names that should be ignored by resource processing if they don't
+   * exist.
    *
-   * <p>Currently, when using {@link FilterBehavior#FILTER_IN_ANALYSIS}, android_library targets do
-   * no filtering, and all resources are built into their symbol files. The android_binary target
-   * filters out these resources in analysis. However, the filtered resources must be passed to
-   * resource processing at execution time so the code knows to ignore resources that were filtered
-   * out. Without this, resource processing code would see references to those resources in
+   * <p>A target might filter out some of its dependency's targets. However, those filtered targets
+   * have already been built into symbols files. The filtered resources must be passed to resource
+   * processing at execution time so the code knows to ignore resources that were filtered out.
+   * Without this, resource processing code would see references to those resources in
    * dependencies's symbol files, but then be unable to follow those references or know whether they
    * were missing due to resource filtering or a bug.
    */