AAPT2: Allow merging of Style attributes from overlays
Previously style overlays would completely override an existing style.
To be compatible with AAPT, styles now merge with the overlay, allowing
the overlay's attributes and parent to take precedence.
Bug: 38355988
Test: make aapt2_tests
Change-Id: Id25c7240050a43e6a4a177c6e3d51e048d0cceb5
diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index bf18949..b2f1f62 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -160,7 +160,7 @@
cc_test_host {
name: "aapt2_tests",
srcs: ["**/*_test.cpp"],
- static_libs: ["libaapt2"],
+ static_libs: ["libaapt2", "libgmock"],
defaults: ["aapt_defaults"],
}
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 1bb7d9b..818c8ce 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -496,19 +496,17 @@
std::unique_ptr<BinaryPrimitive> TryParseBool(const StringPiece& str) {
if (Maybe<bool> maybe_result = ParseBool(str)) {
- android::Res_value value = {};
- value.dataType = android::Res_value::TYPE_INT_BOOLEAN;
-
- if (maybe_result.value()) {
- value.data = 0xffffffffu;
- } else {
- value.data = 0;
- }
- return util::make_unique<BinaryPrimitive>(value);
+ const uint32_t data = maybe_result.value() ? 0xffffffffu : 0u;
+ return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN, data);
}
return {};
}
+std::unique_ptr<BinaryPrimitive> MakeBool(bool val) {
+ return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN,
+ val ? 0xffffffffu : 0u);
+}
+
std::unique_ptr<BinaryPrimitive> TryParseInt(const StringPiece& str) {
std::u16string str16 = util::Utf8ToUtf16(str);
android::Res_value value;
diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h
index 48922b7..da0fc8e 100644
--- a/tools/aapt2/ResourceUtils.h
+++ b/tools/aapt2/ResourceUtils.h
@@ -147,6 +147,9 @@
*/
std::unique_ptr<BinaryPrimitive> TryParseBool(const android::StringPiece& str);
+// Returns a boolean BinaryPrimitive.
+std::unique_ptr<BinaryPrimitive> MakeBool(bool val);
+
/*
* Returns a BinaryPrimitve object representing an integer if the string was
* parsed as one.
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 34bd2b4..abfdec4 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -29,6 +29,11 @@
namespace aapt {
+std::ostream& operator<<(std::ostream& out, const Value& value) {
+ value.Print(&out);
+ return out;
+}
+
template <typename Derived>
void BaseValue<Derived>::Accept(RawValueVisitor* visitor) {
visitor->Visit(static_cast<Derived*>(this));
@@ -346,6 +351,15 @@
weak_ = w;
}
+std::ostream& operator<<(std::ostream& out, const Attribute::Symbol& s) {
+ if (s.symbol.name) {
+ out << s.symbol.name.value().entry;
+ } else {
+ out << "???";
+ }
+ return out << "=" << s.value;
+}
+
template <typename T>
constexpr T* add_pointer(T& val) {
return &val;
@@ -361,31 +375,27 @@
return false;
}
- if (type_mask != other->type_mask || min_int != other->min_int ||
- max_int != other->max_int) {
+ if (type_mask != other->type_mask || min_int != other->min_int || max_int != other->max_int) {
return false;
}
std::vector<const Symbol*> sorted_a;
std::transform(symbols.begin(), symbols.end(), std::back_inserter(sorted_a),
add_pointer<const Symbol>);
- std::sort(sorted_a.begin(), sorted_a.end(),
- [](const Symbol* a, const Symbol* b) -> bool {
- return a->symbol.name < b->symbol.name;
- });
+ std::sort(sorted_a.begin(), sorted_a.end(), [](const Symbol* a, const Symbol* b) -> bool {
+ return a->symbol.name < b->symbol.name;
+ });
std::vector<const Symbol*> sorted_b;
- std::transform(other->symbols.begin(), other->symbols.end(),
- std::back_inserter(sorted_b), add_pointer<const Symbol>);
- std::sort(sorted_b.begin(), sorted_b.end(),
- [](const Symbol* a, const Symbol* b) -> bool {
- return a->symbol.name < b->symbol.name;
- });
+ std::transform(other->symbols.begin(), other->symbols.end(), std::back_inserter(sorted_b),
+ add_pointer<const Symbol>);
+ std::sort(sorted_b.begin(), sorted_b.end(), [](const Symbol* a, const Symbol* b) -> bool {
+ return a->symbol.name < b->symbol.name;
+ });
return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(),
[](const Symbol* a, const Symbol* b) -> bool {
- return a->symbol.Equals(&b->symbol) &&
- a->value == b->value;
+ return a->symbol.Equals(&b->symbol) && a->value == b->value;
});
}
@@ -588,14 +598,50 @@
return true;
}
+std::ostream& operator<<(std::ostream& out, const Style::Entry& entry) {
+ if (entry.key.name) {
+ out << entry.key.name.value();
+ } else if (entry.key.id) {
+ out << entry.key.id.value();
+ } else {
+ out << "???";
+ }
+ out << " = " << entry.value;
+ return out;
+}
+
+template <typename T>
+std::vector<T*> ToPointerVec(std::vector<T>& src) {
+ std::vector<T*> dst;
+ dst.reserve(src.size());
+ for (T& in : src) {
+ dst.push_back(&in);
+ }
+ return dst;
+}
+
+template <typename T>
+std::vector<const T*> ToPointerVec(const std::vector<T>& src) {
+ std::vector<const T*> dst;
+ dst.reserve(src.size());
+ for (const T& in : src) {
+ dst.push_back(&in);
+ }
+ return dst;
+}
+
+static bool KeyNameComparator(const Style::Entry* a, const Style::Entry* b) {
+ return a->key.name < b->key.name;
+}
+
bool Style::Equals(const Value* value) const {
const Style* other = ValueCast<Style>(value);
if (!other) {
return false;
}
+
if (bool(parent) != bool(other->parent) ||
- (parent && other->parent &&
- !parent.value().Equals(&other->parent.value()))) {
+ (parent && other->parent && !parent.value().Equals(&other->parent.value()))) {
return false;
}
@@ -603,26 +649,15 @@
return false;
}
- std::vector<const Entry*> sorted_a;
- std::transform(entries.begin(), entries.end(), std::back_inserter(sorted_a),
- add_pointer<const Entry>);
- std::sort(sorted_a.begin(), sorted_a.end(),
- [](const Entry* a, const Entry* b) -> bool {
- return a->key.name < b->key.name;
- });
+ std::vector<const Entry*> sorted_a = ToPointerVec(entries);
+ std::sort(sorted_a.begin(), sorted_a.end(), KeyNameComparator);
- std::vector<const Entry*> sorted_b;
- std::transform(other->entries.begin(), other->entries.end(),
- std::back_inserter(sorted_b), add_pointer<const Entry>);
- std::sort(sorted_b.begin(), sorted_b.end(),
- [](const Entry* a, const Entry* b) -> bool {
- return a->key.name < b->key.name;
- });
+ std::vector<const Entry*> sorted_b = ToPointerVec(other->entries);
+ std::sort(sorted_b.begin(), sorted_b.end(), KeyNameComparator);
return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(),
[](const Entry* a, const Entry* b) -> bool {
- return a->key.Equals(&b->key) &&
- a->value->Equals(b->value.get());
+ return a->key.Equals(&b->key) && a->value->Equals(b->value.get());
});
}
@@ -633,8 +668,7 @@
style->comment_ = comment_;
style->source_ = source_;
for (auto& entry : entries) {
- style->entries.push_back(
- Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))});
+ style->entries.push_back(Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))});
}
return style;
}
@@ -642,26 +676,73 @@
void Style::Print(std::ostream* out) const {
*out << "(style) ";
if (parent && parent.value().name) {
- if (parent.value().private_reference) {
+ const Reference& parent_ref = parent.value();
+ if (parent_ref.private_reference) {
*out << "*";
}
- *out << parent.value().name.value();
+ *out << parent_ref.name.value();
}
*out << " [" << util::Joiner(entries, ", ") << "]";
}
-static ::std::ostream& operator<<(::std::ostream& out,
- const Style::Entry& value) {
- if (value.key.name) {
- out << value.key.name.value();
- } else if (value.key.id) {
- out << value.key.id.value();
- } else {
- out << "???";
+Style::Entry CloneEntry(const Style::Entry& entry, StringPool* pool) {
+ Style::Entry cloned_entry{entry.key};
+ if (entry.value != nullptr) {
+ cloned_entry.value.reset(entry.value->Clone(pool));
}
- out << " = ";
- value.value->Print(&out);
- return out;
+ return cloned_entry;
+}
+
+void Style::MergeWith(Style* other, StringPool* pool) {
+ if (other->parent) {
+ parent = other->parent;
+ }
+
+ // We can't assume that the entries are sorted alphabetically since they're supposed to be
+ // sorted by Resource Id. Not all Resource Ids may be set though, so we can't sort and merge
+ // them keying off that.
+ //
+ // Instead, sort the entries of each Style by their name in a separate structure. Then merge
+ // those.
+
+ std::vector<Entry*> this_sorted = ToPointerVec(entries);
+ std::sort(this_sorted.begin(), this_sorted.end(), KeyNameComparator);
+
+ std::vector<Entry*> other_sorted = ToPointerVec(other->entries);
+ std::sort(other_sorted.begin(), other_sorted.end(), KeyNameComparator);
+
+ auto this_iter = this_sorted.begin();
+ const auto this_end = this_sorted.end();
+
+ auto other_iter = other_sorted.begin();
+ const auto other_end = other_sorted.end();
+
+ std::vector<Entry> merged_entries;
+ while (this_iter != this_end) {
+ if (other_iter != other_end) {
+ if ((*this_iter)->key.name < (*other_iter)->key.name) {
+ merged_entries.push_back(std::move(**this_iter));
+ ++this_iter;
+ } else {
+ // The other overrides.
+ merged_entries.push_back(CloneEntry(**other_iter, pool));
+ if ((*this_iter)->key.name == (*other_iter)->key.name) {
+ ++this_iter;
+ }
+ ++other_iter;
+ }
+ } else {
+ merged_entries.push_back(std::move(**this_iter));
+ ++this_iter;
+ }
+ }
+
+ while (other_iter != other_end) {
+ merged_entries.push_back(CloneEntry(**other_iter, pool));
+ ++other_iter;
+ }
+
+ entries = std::move(merged_entries);
}
bool Array::Equals(const Value* value) const {
@@ -758,11 +839,6 @@
}
}
-static ::std::ostream& operator<<(::std::ostream& out,
- const std::unique_ptr<Item>& item) {
- return out << *item;
-}
-
bool Styleable::Equals(const Value* value) const {
const Styleable* other = ValueCast<Styleable>(value);
if (!other) {
@@ -810,8 +886,7 @@
void Styleable::MergeWith(Styleable* other) {
// Compare only names, because some References may already have their IDs
- // assigned
- // (framework IDs that don't change).
+ // assigned (framework IDs that don't change).
std::set<Reference, NameOnlyComparator> references;
references.insert(entries.begin(), entries.end());
references.insert(other->entries.begin(), other->entries.end());
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 06f949f..ac5795f 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -40,7 +40,8 @@
// type specific operations is to check the Value's type() and
// cast it to the appropriate subclass. This isn't super clean,
// but it is the simplest strategy.
-struct Value {
+class Value {
+ public:
virtual ~Value() = default;
// Whether this value is weak and can be overridden without warning or error. Default is false.
@@ -82,6 +83,8 @@
// Human readable printout of this value.
virtual void Print(std::ostream* out) const = 0;
+ friend std::ostream& operator<<(std::ostream& out, const Value& value);
+
protected:
Source source_;
std::string comment_;
@@ -245,6 +248,8 @@
struct Symbol {
Reference symbol;
uint32_t value;
+
+ friend std::ostream& operator<<(std::ostream& out, const Symbol& symbol);
};
uint32_t type_mask;
@@ -266,6 +271,8 @@
struct Entry {
Reference key;
std::unique_ptr<Item> value;
+
+ friend std::ostream& operator<<(std::ostream& out, const Entry& entry);
};
Maybe<Reference> parent;
@@ -278,6 +285,10 @@
bool Equals(const Value* value) const override;
Style* Clone(StringPool* new_pool) const override;
void Print(std::ostream* out) const override;
+
+ // Merges `style` into this Style. All identical attributes of `style` take precedence, including
+ // the parent, if there is one.
+ void MergeWith(Style* style, StringPool* pool);
};
struct Array : public BaseValue<Array> {
@@ -307,20 +318,15 @@
void MergeWith(Styleable* styleable);
};
-// Stream operator for printing Value objects.
-inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) {
- value.Print(&out);
- return out;
-}
-
-inline ::std::ostream& operator<<(::std::ostream& out,
- const Attribute::Symbol& s) {
- if (s.symbol.name) {
- out << s.symbol.name.value().entry;
+template <typename T>
+typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<(
+ std::ostream& out, const std::unique_ptr<T>& value) {
+ if (value == nullptr) {
+ out << "NULL";
} else {
- out << "???";
+ value->Print(&out);
}
- return out << "=" << s.value;
+ return out;
}
} // namespace aapt
diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp
index 6922580..e154d93 100644
--- a/tools/aapt2/ResourceValues_test.cpp
+++ b/tools/aapt2/ResourceValues_test.cpp
@@ -148,4 +148,36 @@
EXPECT_TRUE(a->Equals(b.get()));
}
+TEST(ResourceValuesTest, StyleMerges) {
+ StringPool pool_a;
+ StringPool pool_b;
+
+ std::unique_ptr<Style> a =
+ test::StyleBuilder()
+ .SetParent("android:style/Parent")
+ .AddItem("android:attr/a", util::make_unique<String>(pool_a.MakeRef("FooA")))
+ .AddItem("android:attr/b", util::make_unique<String>(pool_a.MakeRef("FooB")))
+ .Build();
+
+ std::unique_ptr<Style> b =
+ test::StyleBuilder()
+ .SetParent("android:style/OverlayParent")
+ .AddItem("android:attr/c", util::make_unique<String>(pool_b.MakeRef("OverlayFooC")))
+ .AddItem("android:attr/a", util::make_unique<String>(pool_b.MakeRef("OverlayFooA")))
+ .Build();
+
+ a->MergeWith(b.get(), &pool_a);
+
+ StringPool pool;
+ std::unique_ptr<Style> expected =
+ test::StyleBuilder()
+ .SetParent("android:style/OverlayParent")
+ .AddItem("android:attr/a", util::make_unique<String>(pool.MakeRef("OverlayFooA")))
+ .AddItem("android:attr/b", util::make_unique<String>(pool.MakeRef("FooB")))
+ .AddItem("android:attr/c", util::make_unique<String>(pool.MakeRef("OverlayFooC")))
+ .Build();
+
+ EXPECT_TRUE(a->Equals(expected.get()));
+}
+
} // namespace aapt
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 9311091..14c23ef 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -179,32 +179,39 @@
return true;
}
-/**
- * Modified CollisionResolver which will merge Styleables. Used with overlays.
- *
- * Styleables are not actual resources, but they are treated as such during the
- * compilation phase. Styleables don't simply overlay each other, their
- * definitions merge
- * and accumulate. If both values are Styleables, we just merge them into the
- * existing value.
- */
-static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing,
- Value* incoming) {
+// Modified CollisionResolver which will merge Styleables and Styles. Used with overlays.
+//
+// Styleables are not actual resources, but they are treated as such during the
+// compilation phase.
+//
+// Styleables and Styles don't simply overlay each other, their definitions merge
+// and accumulate. If both values are Styleables/Styles, we just merge them into the
+// existing value.
+static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming,
+ StringPool* pool) {
if (Styleable* existing_styleable = ValueCast<Styleable>(existing)) {
if (Styleable* incoming_styleable = ValueCast<Styleable>(incoming)) {
// Styleables get merged.
existing_styleable->MergeWith(incoming_styleable);
return ResourceTable::CollisionResult::kKeepOriginal;
}
+ } else if (Style* existing_style = ValueCast<Style>(existing)) {
+ if (Style* incoming_style = ValueCast<Style>(incoming)) {
+ // Styles get merged.
+ existing_style->MergeWith(incoming_style, pool);
+ return ResourceTable::CollisionResult::kKeepOriginal;
+ }
}
// Delegate to the default handler.
return ResourceTable::ResolveValueCollision(existing, incoming);
}
-static ResourceTable::CollisionResult MergeConfigValue(
- IAaptContext* context, const ResourceNameRef& res_name, const bool overlay,
- ResourceConfigValue* dst_config_value,
- ResourceConfigValue* src_config_value) {
+static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context,
+ const ResourceNameRef& res_name,
+ const bool overlay,
+ ResourceConfigValue* dst_config_value,
+ ResourceConfigValue* src_config_value,
+ StringPool* pool) {
using CollisionResult = ResourceTable::CollisionResult;
Value* dst_value = dst_config_value->value.get();
@@ -212,10 +219,9 @@
CollisionResult collision_result;
if (overlay) {
- collision_result = ResolveMergeCollision(dst_value, src_value);
+ collision_result = ResolveMergeCollision(dst_value, src_value, pool);
} else {
- collision_result =
- ResourceTable::ResolveValueCollision(dst_value, src_value);
+ collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
}
if (collision_result == CollisionResult::kConflict) {
@@ -224,10 +230,9 @@
}
// Error!
- context->GetDiagnostics()->Error(
- DiagMessage(src_value->GetSource())
- << "resource '" << res_name << "' has a conflicting value for "
- << "configuration (" << src_config_value->config << ")");
+ context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource())
+ << "resource '" << res_name << "' has a conflicting value for "
+ << "configuration (" << src_config_value->config << ")");
context->GetDiagnostics()->Note(DiagMessage(dst_value->GetSource())
<< "originally defined here");
return CollisionResult::kConflict;
@@ -243,8 +248,7 @@
bool error = false;
for (auto& src_type : src_package->types) {
- ResourceTableType* dst_type =
- master_package_->FindOrCreateType(src_type->type);
+ ResourceTableType* dst_type = master_package_->FindOrCreateType(src_type->type);
if (!MergeType(context_, src, dst_type, src_type.get())) {
error = true;
continue;
@@ -253,8 +257,7 @@
for (auto& src_entry : src_type->entries) {
std::string entry_name = src_entry->name;
if (mangle_package) {
- entry_name =
- NameMangler::MangleEntry(src_package->name, src_entry->name);
+ entry_name = NameMangler::MangleEntry(src_package->name, src_entry->name);
}
ResourceEntry* dst_entry;
@@ -264,16 +267,14 @@
dst_entry = dst_type->FindEntry(entry_name);
}
- const ResourceNameRef res_name(src_package->name, src_type->type,
- src_entry->name);
+ const ResourceNameRef res_name(src_package->name, src_type->type, src_entry->name);
if (!dst_entry) {
- context_->GetDiagnostics()->Error(
- DiagMessage(src) << "resource " << res_name
- << " does not override an existing resource");
- context_->GetDiagnostics()->Note(
- DiagMessage(src) << "define an <add-resource> tag or use "
- << "--auto-add-overlay");
+ context_->GetDiagnostics()->Error(DiagMessage(src)
+ << "resource " << res_name
+ << " does not override an existing resource");
+ context_->GetDiagnostics()->Note(DiagMessage(src) << "define an <add-resource> tag or use "
+ << "--auto-add-overlay");
error = true;
continue;
}
@@ -291,7 +292,7 @@
if (dst_config_value) {
CollisionResult collision_result =
MergeConfigValue(context_, res_name, overlay, dst_config_value,
- src_config_value.get());
+ src_config_value.get(), &master_table_->string_pool);
if (collision_result == CollisionResult::kConflict) {
error = true;
continue;
@@ -299,25 +300,22 @@
continue;
}
} else {
- dst_config_value = dst_entry->FindOrCreateValue(
- src_config_value->config, src_config_value->product);
+ dst_config_value =
+ dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product);
}
// Continue if we're taking the new resource.
- if (FileReference* f =
- ValueCast<FileReference>(src_config_value->value.get())) {
+ if (FileReference* f = ValueCast<FileReference>(src_config_value->value.get())) {
std::unique_ptr<FileReference> new_file_ref;
if (mangle_package) {
new_file_ref = CloneAndMangleFile(src_package->name, *f);
} else {
- new_file_ref = std::unique_ptr<FileReference>(
- f->Clone(&master_table_->string_pool));
+ new_file_ref = std::unique_ptr<FileReference>(f->Clone(&master_table_->string_pool));
}
if (callback) {
- if (!callback(res_name, src_config_value->config,
- new_file_ref.get(), f)) {
+ if (!callback(res_name, src_config_value->config, new_file_ref.get(), f)) {
error = true;
continue;
}
@@ -341,18 +339,15 @@
std::string mangled_entry = NameMangler::MangleEntry(package, entry.to_string());
std::string newPath = prefix.to_string() + mangled_entry + suffix.to_string();
std::unique_ptr<FileReference> new_file_ref =
- util::make_unique<FileReference>(
- master_table_->string_pool.MakeRef(newPath));
+ util::make_unique<FileReference>(master_table_->string_pool.MakeRef(newPath));
new_file_ref->SetComment(file_ref.GetComment());
new_file_ref->SetSource(file_ref.GetSource());
return new_file_ref;
}
- return std::unique_ptr<FileReference>(
- file_ref.Clone(&master_table_->string_pool));
+ return std::unique_ptr<FileReference>(file_ref.Clone(&master_table_->string_pool));
}
-bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file,
- bool overlay) {
+bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file, bool overlay) {
ResourceTable table;
std::string path = ResourceUtils::BuildResourceFileName(file_desc);
std::unique_ptr<FileReference> file_ref =
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index 742f5a7..3b791b5 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -20,6 +20,14 @@
#include "io/FileSystem.h"
#include "test/Test.h"
+using ::aapt::test::ValueEq;
+using ::testing::Contains;
+using ::testing::NotNull;
+using ::testing::UnorderedElementsAreArray;
+using ::testing::Pointee;
+using ::testing::Field;
+using ::testing::Eq;
+
namespace aapt {
struct TableMergerTest : public ::testing::Test {
@@ -62,26 +70,20 @@
io::FileCollection collection;
ASSERT_TRUE(merger.Merge({}, table_a.get()));
- ASSERT_TRUE(
- merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
+ ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
EXPECT_TRUE(merger.merged_packages().count("com.app.b") != 0);
// Entries from com.app.a should not be mangled.
- AAPT_EXPECT_TRUE(
- final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
- AAPT_EXPECT_TRUE(
- final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
- AAPT_EXPECT_TRUE(final_table.FindResource(
- test::ParseNameOrDie("com.app.a:styleable/view")));
+ EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
+ EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
+ EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:styleable/view")));
// The unmangled name should not be present.
- AAPT_EXPECT_FALSE(
- final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
+ EXPECT_FALSE(final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
// Look for the mangled name.
- AAPT_EXPECT_TRUE(final_table.FindResource(
- test::ParseNameOrDie("com.app.a:id/com.app.b$foo")));
+ EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/com.app.b$foo")));
}
TEST_F(TableMergerTest, MergeFile) {
@@ -100,7 +102,7 @@
FileReference* file = test::GetValueForConfig<FileReference>(
&final_table, "com.app.a:layout/main", test::ParseConfigOrDie("hdpi-v4"));
- ASSERT_NE(nullptr, file);
+ ASSERT_THAT(file, NotNull());
EXPECT_EQ(std::string("res/layout-hdpi-v4/main.xml"), *file->path);
}
@@ -137,17 +139,14 @@
collection.InsertFile("res/xml/file.xml");
ASSERT_TRUE(merger.Merge({}, table_a.get()));
- ASSERT_TRUE(
- merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
+ ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
- FileReference* f =
- test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
- ASSERT_NE(f, nullptr);
+ FileReference* f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
+ ASSERT_THAT(f, NotNull());
EXPECT_EQ(std::string("res/xml/file.xml"), *f->path);
- f = test::GetValue<FileReference>(&final_table,
- "com.app.a:xml/com.app.b$file");
- ASSERT_NE(f, nullptr);
+ f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/com.app.b$file");
+ ASSERT_THAT(f, NotNull());
EXPECT_EQ(std::string("res/xml/com.app.b$file.xml"), *f->path);
}
@@ -171,10 +170,9 @@
ASSERT_TRUE(merger.Merge({}, base.get()));
ASSERT_TRUE(merger.MergeOverlay({}, overlay.get()));
- BinaryPrimitive* foo =
- test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
- ASSERT_NE(nullptr, foo);
- EXPECT_EQ(0x0u, foo->value.data);
+ BinaryPrimitive* foo = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
+ ASSERT_THAT(foo,
+ Pointee(Field(&BinaryPrimitive::value, Field(&android::Res_value::data, Eq(0u)))));
}
TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) {
@@ -301,7 +299,7 @@
ASSERT_FALSE(merger.MergeOverlay({}, table_b.get()));
}
-TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) {
+TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) {
std::unique_ptr<ResourceTable> table_a =
test::ResourceTableBuilder()
.SetPackageId("com.app.a", 0x7f)
@@ -310,15 +308,27 @@
.AddItem("com.app.a:attr/bar")
.AddItem("com.app.a:attr/foo", ResourceId(0x01010000))
.Build())
+ .AddValue("com.app.a:style/Theme",
+ test::StyleBuilder()
+ .SetParent("com.app.a:style/Parent")
+ .AddItem("com.app.a:attr/bar", util::make_unique<Id>())
+ .AddItem("com.app.a:attr/foo", ResourceUtils::MakeBool(false))
+ .Build())
.Build();
std::unique_ptr<ResourceTable> table_b =
test::ResourceTableBuilder()
.SetPackageId("com.app.a", 0x7f)
- .AddValue("com.app.a:styleable/Foo",
- test::StyleableBuilder()
- .AddItem("com.app.a:attr/bat")
- .AddItem("com.app.a:attr/foo")
+ .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder()
+ .AddItem("com.app.a:attr/bat")
+ .AddItem("com.app.a:attr/foo")
+ .Build())
+ .AddValue("com.app.a:style/Theme",
+ test::StyleBuilder()
+ .SetParent("com.app.a:style/OverlayParent")
+ .AddItem("com.app.a:attr/bat", util::make_unique<Id>())
+ .AddItem("com.app.a:attr/foo", ResourceId(0x01010000),
+ ResourceUtils::MakeBool(true))
.Build())
.Build();
@@ -330,18 +340,29 @@
ASSERT_TRUE(merger.Merge({}, table_a.get()));
ASSERT_TRUE(merger.MergeOverlay({}, table_b.get()));
- Styleable* styleable =
- test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
- ASSERT_NE(nullptr, styleable);
+ Styleable* styleable = test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
+ ASSERT_THAT(styleable, NotNull());
std::vector<Reference> expected_refs = {
Reference(test::ParseNameOrDie("com.app.a:attr/bar")),
Reference(test::ParseNameOrDie("com.app.a:attr/bat")),
- Reference(test::ParseNameOrDie("com.app.a:attr/foo"),
- ResourceId(0x01010000)),
+ Reference(test::ParseNameOrDie("com.app.a:attr/foo"), ResourceId(0x01010000)),
};
+ EXPECT_THAT(styleable->entries, UnorderedElementsAreArray(expected_refs));
- EXPECT_EQ(expected_refs, styleable->entries);
+ Style* style = test::GetValue<Style>(&final_table, "com.app.a:style/Theme");
+ ASSERT_THAT(style, NotNull());
+
+ std::vector<Reference> extracted_refs;
+ for (const auto& entry : style->entries) {
+ extracted_refs.push_back(entry.key);
+ }
+ EXPECT_THAT(extracted_refs, UnorderedElementsAreArray(expected_refs));
+
+ const auto expected = ResourceUtils::MakeBool(true);
+ EXPECT_THAT(style->entries, Contains(Field(&Style::Entry::value, Pointee(ValueEq(*expected)))));
+ EXPECT_THAT(style->parent,
+ Eq(make_value(Reference(test::ParseNameOrDie("com.app.a:style/OverlayParent")))));
}
} // namespace aapt
diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h
index 248921f..a937de8 100644
--- a/tools/aapt2/test/Common.h
+++ b/tools/aapt2/test/Common.h
@@ -22,12 +22,14 @@
#include "android-base/logging.h"
#include "android-base/macros.h"
#include "androidfw/StringPiece.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "ConfigDescription.h"
#include "Debug.h"
#include "ResourceTable.h"
#include "ResourceUtils.h"
+#include "ResourceValues.h"
#include "ValueVisitor.h"
#include "io/File.h"
#include "process/IResourceTableConsumer.h"
@@ -51,13 +53,11 @@
return;
case Level::Warn:
- std::cerr << actual_msg.source << ": warn: " << actual_msg.message
- << "." << std::endl;
+ std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
break;
case Level::Error:
- std::cerr << actual_msg.source << ": error: " << actual_msg.message
- << "." << std::endl;
+ std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl;
break;
}
}
@@ -84,11 +84,9 @@
T* GetValueForConfigAndProduct(ResourceTable* table, const android::StringPiece& res_name,
const ConfigDescription& config,
const android::StringPiece& product) {
- Maybe<ResourceTable::SearchResult> result =
- table->FindResource(ParseNameOrDie(res_name));
+ Maybe<ResourceTable::SearchResult> result = table->FindResource(ParseNameOrDie(res_name));
if (result) {
- ResourceConfigValue* config_value =
- result.value().entry->FindValue(config, product);
+ ResourceConfigValue* config_value = result.value().entry->FindValue(config, product);
if (config_value) {
return ValueCast<T>(config_value->value.get());
}
@@ -111,9 +109,13 @@
public:
explicit TestFile(const android::StringPiece& path) : source_(path) {}
- std::unique_ptr<io::IData> OpenAsData() override { return {}; }
+ std::unique_ptr<io::IData> OpenAsData() override {
+ return {};
+ }
- const Source& GetSource() const override { return source_; }
+ const Source& GetSource() const override {
+ return source_;
+ }
private:
DISALLOW_COPY_AND_ASSIGN(TestFile);
@@ -122,6 +124,47 @@
};
} // namespace test
+
+// Workaround gtest bug (https://github.com/google/googletest/issues/443)
+// that does not select base class operator<< for derived class T.
+template <typename T>
+typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<(
+ std::ostream& out, const T& value) {
+ value.Print(&out);
+ return out;
+}
+
+template std::ostream& operator<<<Item>(std::ostream&, const Item&);
+template std::ostream& operator<<<Reference>(std::ostream&, const Reference&);
+template std::ostream& operator<<<Id>(std::ostream&, const Id&);
+template std::ostream& operator<<<RawString>(std::ostream&, const RawString&);
+template std::ostream& operator<<<String>(std::ostream&, const String&);
+template std::ostream& operator<<<StyledString>(std::ostream&, const StyledString&);
+template std::ostream& operator<<<FileReference>(std::ostream&, const FileReference&);
+template std::ostream& operator<<<BinaryPrimitive>(std::ostream&, const BinaryPrimitive&);
+template std::ostream& operator<<<Attribute>(std::ostream&, const Attribute&);
+template std::ostream& operator<<<Style>(std::ostream&, const Style&);
+template std::ostream& operator<<<Array>(std::ostream&, const Array&);
+template std::ostream& operator<<<Plural>(std::ostream&, const Plural&);
+
+// Add a print method to Maybe.
+template <typename T>
+void PrintTo(const Maybe<T>& value, std::ostream* out) {
+ if (value) {
+ *out << ::testing::PrintToString(value.value());
+ } else {
+ *out << "Nothing";
+ }
+}
+
+namespace test {
+
+MATCHER_P(ValueEq, a,
+ std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) {
+ return arg.Equals(&a);
+}
+
+} // namespace test
} // namespace aapt
#endif /* AAPT_TEST_COMMON_H */
diff --git a/tools/aapt2/test/Test.h b/tools/aapt2/test/Test.h
index ec07432..a24c01c 100644
--- a/tools/aapt2/test/Test.h
+++ b/tools/aapt2/test/Test.h
@@ -17,6 +17,7 @@
#ifndef AAPT_TEST_TEST_H
#define AAPT_TEST_TEST_H
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "test/Builders.h"