Tool attribute processing added to resource shrinking.

RELNOTES: None
PiperOrigin-RevId: 207171755

5 files changed, 169 insertions, 76 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/ResourcesZip.java b/src/tools/android/java/com/google/devtools/build/android/ResourcesZip.java
index ee8e57df8f..3e1ecda49e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/ResourcesZip.java
+++ b/src/tools/android/java/com/google/devtools/build/android/ResourcesZip.java
@@ -14,9 +14,13 @@
 package com.google.devtools.build.android;
 
 import static com.google.common.base.Predicates.not;
+import static java.util.stream.Collectors.toMap;
 
+import com.android.SdkConstants;
+import com.android.annotations.VisibleForTesting;
 import com.android.build.gradle.tasks.ResourceUsageAnalyzer;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.android.AndroidResourceOutputs.ZipBuilder;
 import com.google.devtools.build.android.AndroidResourceOutputs.ZipBuilderVisitorWithDirectories;
@@ -25,12 +29,15 @@ import com.google.devtools.build.android.aapt2.ProtoApk;
 import com.google.devtools.build.android.aapt2.ProtoResourceUsageAnalyzer;
 import com.google.devtools.build.android.aapt2.ResourceCompiler;
 import com.google.devtools.build.android.aapt2.ResourceLinker;
