Update ApkSubject to use the AndroidCompiledDataDeserializer for proto apks.

Minor fixes to the AndroidCompiledDataDeserializer

RELNOTES: None
PiperOrigin-RevId: 193535766

6 files changed, 141 insertions, 58 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidCompiledDataDeserializer.java b/src/tools/android/java/com/google/devtools/build/android/AndroidCompiledDataDeserializer.java
index ec29c67fb7..9cf70c409f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidCompiledDataDeserializer.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidCompiledDataDeserializer.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.android;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Predicates.not;
 import static java.util.stream.Collectors.toList;
 
 import android.aapt.pb.internal.ResourcesInternal.CompiledFile;
@@ -28,6 +30,7 @@ import com.android.aapt.ConfigurationOuterClass.Configuration.UiModeNight;
 import com.android.aapt.ConfigurationOuterClass.Configuration.UiModeType;
 import com.android.aapt.Resources;
 import com.android.aapt.Resources.ConfigValue;
+import com.android.aapt.Resources.Entry;
 import com.android.aapt.Resources.Package;
 import com.android.aapt.Resources.ResourceTable;
 import com.android.aapt.Resources.Type;
@@ -80,6 +83,7 @@ import com.google.devtools.build.android.FullyQualifiedName.Factory;
 import com.google.devtools.build.android.proto.SerializeFormat;
 import com.google.devtools.build.android.proto.SerializeFormat.Header;
 import com.google.devtools.build.android.xml.ResourcesAttribute.AttributeType;
+import com.google.protobuf.InvalidProtocolBufferException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
@@ -97,10 +101,12 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
+import javax.annotation.concurrent.NotThreadSafe;
 
 /** Deserializes {@link DataKey}, {@link DataValue} entries from compiled resource files. */
 public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer {
@@ -247,14 +253,20 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
     resourceTableStream.read(tableBytes, 0, (int) alignedSize);
     ResourceTable resourceTable = ResourceTable.parseFrom(tableBytes);
 
+    readPackages(consumers, resourceTable);
+  }
+
+  private void readPackages(KeyValueConsumers consumers, ResourceTable resourceTable)
+      throws UnsupportedEncodingException, InvalidProtocolBufferException {
     List<String> sourcePool =
         decodeSourcePool(resourceTable.getSourcePool().getData().toByteArray());
-
-    Map<String, Boolean> qualifiedReferenceInlineStatus = new HashMap<>();
+    ReferenceResolver resolver = ReferenceResolver.asRoot();
 
     for (int i = resourceTable.getPackageCount() - 1; i >= 0; i--) {
       Package resourceTablePackage = resourceTable.getPackage(i);
 
+      ReferenceResolver packageResolver =
+          resolver.resolveFor(resourceTablePackage.getPackageName());
       String packageName = resourceTablePackage.getPackageName();
 
       for (Type resourceFormatType : resourceTablePackage.getTypeList()) {
@@ -264,7 +276,7 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
           if (resource.getConfigValueList().isEmpty()
               && resource.getVisibility().getLevel() == Level.PUBLIC) {
 
-            // Public resource definition.
+            // This is a public resource definition.
             int sourceIndex = resource.getVisibility().getSource().getPathIdx();
 
             String source = sourcePool.get(sourceIndex);
@@ -274,18 +286,14 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
                 DataResourceXml.fromPublic(dataSource, resourceType, resource.getEntryId().getId());
             final FullyQualifiedName fqn =
                 createAndRecordFqn(
-                    qualifiedReferenceInlineStatus,
-                    packageName,
-                    resourceType,
-                    resource,
-                    ImmutableList.of());
+                    packageResolver, packageName, resourceType, resource, ImmutableList.of());
             consumers.combiningConsumer.accept(fqn, dataResourceXml);
           } else if (!"android".equals(packageName)) {
             // This means this resource is not in the android sdk, add it to the set.
             for (ConfigValue configValue : resource.getConfigValueList()) {
               FullyQualifiedName fqn =
                   createAndRecordFqn(
-                      qualifiedReferenceInlineStatus,
+                      packageResolver,
                       packageName,
                       resourceType,
                       resource,
@@ -297,24 +305,23 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
               DataSource dataSource = DataSource.of(Paths.get(source));
 
               Value resourceValue = resource.getConfigValue(0).getValue();
-              DataResourceXml dataResourceXml =
-                  DataResourceXml.from(
-                      resourceValue, dataSource, resourceType, qualifiedReferenceInlineStatus);
+
+              DataResource dataResource =
+                  resourceValue.getItem().hasFile()
+                      ? DataValueFile.of(dataSource)
+                      : DataResourceXml.from(
+                          resourceValue, dataSource, resourceType, packageResolver);
 
               if (!fqn.isOverwritable()) {
-                consumers.combiningConsumer.accept(fqn, dataResourceXml);
+                consumers.combiningConsumer.accept(fqn, dataResource);
               } else {
-                consumers.overwritingConsumer.accept(fqn, dataResourceXml);
+                consumers.overwritingConsumer.accept(fqn, dataResource);
               }
             }
           } else {
             // In the sdk, just add the fqn for styleables
             createAndRecordFqn(
-                    qualifiedReferenceInlineStatus,
-                    packageName,
-                    resourceType,
-                    resource,
-                    ImmutableList.of())
+                    packageResolver, packageName, resourceType, resource, ImmutableList.of())
                 .toPrettyString();
           }
         }
@@ -322,22 +329,76 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
     }
   }
 
