Handle public tags in merger -> R.class writer

Developers do use public tags, and before this change
it would assert in the AndroidResourceClassWriter
as an unhandled ResourceType. We probably didn't want
to write out a R.public.field anyway.

Also, handle public tags with the same name,
but different type. They get mapped to the same FQN,
so use the combining mechanism to keep track of
the different types and ids.

--
MOS_MIGRATED_REVID=130395089

1 files changed, 183 insertions, 23 deletions

diff --git a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceClassWriter.java b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceClassWriter.java
index 0d51f50d2e..4c3118e61b 100644
--- a/src/tools/android/java/com/google/devtools/build/android/AndroidResourceClassWriter.java
+++ b/src/tools/android/java/com/google/devtools/build/android/AndroidResourceClassWriter.java
@@ -17,9 +17,11 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.android.SdkConstants;
 import com.android.resources.ResourceType;
+import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
 import com.google.devtools.build.android.AndroidFrameworkAttrIdProvider.AttrLookupException;
 import com.google.devtools.build.android.resources.FieldInitializer;
@@ -39,7 +41,10 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.TreeSet;
+import java.util.logging.Logger;
 
 /**
  * Generates the R class for an android_library with made up field initializers for the ids. The
@@ -50,12 +55,18 @@ import java.util.TreeSet;
  */
 public class AndroidResourceClassWriter implements Flushable {
 
+  private static final Logger logger =
+      Logger.getLogger(AndroidResourceClassWriter.class.getName());
+  private static final int APP_PACKAGE_MASK = 0x7f000000;
+  private static final int ATTR_TYPE_ID = 1;
   private final AndroidFrameworkAttrIdProvider androidIdProvider;
   private final Path outputBasePath;
   private final String packageName;
 
   private final Map<ResourceType, Set<String>> innerClasses = new EnumMap<>(ResourceType.class);
   private final Map<String, Map<String, Boolean>> styleableAttrs = new HashMap<>();
+  private final Map<ResourceType, SortedMap<String, Optional<Integer>>> publicIds =
+      new EnumMap<>(ResourceType.class);
 
   private static final String NORMALIZED_ANDROID_PREFIX = "android_";
 
@@ -77,6 +88,29 @@ public class AndroidResourceClassWriter implements Flushable {
     fields.add(normalizeName(name));
   }
 
+  public void writePublicValue(ResourceType type, String name, Optional<Integer> value) {
+    SortedMap<String, Optional<Integer>> publicMappings = publicIds.get(type);
+    if (publicMappings == null) {
+      publicMappings = new TreeMap<>();
+      publicIds.put(type, publicMappings);
+    }
+    Optional<Integer> oldValue = publicMappings.put(name, value);
+    // AAPT should issue an error, but do a bit of sanity checking here just in case.
+    if (oldValue != null && !oldValue.equals(value)) {
+      // Enforce a consistent ordering on the warning message.
+      Integer lower = oldValue.orNull();
+      Integer higher = value.orNull();
+      if (Ordering.natural().compare(oldValue.orNull(), value.orNull()) > 0) {
+        lower = higher;
+        higher = oldValue.orNull();
+      }
+      logger.warning(
+          String.format(
+              "resource %s/%s has conflicting public identifiers (0x%x vs 0x%x)",
+              type, name, lower, higher));
+    }
+  }
+
   public void writeStyleableResource(FullyQualifiedName key,
       Map<FullyQualifiedName, Boolean> attrs) {
     ResourceType type = ResourceType.STYLEABLE;
@@ -161,21 +195,83 @@ public class AndroidResourceClassWriter implements Flushable {
   );
 
   private Map<ResourceType, Integer> chooseTypeIds() {
-    Map<ResourceType, Integer> allocatedTypeIds = new EnumMap<>(ResourceType.class);
+    // Go through public entries. Those may have forced certain type assignments, so take those
+    // into account first.
+    Map<ResourceType, Integer> allocatedTypeIds = assignTypeIdsForPublic();
+    Set<Integer> reservedTypeSlots = ImmutableSet.copyOf(allocatedTypeIds.values());
     // ATTR always takes up slot #1, even if it isn't present.
-    allocatedTypeIds.put(ResourceType.ATTR, 1);
-    // The rest are packed starting at #2.
-    int nextTypeId = 2;
+    allocatedTypeIds.put(ResourceType.ATTR, ATTR_TYPE_ID);
+    // The rest are packed after that.
+    int nextTypeId = nextFreeId(ATTR_TYPE_ID + 1, reservedTypeSlots);
     for (ResourceType t : AAPT_TYPE_ORDERING) {
-      if (innerClasses.containsKey(t)) {
+      if (innerClasses.containsKey(t) && !allocatedTypeIds.containsKey(t)) {
         allocatedTypeIds.put(t, nextTypeId);
-        ++nextTypeId;
+        nextTypeId = nextFreeId(nextTypeId + 1, reservedTypeSlots);
       }
     }
-    // Sanity check that everything has been assigned, except STYLEABLE.
+    // Sanity check that everything has been assigned, except STYLEABLE. There shouldn't be
+    // anything of type PUBLIC either (since that isn't a real resource).
     // We will need to update the list if there is a new resource type.
     for (ResourceType t : innerClasses.keySet()) {
-      Preconditions.checkArgument(t == ResourceType.STYLEABLE || allocatedTypeIds.containsKey(t));
+      Preconditions.checkState(
+          t == ResourceType.STYLEABLE || allocatedTypeIds.containsKey(t),
+          "Resource type %s is not allocated a type ID",
+          t);
+    }
+    return allocatedTypeIds;
+  }
+
+  private Map<ResourceType, Integer> assignTypeIdsForPublic() {
+    Map<ResourceType, Integer> allocatedTypeIds = new EnumMap<>(ResourceType.class);
+    if (publicIds.isEmpty()) {
+      return allocatedTypeIds;
+    }
+    // Keep track of the reverse mapping from Int -> Type for validation.
+    Map<Integer, ResourceType> assignedIds = new HashMap<>();
+    for (Map.Entry<ResourceType, SortedMap<String, Optional<Integer>>> publicTypeEntry :
+        publicIds.entrySet()) {
+      ResourceType currentType = publicTypeEntry.getKey();
+      Integer reservedTypeSlot = null;
+      String previousResource = null;
+      for (Map.Entry<String, Optional<Integer>> publicEntry :
+          publicTypeEntry.getValue().entrySet()) {
+        Optional<Integer> reservedId = publicEntry.getValue();
+        if (!reservedId.isPresent()) {
+          continue;
+        }
+        Integer typePortion = extractTypeId(reservedId.get());
+        if (reservedTypeSlot == null) {
+          reservedTypeSlot = typePortion;
+          previousResource = publicEntry.getKey();
+        } else {
+          if (!reservedTypeSlot.equals(typePortion)) {
+            logger.warning(
+                String.format(
+                    "%s has conflicting type codes for its public identifiers (%s=%s vs %s=%s)",
+                    currentType.getName(),
+                    previousResource,
+                    reservedTypeSlot,
+                    publicEntry.getKey(),
+                    typePortion));
+          }
+        }
+      }
+      if (currentType == ResourceType.ATTR
+          && reservedTypeSlot != null
+          && !reservedTypeSlot.equals(ATTR_TYPE_ID)) {
+        logger.warning(
+            String.format(
+                "Cannot force ATTR to have type code other than 0x%02x (got 0x%02x from %s)",
+                ATTR_TYPE_ID, reservedTypeSlot, previousResource));
+      }
+      allocatedTypeIds.put(currentType, reservedTypeSlot);
+      ResourceType alreadyAssigned = assignedIds.put(reservedTypeSlot, currentType);
+      if (alreadyAssigned != null) {
+        logger.warning(
+            String.format(
+                "Multiple type names declared for public type identifier 0x%x (%s vs %s)",
+                reservedTypeSlot, alreadyAssigned, currentType));
+      }
     }
     return allocatedTypeIds;
   }
@@ -187,6 +283,13 @@ public class AndroidResourceClassWriter implements Flushable {
     if (!innerClasses.containsKey(ResourceType.ATTR)) {
       return attrToIdBuilder.build();
     }
+    // After assigning public IDs, we count up monotonically, so we don't need to track additional
+    // assignedIds to avoid collisions (use an ImmutableSet to ensure we don't add more).
+    Set<Integer> assignedIds = ImmutableSet.of();
+    if (publicIds.containsKey(ResourceType.ATTR)) {
+      assignedIds = assignPublicIds(attrToIdBuilder, publicIds.get(ResourceType.ATTR), attrTypeId);
+    }
+    ImmutableMap<String, Integer> publicAttrs = attrToIdBuilder.build();
     Set<String> inlineAttrs = new HashSet<>();
     Set<String> styleablesWithInlineAttrs = new TreeSet<>();
     for (Map.Entry<String, Map<String, Boolean>> styleableAttrEntry
@@ -199,23 +302,23 @@ public class AndroidResourceClassWriter implements Flushable {
         }
       }
     }
-    int nextId = 0x7f000000 | (attrTypeId << 16);
+    int nextId = nextFreeId(getInitialIdForTypeId(attrTypeId), assignedIds);
     // Technically, aapt assigns based on declaration order, but the merge should have sorted
     // the non-inline attributes, so assigning by sorted order is the same.
     ImmutableList<String> sortedAttrs = Ordering.natural()
         .immutableSortedCopy(innerClasses.get(ResourceType.ATTR));
     for (String attr : sortedAttrs) {
-      if (!inlineAttrs.contains(attr)) {
+      if (!inlineAttrs.contains(attr) && !publicAttrs.containsKey(attr)) {
         attrToIdBuilder.put(attr, nextId);
-        ++nextId;
+        nextId = nextFreeId(nextId + 1, assignedIds);
       }
     }
     for (String styleable : styleablesWithInlineAttrs) {
       Map<String, Boolean> attrs = styleableAttrs.get(styleable);
       for (Map.Entry<String, Boolean> attrEntry : attrs.entrySet()) {
-        if (attrEntry.getValue()) {
+        if (attrEntry.getValue() && !publicAttrs.containsKey(attrEntry.getKey())) {
           attrToIdBuilder.put(attrEntry.getKey(), nextId);
-          ++nextId;
+          nextId = nextFreeId(nextId + 1, assignedIds);
         }
       }
     }
@@ -238,7 +341,7 @@ public class AndroidResourceClassWriter implements Flushable {
         fields = getAttrInitializers(attrAssignments, sortedFields);
       } else {
         int typeId = typeIdMap.get(type);
-        fields = getResourceInitializers(typeId, sortedFields);
+        fields = getResourceInitializers(type, typeId, sortedFields);
       }
       // The maximum number of Java fields is 2^16.
       // See the JVM reference "4.11. Limitations of the Java Virtual Machine."
@@ -283,10 +386,9 @@ public class AndroidResourceClassWriter implements Flushable {
   }
 
   private List<FieldInitializer> getAttrInitializers(
-      Map<String, Integer> attrAssignments,
-      Collection<String> fields) {
+      Map<String, Integer> attrAssignments, Collection<String> sortedFields) {
     ImmutableList.Builder<FieldInitializer> initList = ImmutableList.builder();
-    for (String field : fields) {
+    for (String field : sortedFields) {
       int attrId = attrAssignments.get(field);
       initList.add(new IntFieldInitializer(field, attrId));
     }
@@ -294,13 +396,22 @@ public class AndroidResourceClassWriter implements Flushable {
   }
 
   private List<FieldInitializer> getResourceInitializers(
-      int typeId,
-      Collection<String> fields) {
+      ResourceType type, int typeId, Collection<String> sortedFields) {
     ImmutableList.Builder<FieldInitializer> initList = ImmutableList.builder();
-    int resourceIds = 0x7f000000 | typeId << 16;
-    for (String field : fields) {
-      initList.add(new IntFieldInitializer(field, resourceIds));
-      ++resourceIds;
+    ImmutableMap.Builder<String, Integer> publicBuilder = ImmutableMap.builder();
+    Set<Integer> assignedIds = ImmutableSet.of();
+    if (publicIds.containsKey(type)) {
+      assignedIds = assignPublicIds(publicBuilder, publicIds.get(type), typeId);
+    }
+    Map<String, Integer> publicAssignments = publicBuilder.build();
+    int resourceIds = nextFreeId(getInitialIdForTypeId(typeId), assignedIds);
+    for (String field : sortedFields) {
+      Integer fieldValue = publicAssignments.get(field);
+      if (fieldValue == null) {
+        fieldValue = resourceIds;
+        resourceIds = nextFreeId(resourceIds + 1, assignedIds);
+      }
+      initList.add(new IntFieldInitializer(field, fieldValue));
     }
     return initList.build();
   }
@@ -348,4 +459,53 @@ public class AndroidResourceClassWriter implements Flushable {
     return normalizeName(attrName).replace(':', '_');
   }
 
+  /**
+   * Assign any public ids to the given idBuilder.
+   *
+   * @param idBuilder where to store the final name -> id mappings
+   * @param publicIds known public resources (can contain null values, if ID isn't reserved)
+   * @param typeId the type slot for the current resource type.
+   * @return the final set of assigned resource ids (includes those without apriori assignments).
+   */
+  private static Set<Integer> assignPublicIds(
+      ImmutableMap.Builder<String, Integer> idBuilder,
+      SortedMap<String, Optional<Integer>> publicIds,
+      int typeId) {
+    HashMap<Integer, String> assignedIds = new HashMap<>();
+    int prevId = getInitialIdForTypeId(typeId);
+    for (Map.Entry<String, Optional<Integer>> entry : publicIds.entrySet()) {
+      Optional<Integer> id = entry.getValue();
+      if (id.isPresent()) {
+        prevId = id.get();
+      } else {
+        prevId = nextFreeId(prevId + 1, assignedIds.keySet());
+      }
+      String previousMapping = assignedIds.put(prevId, entry.getKey());
+      if (previousMapping != null) {
+        logger.warning(
+            String.format(
+                "Multiple entry names declared for public entry identifier 0x%x (%s and %s)",
+                prevId, previousMapping, entry.getKey()));
+      }
+      idBuilder.put(entry.getKey(), prevId);
+    }
+    return assignedIds.keySet();
+  }
+
+  private static int extractTypeId(int fullID) {
+    return (fullID & 0x00FF0000) >> 16;
+  }
+
+  private static int getInitialIdForTypeId(int typeId) {
+    return APP_PACKAGE_MASK | (typeId << 16);
+  }
+
+  private static int nextFreeId(int nextSlot, Set<Integer> reservedSlots) {
+    // Linear search for the next free slot. This assumes that reserved <public> ids are rare.
+    // Otherwise we should use a NavigableSet or some other smarter data-structure.
+    while (reservedSlots.contains(nextSlot)) {
+      ++nextSlot;
+    }
+    return nextSlot;
+  }
 }