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/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 =