AAPT2: Merge Styleables instead of overriding them

Styleables merge in AAPT. Preserve this behavior.

Bug:30970091
Change-Id: Ie68ca675aeecd873c0648682182e2fc574e329a0

tools/aapt2/Resource.h

@@ -82,6 +82,8 @@ struct ResourceName {
     ResourceName() : type(ResourceType::kRaw) {}
     ResourceName(const StringPiece& p, ResourceType t, const StringPiece& e);
 
+    int compare(const ResourceName& other) const;
+
     bool isValid() const;
     std::string toString() const;
 };
@@ -258,6 +260,15 @@ inline ResourceName::ResourceName(const StringPiece& p, ResourceType t, const St
         package(p.toString()), type(t), entry(e.toString()) {
 }
 
+inline int ResourceName::compare(const ResourceName& other) const {
+    int cmp = package.compare(other.package);
+    if (cmp != 0) return cmp;
+    cmp = static_cast<int>(type) - static_cast<int>(other.type);
+    if (cmp != 0) return cmp;
+    cmp = entry.compare(other.entry);
+    return cmp;
+}
+
 inline bool ResourceName::isValid() const {
     return !package.empty() && !entry.empty();
 }

tools/aapt2/ResourceTable.cpp

@@ -203,11 +203,38 @@ std::vector<ResourceConfigValue*> ResourceEntry::findValuesIf(
 /**
  * The default handler for collisions. A return value of -1 means keep the
  * existing value, 0 means fail, and +1 means take the incoming value.
+ *
+ * Typically, a weak value will be overridden by a strong value. An existing weak
+ * value will not be overridden by an incoming weak value.
+ *
+ * There are some exceptions:
+ *
+ * Attributes: There are two types of Attribute values: USE and DECL.
+ *
+ * USE is anywhere an Attribute is declared without a format, and in a place that would
+ * be legal to declare if the Attribute already existed. This is typically in a
+ * <declare-styleable> tag. Attributes defined in a <declare-styleable> are also weak.
+ *
+ * DECL is an absolute declaration of an Attribute and specifies an explicit format.
+ *
+ * A DECL will override a USE without error. Two DECLs must match in their format for there to be
+ * no error.
+ *
+ * Styleables: Styleables are not actual resources, but they are treated as such during the
+ * compilation phase. Styleables are allowed to override each other, and their definitions merge
+ * and accumulate. If both values are Styleables, we just merge them into the existing value.
  */
 int ResourceTable::resolveValueCollision(Value* existing, Value* incoming) {
+    if (Styleable* existingStyleable = valueCast<Styleable>(existing)) {
+        if (Styleable* incomingStyleable = valueCast<Styleable>(incoming)) {
+            // Styleables get merged.
+            existingStyleable->mergeWith(incomingStyleable);
+            return -1;
+        }
+    }
+
     Attribute* existingAttr = valueCast<Attribute>(existing);
     Attribute* incomingAttr = valueCast<Attribute>(incoming);
-
     if (!incomingAttr) {
         if (incoming->isWeak()) {
             // We're trying to add a weak resource but a resource

tools/aapt2/ResourceTable_test.cpp

@@ -118,6 +118,39 @@ TEST(ResourceTableTest, OverrideWeakResourceValue) {
     EXPECT_FALSE(attr->isWeak());
 }
 
+TEST(ResourceTable, MergeStyleables) {
+    ResourceTable table;
+
+    ASSERT_TRUE(table.addResource(
+            test::parseNameOrDie("android:styleable/Foo"),
+            ConfigDescription{}, "",
+            test::StyleableBuilder()
+                    .addItem("android:attr/bar")
+                    .addItem("android:attr/foo")
+                    .build(),
+            test::getDiagnostics()));
+
+    ASSERT_TRUE(table.addResource(
+            test::parseNameOrDie("android:styleable/Foo"),
+            ConfigDescription{}, "",
+            test::StyleableBuilder()
+                    .addItem("android:attr/bat")
+                    .addItem("android:attr/foo")
+                    .build(),
+            test::getDiagnostics()));
+
+    Styleable* styleable = test::getValue<Styleable>(&table, "android:styleable/Foo");
+    ASSERT_NE(nullptr, styleable);
+
+    std::vector<Reference> expectedRefs = {
+            Reference(test::parseNameOrDie("android:attr/bar")),
+            Reference(test::parseNameOrDie("android:attr/bat")),
+            Reference(test::parseNameOrDie("android:attr/foo")),
+    };
+
+    EXPECT_EQ(expectedRefs, styleable->entries);
+}
+
 TEST(ResourceTableTest, ProductVaryingValues) {
     ResourceTable table;
 

tools/aapt2/ResourceValues.cpp

@@ -24,6 +24,7 @@
 #include <algorithm>
 #include <androidfw/ResourceTypes.h>
 #include <limits>
+#include <set>
 
 namespace aapt {
 
@@ -732,4 +733,27 @@ void Styleable::print(std::ostream* out) const {
         << "]";
 }
 
+bool operator<(const Reference& a, const Reference& b) {
+    int cmp = a.name.valueOrDefault({}).compare(b.name.valueOrDefault({}));
+    if (cmp != 0) return cmp < 0;
+    return a.id < b.id;
+}
+
+bool operator==(const Reference& a, const Reference& b) {
+    return a.name == b.name && a.id == b.id;
+}
+
+bool operator!=(const Reference& a, const Reference& b) {
+    return a.name != b.name || a.id != b.id;
+}
+
+void Styleable::mergeWith(Styleable* other) {
+    std::set<Reference> references;
+    references.insert(entries.begin(), entries.end());
+    references.insert(other->entries.begin(), other->entries.end());
+    entries.clear();
+    entries.reserve(references.size());
+    entries.insert(entries.end(), references.begin(), references.end());
+}
+
 } // namespace aapt

tools/aapt2/ResourceValues.h

@@ -179,6 +179,9 @@ struct Reference : public BaseItem<Reference> {
     void print(std::ostream* out) const override;
 };
 
+bool operator<(const Reference&, const Reference&);
+bool operator==(const Reference&, const Reference&);
+
 /**
  * An ID resource. Has no real value, just a place holder.
  */
@@ -334,6 +337,7 @@ struct Styleable : public BaseValue<Styleable> {
     bool equals(const Value* value) const override;
     Styleable* clone(StringPool* newPool) const override;
     void print(std::ostream* out) const override;
+    void mergeWith(Styleable* styleable);
 };
 
 /**