AAPT2: Merge Styleables instead of overriding them
Styleables merge in AAPT. Preserve this behavior.
Bug:30970091
Change-Id: Ie68ca675aeecd873c0648682182e2fc574e329a0
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index 09a04e0..2969b8c 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -82,6 +82,8 @@
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 @@
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();
}
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 460de0e..ae5d299 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -203,11 +203,38 @@
/**
* 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
diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp
index 4db40a6..feaca4e 100644
--- a/tools/aapt2/ResourceTable_test.cpp
+++ b/tools/aapt2/ResourceTable_test.cpp
@@ -118,6 +118,39 @@
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;
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 73682ab..321f776 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -24,6 +24,7 @@
#include <algorithm>
#include <androidfw/ResourceTypes.h>
#include <limits>
+#include <set>
namespace aapt {
@@ -732,4 +733,27 @@
<< "]";
}
+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
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index e6af716..eb7559a 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -179,6 +179,9 @@
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 @@
bool equals(const Value* value) const override;
Styleable* clone(StringPool* newPool) const override;
void print(std::ostream* out) const override;
+ void mergeWith(Styleable* styleable);
};
/**