+  /** Maintains state for all references in each package of a resource table. */
+  @NotThreadSafe
+  public static class ReferenceResolver {
+
+    enum InlineStatus {
+      INLINEABLE,
+      INLINED,
+    }
+
+    private final Optional<String> packageName;
+    private final Map<FullyQualifiedName, InlineStatus> qualifiedReferenceInlineStatus;
+
+    private ReferenceResolver(
+        Optional<String> packageName,
+        Map<FullyQualifiedName, InlineStatus> qualifiedReferenceInlineStatus) {
+      this.packageName = packageName;
+      this.qualifiedReferenceInlineStatus = qualifiedReferenceInlineStatus;
+    }
+
+    static ReferenceResolver asRoot() {
+      return new ReferenceResolver(Optional.empty(), new HashMap<>());
+    }
+
+    public ReferenceResolver resolveFor(String packageName) {
+      return new ReferenceResolver(
+          Optional.of(packageName).filter(not(String::isEmpty)), qualifiedReferenceInlineStatus);
+    }
+
+    public FullyQualifiedName parse(String reference) {
+      return FullyQualifiedName.fromReference(reference, packageName);
+    }
+
+    public FullyQualifiedName register(FullyQualifiedName fullyQualifiedName) {
+      // The default is that the name can be inlined.
+      qualifiedReferenceInlineStatus.put(fullyQualifiedName, InlineStatus.INLINEABLE);
+      return fullyQualifiedName;
+    }
+
+    /** Indicates if a reference can be inlined in a styleable. */
+    public boolean shouldInline(FullyQualifiedName reference) {
+      return checkNotNull(
+                  qualifiedReferenceInlineStatus.get(reference),
+                  "%s reference is unsatisfied. Available names: %s",
+                  reference,
+                  qualifiedReferenceInlineStatus.keySet())
+              .equals(InlineStatus.INLINEABLE)
+          // Only inline if it's in the current package.
+          && reference.isInPackage(packageName.orElse(FullyQualifiedName.DEFAULT_PACKAGE));
+    }
+
+    /** Update the reference's inline state. */
+    public FullyQualifiedName markInlined(FullyQualifiedName reference) {
+      qualifiedReferenceInlineStatus.put(reference, InlineStatus.INLINED);
+      return reference;
+    }
+  }
+
   private FullyQualifiedName createAndRecordFqn(
-      Map<String, Boolean> qualifiedReferenceInlineStatus,
+      ReferenceResolver packageResolver,
       String packageName,
       ResourceType resourceType,
-      Resources.Entry resource,
-      List<String> of) {
-    Preconditions.checkArgument(!packageName.contains(":"));
+      Entry resource,
+      List<String> qualifiers) {
     final FullyQualifiedName fqn =
         FullyQualifiedName.of(
             packageName.isEmpty() ? FullyQualifiedName.DEFAULT_PACKAGE : packageName,
-            of,
+            qualifiers,
             resourceType,
             resource.getName());
-    // Record if the definition of the attr is defined in styleable.
-    // Currently, we consider any reference without a package as being defined inline.
-    qualifiedReferenceInlineStatus.put(fqn.asQualifiedReference(), packageName.isEmpty());
+    packageResolver.register(fqn);
     return fqn;
   }
 
