From ced9a5c1081d36edd2e5936fdbe849bfed06136f Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Fri, 5 Apr 2019 10:44:16 -0700 Subject: [PATCH] Allow merging of resources with the same overlayable If a resource is redefined with the same overlayable name, actor, and policies, do not error. Bug: 128843658 Test: m -j Launcher3 && aapt2_tests Change-Id: I2c79f7d9fa7ff16b38ec41cec7e9804d39b372d5 --- tools/aapt2/link/TableMerger.cpp | 32 ++++++++---- tools/aapt2/link/TableMerger_test.cpp | 72 ++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index a24e0d2f93d06..c0802e60103a6 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -138,17 +138,29 @@ static bool MergeEntry(IAaptContext* context, const Source& src, if (src_entry->overlayable_item) { if (dst_entry->overlayable_item) { - // Do not allow a resource with an overlayable declaration to have that overlayable - // declaration redefined - context->GetDiagnostics()->Error(DiagMessage(src_entry->overlayable_item.value().source) - << "duplicate overlayable declaration for resource '" - << src_entry->name << "'"); - context->GetDiagnostics()->Error(DiagMessage(dst_entry->overlayable_item.value().source) - << "previous declaration here"); - return false; - } else { - dst_entry->overlayable_item = std::move(src_entry->overlayable_item); + CHECK(src_entry->overlayable_item.value().overlayable != nullptr); + Overlayable* src_overlayable = src_entry->overlayable_item.value().overlayable.get(); + + CHECK(dst_entry->overlayable_item.value().overlayable != nullptr); + Overlayable* dst_overlayable = dst_entry->overlayable_item.value().overlayable.get(); + + if (src_overlayable->name != dst_overlayable->name + || src_overlayable->actor != dst_overlayable->actor + || src_entry->overlayable_item.value().policies != + dst_entry->overlayable_item.value().policies) { + + // Do not allow a resource with an overlayable declaration to have that overlayable + // declaration redefined. + context->GetDiagnostics()->Error(DiagMessage(src_entry->overlayable_item.value().source) + << "duplicate overlayable declaration for resource '" + << src_entry->name << "'"); + context->GetDiagnostics()->Error(DiagMessage(dst_entry->overlayable_item.value().source) + << "previous declaration here"); + return false; + } } + + dst_entry->overlayable_item = std::move(src_entry->overlayable_item); } return true; diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index ad3674e167746..9dd31e6829378 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -521,6 +521,35 @@ TEST_F(TableMergerTest, SameResourceDifferentNameFail) { .Build(); auto overlayable_second = std::make_shared("ThemeResources", + "overlay://customization"); + OverlayableItem overlayable_item_second(overlayable_second); + overlayable_item_second.policies |= OverlayableItem::Policy::kProduct; + std::unique_ptr table_b = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .SetOverlayable("bool/foo", overlayable_item_second) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = true; + TableMerger merger(context_.get(), &final_table, options); + ASSERT_TRUE(merger.Merge({}, table_a.get(), false /*overlay*/)); + ASSERT_FALSE(merger.Merge({}, table_b.get(), false /*overlay*/)); +} + +TEST_F(TableMergerTest, SameResourceDifferentActorFail) { + auto overlayable_first = std::make_shared("CustomizableResources", + "overlay://customization"); + OverlayableItem overlayable_item_first(overlayable_first); + overlayable_item_first.policies |= OverlayableItem::Policy::kProduct; + std::unique_ptr table_a = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .SetOverlayable("bool/foo", overlayable_item_first) + .Build(); + + auto overlayable_second = std::make_shared("CustomizableResources", "overlay://theme"); OverlayableItem overlayable_item_second(overlayable_second); overlayable_item_second.policies |= OverlayableItem::Policy::kProduct; @@ -538,11 +567,10 @@ TEST_F(TableMergerTest, SameResourceDifferentNameFail) { ASSERT_FALSE(merger.Merge({}, table_b.get(), false /*overlay*/)); } -TEST_F(TableMergerTest, SameResourceSameNameFail) { - auto overlayable = std::make_shared("CustomizableResources", - "overlay://customization"); - - OverlayableItem overlayable_item_first(overlayable); +TEST_F(TableMergerTest, SameResourceDifferentPoliciesFail) { + auto overlayable_first = std::make_shared("CustomizableResources", + "overlay://customization"); + OverlayableItem overlayable_item_first(overlayable_first); overlayable_item_first.policies |= OverlayableItem::Policy::kProduct; std::unique_ptr table_a = test::ResourceTableBuilder() @@ -550,8 +578,10 @@ TEST_F(TableMergerTest, SameResourceSameNameFail) { .SetOverlayable("bool/foo", overlayable_item_first) .Build(); - OverlayableItem overlayable_item_second(overlayable); - overlayable_item_second.policies |= OverlayableItem::Policy::kSystem; + auto overlayable_second = std::make_shared("CustomizableResources", + "overlay://customization"); + OverlayableItem overlayable_item_second(overlayable_second); + overlayable_item_second.policies |= OverlayableItem::Policy::kSignature; std::unique_ptr table_b = test::ResourceTableBuilder() .SetPackageId("com.app.a", 0x7f) @@ -566,4 +596,32 @@ TEST_F(TableMergerTest, SameResourceSameNameFail) { ASSERT_FALSE(merger.Merge({}, table_b.get(), false /*overlay*/)); } +TEST_F(TableMergerTest, SameResourceSameOverlayable) { + auto overlayable = std::make_shared("CustomizableResources", + "overlay://customization"); + + OverlayableItem overlayable_item_first(overlayable); + overlayable_item_first.policies |= OverlayableItem::Policy::kProduct; + std::unique_ptr table_a = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .SetOverlayable("bool/foo", overlayable_item_first) + .Build(); + + OverlayableItem overlayable_item_second(overlayable); + overlayable_item_second.policies |= OverlayableItem::Policy::kProduct; + std::unique_ptr table_b = + test::ResourceTableBuilder() + .SetPackageId("com.app.a", 0x7f) + .SetOverlayable("bool/foo", overlayable_item_second) + .Build(); + + ResourceTable final_table; + TableMergerOptions options; + options.auto_add_overlay = true; + TableMerger merger(context_.get(), &final_table, options); + ASSERT_TRUE(merger.Merge({}, table_a.get(), false /*overlay*/)); + ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/)); +} + } // namespace aapt