+import com.google.devtools.build.android.proto.SerializeFormat.ToolAttributes;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.logging.Logger;
@@ -245,13 +252,28 @@ public class ResourcesZip {
       throws ParserConfigurationException, IOException, SAXException {
     final Path shrunkApkProto = workingDirectory.resolve("shrunk.apk.pb");
     try (final ProtoApk apk = ProtoApk.readFrom(proto)) {
+      final Map<String, Set<String>> toolAttributes = toAttributes();
       // record resources and manifest
       new ProtoResourceUsageAnalyzer(packages, proguardMapping, logFile)
-          .shrink(apk, classJar, shrunkApkProto);
+          .shrink(
+              apk,
+              classJar,
+              shrunkApkProto,
+              toolAttributes.getOrDefault(SdkConstants.ATTR_KEEP, ImmutableSet.of()),
+              toolAttributes.getOrDefault(SdkConstants.ATTR_DISCARD, ImmutableSet.of()));
       return new ShrunkProtoApk(shrunkApkProto, logFile);
     }
   }
 
+  @VisibleForTesting
+  public Map<String, Set<String>> toAttributes() throws IOException {
+    return ToolAttributes.parseFrom(Files.readAllBytes(attributes))
+        .getAttributesMap()
+        .entrySet()
+        .stream()
+        .collect(toMap(Entry::getKey, e -> ImmutableSet.copyOf(e.getValue().getValuesList())));
+  }
+
   static class ShrunkProtoApk {
     private final Path apk;
     private final Path report;
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoApk.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoApk.java
index 33bb6e1bba..4b07ce3a9f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoApk.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoApk.java
@@ -434,7 +434,7 @@ public class ProtoApk implements Closeable {
     } else if (!ref.getName().isEmpty()) {
       visitor.accept(ref.getName());
     } else {
-      throw new IllegalStateException("Reference without number or id in :" + ref);
+      visitor.acceptNullReference();
     }
   }
 
@@ -495,8 +495,8 @@ public class ProtoApk implements Closeable {
     /** Called when a reference is defined by id (full id, with package and type.) */
     void accept(int value);
 
-    /** Called when a reference has no id or name. */
-    default void acceptEmptyReference() {
+    /** Called when a reference is null. */
+    default void acceptNullReference() {
       // pass
     }
   }
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoResourceUsageAnalyzer.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoResourceUsageAnalyzer.java
index af81ce0305..171bbf3791 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoResourceUsageAnalyzer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ProtoResourceUsageAnalyzer.java
@@ -14,11 +14,14 @@
 
 package com.google.devtools.build.android.aapt2;
 
+import static java.util.stream.Collectors.joining;
+
 import com.android.build.gradle.tasks.ResourceUsageAnalyzer;
 import com.android.resources.ResourceType;
 import com.android.tools.lint.checks.ResourceUsageModel;
 import com.android.tools.lint.checks.ResourceUsageModel.Resource;
 import com.android.tools.lint.detector.api.LintUtils;
+import com.android.utils.XmlUtils;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSet;
@@ -27,17 +30,20 @@ import com.google.devtools.build.android.aapt2.ProtoApk.ReferenceVisitor;
 import com.google.devtools.build.android.aapt2.ProtoApk.ResourcePackageVisitor;
 import com.google.devtools.build.android.aapt2.ProtoApk.ResourceValueVisitor;
 import com.google.devtools.build.android.aapt2.ProtoApk.ResourceVisitor;
-import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import java.util.logging.Logger;
 import javax.xml.parsers.ParserConfigurationException;
+import org.w3c.dom.Attr;
 import org.w3c.dom.DOMException;
+import org.w3c.dom.NamedNodeMap;
+import org.xml.sax.SAXException;
 
 /** A resource usage analyzer tha functions on apks in protocol buffer format. */
 public class ProtoResourceUsageAnalyzer extends ResourceUsageAnalyzer {
@@ -63,8 +69,16 @@ public class ProtoResourceUsageAnalyzer extends ResourceUsageAnalyzer {
    * @param apk An apk in the aapt2 proto format.
    * @param classes The associated classes for the apk.
    * @param destination Where to write the reduced resources.
+   * @param keep A list of resource urls to keep, unused or not.
+   * @param discard A list of resource urls to always discard.
    */
-  public void shrink(ProtoApk apk, Path classes, Path destination) throws IOException {
+  public void shrink(
+      ProtoApk apk,
+      Path classes,
+      Path destination,
+      Collection<String> keep,
+      Collection<String> discard)
+      throws IOException, ParserConfigurationException, SAXException {
 
     // record resources and manifest
     apk.visitResources(
@@ -75,6 +89,22 @@ public class ProtoResourceUsageAnalyzer extends ResourceUsageAnalyzer {
 
     recordClassUsages(classes);
 
+    // Have to give the model xml attributes with keep and discard urls.
+    final NamedNodeMap toolAttributes =
+        XmlUtils.parseDocument(
+                String.format(
+                    "<resources xmlns:tools='http://schemas.android.com/tools' tools:keep='%s'"
+                        + " tools:discard='%s'></resources>",
+                    keep.stream().collect(joining(",")), discard.stream().collect(joining(","))),
+                true)
+            .getDocumentElement()
+            .getAttributes();
+
+    for (int i = 0; i < toolAttributes.getLength(); i++) {
+      model().recordToolsAttributes((Attr) toolAttributes.item(i));
+    }
+    model().processToolsAttributes();
+
     keepPossiblyReferencedResources();
 
     dumpReferences();
@@ -187,7 +217,6 @@ public class ProtoResourceUsageAnalyzer extends ResourceUsageAnalyzer {
 
     @Override
     public ReferenceVisitor entering(Path path) {
-      declaredResource.addLocation(new File(path.toString()));
       return this;
     }
 
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
index e33196d1a1..f4fd76713c 100644
--- a/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/ResourceLinker.java
@@ -21,28 +21,20 @@ import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
-import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Multimap;
 import com.google.common.collect.Streams;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.devtools.build.android.AaptCommandBuilder;
 import com.google.devtools.build.android.AndroidCompiledDataDeserializer;
-import com.google.devtools.build.android.AndroidDataWritingVisitor;
-import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
 import com.google.devtools.build.android.AndroidResourceOutputs;
 import com.google.devtools.build.android.FullyQualifiedName;
 import com.google.devtools.build.android.Profiler;
 import com.google.devtools.build.android.aapt2.ResourceCompiler.CompiledType;
-import com.google.devtools.build.android.proto.SerializeFormat.ToolAttributes;
-import com.google.devtools.build.android.xml.Namespaces;
 import com.google.devtools.build.android.ziputils.DirectoryEntry;
 import com.google.devtools.build.android.ziputils.ZipIn;
 import com.google.devtools.build.android.ziputils.ZipOut;
-import java.io.BufferedOutputStream;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.nio.channels.FileChannel;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -50,7 +42,6 @@ import java.nio.file.StandardOpenOption;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -452,7 +443,7 @@ public class ResourceLinker {
 
       final Path attributes = workingDirectory.resolve("tool.attributes");
       // extract tool annotations from the compile resources.
-      final ToolProtoWriter writer = new ToolProtoWriter(attributes);
+      final SdkToolAttributeWriter writer = new SdkToolAttributeWriter(attributes);
       Stream.concat(include.stream(), Stream.of(compiled))
           .parallel()
           .map(AndroidCompiledDataDeserializer.create()::readAttributes)
@@ -533,63 +524,4 @@ public class ResourceLinker {
         .add("baseApk", baseApk)
         .toString();
   }
-
-  private static class ToolProtoWriter implements AndroidDataWritingVisitor {
-
-    final Multimap<String, String> attributes = HashMultimap.create();
-    private final Path out;
-
-    ToolProtoWriter(Path out) {
-      this.out = out;
-    }
-
-    @Override
-    public void flush() throws IOException {
-      ToolAttributes.Builder builder = ToolAttributes.newBuilder();
-      for (Entry<String, Collection<String>> entry : attributes.asMap().entrySet()) {
-        builder.putAttributes(
-            entry.getKey(),
-            ToolAttributes.ToolAttributeValues.newBuilder().addAllValues(entry.getValue()).build());
-      }
-      try (OutputStream stream = new BufferedOutputStream(Files.newOutputStream(out))) {
-        builder.build().writeTo(stream);
-      }
-    }
-
-    @Override
-    public Path copyManifest(Path sourceManifest) {
-      throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public void copyAsset(Path source, String relativeDestinationPath) {
-      throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public void copyResource(Path source, String relativeDestinationPath) throws MergingException {
-      throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public void defineAttribute(FullyQualifiedName fqn, String name, String value) {
-      attributes.put(removeNamespace(name), value);
-    }
-
-    private String removeNamespace(String qualifiedName) {
-      int indexColon = qualifiedName.indexOf(':');
-      if (indexColon == -1) {
-        return qualifiedName;
-      }
-      return qualifiedName.substring(indexColon);
-    }
-
-    @Override
-    public void defineNamespacesFor(FullyQualifiedName fqn, Namespaces namespaces) {}
-
-    @Override
-    public ValueResourceDefinitionMetadata define(FullyQualifiedName fqn) {
-      throw new UnsupportedOperationException();
-    }
-  }
 }
diff --git a/src/tools/android/java/com/google/devtools/build/android/aapt2/SdkToolAttributeWriter.java b/src/tools/android/java/com/google/devtools/build/android/aapt2/SdkToolAttributeWriter.java
new file mode 100644
index 0000000000..3f4f9dec34
--- /dev/null
+++ b/src/tools/android/java/com/google/devtools/build/android/aapt2/SdkToolAttributeWriter.java
@@ -0,0 +1,110 @@
+// Copyright 2018 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.android.aapt2;
+
+import com.android.SdkConstants;
+import com.google.common.base.Splitter;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.MultimapBuilder;
+import com.google.devtools.build.android.AndroidDataWritingVisitor;
+import com.google.devtools.build.android.AndroidResourceMerger.MergingException;
+import com.google.devtools.build.android.FullyQualifiedName;
+import com.google.devtools.build.android.proto.SerializeFormat.ToolAttributes;
+import com.google.devtools.build.android.xml.Namespaces;
+import java.io.BufferedOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.Map.Entry;
+import java.util.function.Function;
+import javax.xml.namespace.QName;
+
+/**
+ * An AndroidDataWritingVisitor that only records {@link SdkConstants#TOOLS_URI} attributes to the
+ * {@link ToolAttributes} proto format. Comma delimited values {@link SdkConstants#ATTR_KEEP} and
+ * {@link SdkConstants#ATTR_DISCARD} will be expanded into separate values.
+ */
+class SdkToolAttributeWriter implements AndroidDataWritingVisitor {
+
+  private static final Splitter COMMA_SPLITTER = Splitter.on(',');
+
+  private static final Function<String, Iterable<String>> DEFAULT_ATTRIBUTE_PROCESSOR =
+      ImmutableList::of;
+
+  private static final ImmutableMap<String, Function<String, Iterable<String>>>
+      ATTRIBUTE_PROCESSORS =
+          ImmutableMap.of(
+              SdkConstants.ATTR_KEEP, COMMA_SPLITTER::split,
+              SdkConstants.ATTR_DISCARD, COMMA_SPLITTER::split);
+
+  final Multimap<String, String> attributes = MultimapBuilder.hashKeys().hashSetValues().build();
+  private final Path out;
+
+  SdkToolAttributeWriter(Path out) {
+    this.out = out;
+  }
+
+  @Override
+  public void flush() throws IOException {
+    ToolAttributes.Builder builder = ToolAttributes.newBuilder();
+    for (Entry<String, Collection<String>> entry : attributes.asMap().entrySet()) {
+      builder.putAttributes(
+          entry.getKey(),
+          ToolAttributes.ToolAttributeValues.newBuilder().addAllValues(entry.getValue()).build());
+    }
+    try (OutputStream stream = new BufferedOutputStream(Files.newOutputStream(out))) {
+      builder.build().writeTo(stream);
+    }
+  }
+
+  @Override
+  public Path copyManifest(Path sourceManifest) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void copyAsset(Path source, String relativeDestinationPath) throws IOException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void copyResource(Path source, String relativeDestinationPath) throws MergingException {
+    throw new UnsupportedOperationException();
+  }
+
+  @Override
+  public void defineAttribute(FullyQualifiedName fqn, String name, String value) {
+    final QName qName = QName.valueOf(fqn.name());
+    if (SdkConstants.TOOLS_URI.equals(qName.getNamespaceURI())) {
+      attributes.putAll(
+          qName.getLocalPart(),
+          ATTRIBUTE_PROCESSORS
+              .getOrDefault(qName.getLocalPart(), DEFAULT_ATTRIBUTE_PROCESSOR)
+              .apply(value));
+    }
+  }
+
+  @Override
+  public void defineNamespacesFor(FullyQualifiedName fqn, Namespaces namespaces) {}
+
+  @Override
+  public ValueResourceDefinitionMetadata define(FullyQualifiedName fqn) {
+    throw new UnsupportedOperationException();
+  }
+}