@@ -358,9 +419,11 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
                   protoConfig.getMnc() == 0xffff ? 0 : protoConfig.getMnc())));
     }
 
-    // locales are the wild, wild west: no enums.
     if (!protoConfig.getLocale().isEmpty()) {
-      new LocaleQualifier().checkAndSet(protoConfig.getLocale(), configuration);
+      // The proto stores it in a BCP-47 format, but the parser requires a b+ and all the - as +.
+      // It's a nice a little impedance mismatch.
+      new LocaleQualifier()
+          .checkAndSet("b+" + protoConfig.getLocale().replaceAll("-", "+"), configuration);
     }
 
     if (LAYOUT_DIRECTION_MAP.containsKey(protoConfig.getLayoutDirection())) {
@@ -541,6 +604,11 @@ public class AndroidCompiledDataDeserializer implements AndroidDataDeserializer
     }
   }
 
+  public void readTable(InputStream in, KeyValueConsumers consumers) throws IOException {
+    final ResourceTable resourceTable = ResourceTable.parseFrom(in);
+    readPackages(consumers, resourceTable);
+  }
+
   @Override
   public void read(Path inPath, KeyValueConsumers consumers) {
     Stopwatch timer = Stopwatch.createStarted();
diff --git a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
index 59a6fa3504..a3a6f2f494 100644
--- a/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
+++ b/src/tools/android/java/com/google/devtools/build/android/DataResourceXml.java
@@ -22,6 +22,7 @@ import com.android.resources.ResourceType;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.android.AndroidCompiledDataDeserializer.ReferenceResolver;
 import com.google.devtools.build.android.FullyQualifiedName.Factory;
 import com.google.devtools.build.android.FullyQualifiedName.VirtualType;
 import com.google.devtools.build.android.ParsedAndroidData.KeyValueConsumer;
@@ -45,7 +46,6 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Iterator;
-import java.util.Map;
 import java.util.Objects;
 import javax.xml.stream.FactoryConfigurationError;
 import javax.xml.stream.XMLEventReader;
@@ -173,13 +173,11 @@ public class DataResourceXml implements DataResource {
       Value protoValue,
       DataSource source,
       ResourceType resourceType,
-      Map<String, Boolean> fullyQualifiedNames)
+      ReferenceResolver packageResolver)
       throws InvalidProtocolBufferException {
     DataResourceXml dataResourceXml =
         createWithNamespaces(
-            source,
-            valueFromProto(protoValue, resourceType, fullyQualifiedNames),
-            Namespaces.empty());
+            source, valueFromProto(protoValue, resourceType, packageResolver), Namespaces.empty());
     return dataResourceXml;
   }
 
