Merge "DO NOT MERGE: Do not allow overlaying of attributes with conflicting formats" into qt-dev

This commit is contained in:
TreeHugger Robot
2019-04-25 09:46:43 +00:00
committed by Android (Google) Code Review
5 changed files with 122 additions and 16 deletions

View File

@@ -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<Attribute>(existing);
Attribute* incoming_attr = ValueCast<Attribute>(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<ResourceConfigValue>(config, product));

View File

@@ -228,13 +228,13 @@ class ResourceTable {
enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew };
using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>;
using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*, bool)>;
// 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> value,

View File

@@ -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 ||

View File

@@ -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 << ")");

View File

@@ -352,6 +352,110 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlayWithAutoAddOverlay) {
ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/));
}
TEST_F(TableMergerTest, OverrideAttributeSameFormatsWithOverlay) {
std::unique_ptr<ResourceTable> base =
test::ResourceTableBuilder()
.SetPackageId("", 0x7f)
.AddValue("attr/foo", test::AttributeBuilder()
.SetTypeMask(android::ResTable_map::TYPE_STRING)
.SetWeak(false)
.Build())
.Build();
std::unique_ptr<ResourceTable> 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<ResourceTable> base =
test::ResourceTableBuilder()
.SetPackageId("", 0x7f)
.AddValue("attr/foo", test::AttributeBuilder()
.SetTypeMask(android::ResTable_map::TYPE_ANY)
.SetWeak(false)
.Build())
.Build();
std::unique_ptr<ResourceTable> 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<ResourceTable> base =
test::ResourceTableBuilder()
.SetPackageId("", 0x7f)
.AddValue("attr/foo", test::AttributeBuilder()
.SetTypeMask(android::ResTable_map::TYPE_FLAGS)
.Build())
.Build();
std::unique_ptr<ResourceTable> 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<ResourceTable> table_a =
test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();