diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 1773b5a8addfd..836e199593fce 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -267,7 +267,8 @@ bool ResourceEntry::HasDefaultValue() const { // A DECL will override a USE without error. Two DECLs must match in their format for there to be // no error. ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* existing, - Value* incoming) { + Value* incoming, + bool overlay) { Attribute* existing_attr = ValueCast(existing); Attribute* incoming_attr = ValueCast(incoming); if (!incoming_attr) { @@ -281,7 +282,7 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist } // The existing and incoming values are strong, this is an error // if the values are not both attributes. - return CollisionResult::kConflict; + return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict; } if (!existing_attr) { @@ -292,7 +293,7 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist } // The existing value is not an attribute and it is strong, // so the incoming attribute value is an error. - return CollisionResult::kConflict; + return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict; } CHECK(incoming_attr != nullptr && existing_attr != nullptr); @@ -323,8 +324,9 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist return CollisionResult::kConflict; } -ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /** existing **/, - Value* /** incoming **/) { +ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /* existing */, + Value* /* incoming */, + bool /* overlay */) { return CollisionResult::kKeepBoth; } @@ -440,7 +442,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI // Resource does not exist, add it now. config_value->value = std::move(value); } else { - switch (conflict_resolver(config_value->value.get(), value.get())) { + switch (conflict_resolver(config_value->value.get(), value.get(), false /* overlay */)) { case CollisionResult::kKeepBoth: // Insert the value ignoring for duplicate configurations entry->values.push_back(util::make_unique(config, product)); diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 30ba1aed25f89..e8793800b1487 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -228,13 +228,13 @@ class ResourceTable { enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew }; - using CollisionResolverFunc = std::function; + using CollisionResolverFunc = std::function; // When a collision of resources occurs, this method decides which value to keep. - static CollisionResult ResolveValueCollision(Value* existing, Value* incoming); + static CollisionResult ResolveValueCollision(Value* existing, Value* incoming, bool overlay); // When a collision of resources occurs, this method keeps both values - static CollisionResult IgnoreCollision(Value* existing, Value* incoming); + static CollisionResult IgnoreCollision(Value* existing, Value* incoming, bool overlay); bool AddResource(const ResourceNameRef& name, const android::ConfigDescription& config, const android::StringPiece& product, std::unique_ptr value, diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 34b46c552e0c2..696012786e6de 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -574,6 +574,10 @@ bool Attribute::Equals(const Value* value) const { } bool Attribute::IsCompatibleWith(const Attribute& attr) const { + if (Equals(&attr)) { + return true; + } + // If the high bits are set on any of these attribute type masks, then they are incompatible. // We don't check that flags and enums are identical. if ((type_mask & ~android::ResTable_map::TYPE_ANY) != 0 || diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index c0802e60103a6..3f65e868505de 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -188,7 +188,7 @@ static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Val } } // Delegate to the default handler. - return ResourceTable::ResolveValueCollision(existing, incoming); + return ResourceTable::ResolveValueCollision(existing, incoming, true /* overlay */); } static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, @@ -206,15 +206,11 @@ static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, if (overlay) { 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, + false /* overlay */); } if (collision_result == CollisionResult::kConflict) { - if (overlay) { - return CollisionResult::kTakeNew; - } - - // Error! context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource()) << "resource '" << res_name << "' has a conflicting value for " << "configuration (" << src_config_value->config << ")"); diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 9dd31e6829378..be9c84b3f8a68 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -352,6 +352,110 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlayWithAutoAddOverlay) { ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/)); } +TEST_F(TableMergerTest, OverrideAttributeSameFormatsWithOverlay) { + std::unique_ptr base = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_STRING) + .SetWeak(false) + .Build()) + .Build(); + + std::unique_ptr overlay = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_STRING) + .SetWeak(false) + .Build()) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = false; + TableMerger merger(context_.get(), &final_table, options); + + ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/)); + ASSERT_TRUE(merger.Merge({}, overlay.get(), true /*overlay*/)); +} + +TEST_F(TableMergerTest, FailToOverrideConflictingAttributeFormatsWithOverlay) { + std::unique_ptr base = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_ANY) + .SetWeak(false) + .Build()) + .Build(); + + std::unique_ptr overlay = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_STRING) + .SetWeak(false) + .Build()) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = false; + TableMerger merger(context_.get(), &final_table, options); + + ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/)); + ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/)); +} + +TEST_F(TableMergerTest, FailToOverrideConflictingFlagsAndEnumsWithOverlay) { + std::unique_ptr base = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_FLAGS) + .Build()) + .Build(); + + std::unique_ptr overlay = + test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_FLAGS) + .SetWeak(false) + .Build()) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = false; + TableMerger merger(context_.get(), &final_table, options); + + ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/)); + ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/)); + + base = test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_ENUM) + .Build()) + .Build(); + + overlay = test::ResourceTableBuilder() + .SetPackageId("", 0x7f) + .AddValue("attr/foo", test::AttributeBuilder() + .SetTypeMask(android::ResTable_map::TYPE_ENUM) + .SetWeak(false) + .Build()) + .Build(); + + ResourceTable final_table2; + TableMerger merger2(context_.get(), &final_table2, options); + + ASSERT_TRUE(merger2.Merge({}, base.get(), false /*overlay*/)); + ASSERT_FALSE(merger2.Merge({}, overlay.get(), true /*overlay*/)); +} + TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) { std::unique_ptr table_a = test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();