@@ -211,7 +209,7 @@ public class DataResourceXml implements DataResource {
   }
 
   private static XmlResourceValue valueFromProto(
-      Value proto, ResourceType resourceType, Map<String, Boolean> qualifiedReferenceToInlineStatus)
+      Value proto, ResourceType resourceType, ReferenceResolver packageResolver)
       throws InvalidProtocolBufferException {
     switch (resourceType) {
       case STYLE:
@@ -223,7 +221,7 @@ public class DataResourceXml implements DataResource {
       case ATTR:
         return AttrXmlResourceValue.from(proto);
       case STYLEABLE:
-        return StyleableXmlResourceValue.from(proto, qualifiedReferenceToInlineStatus);
+        return StyleableXmlResourceValue.from(proto, packageResolver);
       case ID:
         return IdXmlResourceValue.of();
       case DIMEN:
diff --git a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
index 3663c34673..077b51f17f 100644
--- a/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
+++ b/src/tools/android/java/com/google/devtools/build/android/FullyQualifiedName.java
@@ -14,7 +14,9 @@
 package com.google.devtools.build.android;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Strings.emptyToNull;
 
+import com.android.annotations.VisibleForTesting;
 import com.android.ide.common.resources.configuration.FolderConfiguration;
 import com.android.ide.common.resources.configuration.ResourceQualifier;
 import com.android.resources.ResourceType;
@@ -64,6 +66,7 @@ public class FullyQualifiedName implements DataKey {
   private final String name;
 
   private FullyQualifiedName(String pkg, ImmutableList<String> qualifiers, Type type, String name) {
+    Preconditions.checkArgument(!pkg.isEmpty());
     this.pkg = pkg;
     this.qualifiers = qualifiers;
     this.type = type;
@@ -84,18 +87,20 @@ public class FullyQualifiedName implements DataKey {
   /**
    * Creates a new FullyQualifiedName with normalized qualifiers.
    *
-   * @param pkg The resource package of the name. If unknown the default should be "res-auto"
+   * @param rawPkg The resource package of the name. If unknown the default should be "res-auto"
    * @param qualifiers The resource qualifiers of the name, such as "en" or "xhdpi".
    * @param type The type of the name.
    * @param name The name of the name.
    * @return A new FullyQualifiedName.
    */
-  public static FullyQualifiedName of(String pkg, List<String> qualifiers, Type type, String name) {
-    checkNotNull(pkg);
+  public static FullyQualifiedName of(
+      String rawPkg, List<String> qualifiers, Type type, String name) {
+    checkNotNull(rawPkg);
     checkNotNull(qualifiers);
     checkNotNull(type);
     checkNotNull(name);
     ImmutableList<String> immutableQualifiers = ImmutableList.copyOf(qualifiers);
+    String pkg = rawPkg.isEmpty() ? DEFAULT_PACKAGE : rawPkg;
     // TODO(corysmith): Address the GC thrash this creates by managing a simplified, mutable key to
     // do the instance check.
     FullyQualifiedName fqn = new FullyQualifiedName(pkg, immutableQualifiers, type, name);
@@ -135,7 +140,8 @@ public class FullyQualifiedName implements DataKey {
   static final Pattern QUALIFIED_REFERENCE =
       Pattern.compile("((?<package>[^:]+):)?(?<type>\\w+)/(?<name>\\w+)");
 
-  public static FullyQualifiedName fromReference(String qualifiedReference) {
+  public static FullyQualifiedName fromReference(
+      String qualifiedReference, Optional<String> packageName) {
     final Matcher matcher = QUALIFIED_REFERENCE.matcher(qualifiedReference);
     Preconditions.checkArgument(
         matcher.find(),
@@ -143,7 +149,8 @@ public class FullyQualifiedName implements DataKey {
         qualifiedReference,
         QUALIFIED_REFERENCE.pattern());
     return of(
-        Optional.ofNullable(matcher.group("package")).orElse(DEFAULT_PACKAGE),
+        Optional.ofNullable(emptyToNull(matcher.group("package")))
+            .orElse(packageName.orElse(DEFAULT_PACKAGE)),
         ImmutableList.of(),
         ResourceType.getEnum(matcher.group("type")),
         matcher.group("name"));
@@ -178,7 +185,8 @@ public class FullyQualifiedName implements DataKey {
   public String toPrettyString() {
     // TODO(corysmith): Add package when we start tracking it.
     return String.format(
-        "%s/%s",
+        "%s%s/%s",
+        pkg != DEFAULT_PACKAGE ? pkg + ':' : "",
         DASH_JOINER.join(
             ImmutableList.<String>builder().add(type.getName()).addAll(qualifiers).build()),
         name);
@@ -201,10 +209,23 @@ public class FullyQualifiedName implements DataKey {
         .toString();
   }
 
+  @VisibleForTesting
+  public String asUnqualifedName() {
+    return String.format(
+        "%s/%s",
+        DASH_JOINER.join(
+            ImmutableList.<String>builder().add(type.getName()).addAll(qualifiers).build()),
+        name);
+  }
+
   public String name() {
     return name;
   }
 
+  public boolean isInPackage(String packageName) {
+    return pkg.equals(packageName);
+  }
+
   /** Provides the name qualified by the package it belongs to. */
   public String qualifiedName() {
     return (pkg.equals(DEFAULT_PACKAGE) ? "" : pkg + ":") + name;
@@ -563,7 +584,7 @@ public class FullyQualifiedName implements DataKey {
     }
 
     public static Factory from(List<String> qualifiers, String pkg) {
-      return new Factory(ImmutableList.copyOf(qualifiers), pkg);
+      return new Factory(ImmutableList.copyOf(qualifiers), pkg.isEmpty() ? DEFAULT_PACKAGE : pkg);
     }
 
     public static Factory from(List<String> qualifiers) {
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 7c4949f9ea..d7d1ad13d7 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
@@ -350,6 +350,8 @@ public class ResourceLinker {
               .forBuildToolsVersion(buildToolsVersion)
               .forVariantType(VariantType.DEFAULT)
               .add("optimize")
+              .when(Objects.equals(logger.getLevel(), Level.FINE))
+              .thenAdd("-v")
               .add("--target-densities", densities.stream().collect(Collectors.joining(",")))
               .add("-o", optimized)
               .add(outPath.toString())
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java b/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
index 32147cd49c..98aedd3145 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/SimpleXmlResourceValue.java
@@ -279,7 +279,7 @@ public class SimpleXmlResourceValue implements XmlResourceValue {
       stringValue = Integer.toString(item.getPrim().getData());
     } else {
       throw new IllegalArgumentException(
-          String.format("'%s' is not a valid resource type.", resourceType));
+          String.format("'%s' with value %s is not a simple resource type.", resourceType, proto));
     }
 
     return of(
diff --git a/src/tools/android/java/com/google/devtools/build/android/xml/StyleableXmlResourceValue.java b/src/tools/android/java/com/google/devtools/build/android/xml/StyleableXmlResourceValue.java
index 72707cf87d..3f56cf726e 100644
--- a/src/tools/android/java/com/google/devtools/build/android/xml/StyleableXmlResourceValue.java
+++ b/src/tools/android/java/com/google/devtools/build/android/xml/StyleableXmlResourceValue.java
@@ -18,10 +18,10 @@ import com.android.aapt.Resources.Value;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMap.Builder;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.android.AndroidCompiledDataDeserializer.ReferenceResolver;
 import com.google.devtools.build.android.AndroidDataWritingVisitor;
 import com.google.devtools.build.android.AndroidDataWritingVisitor.ValuesResourceDefinition;
 import com.google.devtools.build.android.AndroidResourceSymbolSink;
@@ -165,23 +165,17 @@ public class StyleableXmlResourceValue implements XmlResourceValue {
             Iterables.transform(proto.getReferencesList(), DATA_KEY_TO_FULLY_QUALIFIED_NAME)));
   }
 
-  public static XmlResourceValue from(
-      Value proto, Map<String, Boolean> qualifiedReferenceInlineStatus) {
+  public static XmlResourceValue from(Value proto, ReferenceResolver packageResolver) {
     Map<FullyQualifiedName, Boolean> attributes = new HashMap<>();
 
     Styleable styleable = proto.getCompoundValue().getStyleable();
     for (Styleable.Entry entry : styleable.getEntryList()) {
-      final FullyQualifiedName reference =
-          FullyQualifiedName.fromReference(entry.getAttr().getName());
-      final String qualifiedReference = reference.asQualifiedReference();
-      Preconditions.checkArgument(
-          qualifiedReferenceInlineStatus.containsKey(qualifiedReference),
-          "Styleable reference %s is not in %s",
-          qualifiedReference,
-          qualifiedReferenceInlineStatus.keySet());
-
-      attributes.put(reference, qualifiedReferenceInlineStatus.get(qualifiedReference));
-      qualifiedReferenceInlineStatus.put(qualifiedReference, false);
+      final FullyQualifiedName reference = packageResolver.parse(entry.getAttr().getName());
+      final boolean shouldInline = packageResolver.shouldInline(reference);
+      attributes.put(reference, shouldInline);
+      if (shouldInline) {
+        packageResolver.markInlined(reference);
+      }
     }
 
     return of(ImmutableMap.copyOf(attributes));