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

2 files changed, 83 insertions, 13 deletions

diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index a24e0d2f93d0..c0802e60103a 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 ad3674e16774..9dd31e682937 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<Overlayable>("ThemeResources",
+                                                          "overlay://customization");
+  OverlayableItem overlayable_item_second(overlayable_second);
+  overlayable_item_second.policies |= OverlayableItem::Policy::kProduct;
+  std::unique_ptr<ResourceTable> 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<Overlayable>("CustomizableResources",
+                                                         "overlay://customization");
+  OverlayableItem overlayable_item_first(overlayable_first);
+  overlayable_item_first.policies |= OverlayableItem::Policy::kProduct;
+  std::unique_ptr<ResourceTable> table_a =
+      test::ResourceTableBuilder()
+          .SetPackageId("com.app.a", 0x7f)
+          .SetOverlayable("bool/foo", overlayable_item_first)
+          .Build();
+
+  auto overlayable_second = std::make_shared<Overlayable>("CustomizableResources",
                                                           "overlay://theme");
   OverlayableItem overlayable_item_second(overlayable_second);
   overlayable_item_second.policies |= OverlayableItem::Policy::kProduct;
@@ -538,7 +567,36 @@ TEST_F(TableMergerTest, SameResourceDifferentNameFail) {
   ASSERT_FALSE(merger.Merge({}, table_b.get(), false /*overlay*/));
 }
 
-TEST_F(TableMergerTest, SameResourceSameNameFail) {
+TEST_F(TableMergerTest, SameResourceDifferentPoliciesFail) {
+  auto overlayable_first = std::make_shared<Overlayable>("CustomizableResources",
+                                                         "overlay://customization");
+  OverlayableItem overlayable_item_first(overlayable_first);
+  overlayable_item_first.policies |= OverlayableItem::Policy::kProduct;
+  std::unique_ptr<ResourceTable> table_a =
+      test::ResourceTableBuilder()
+          .SetPackageId("com.app.a", 0x7f)
+          .SetOverlayable("bool/foo", overlayable_item_first)
+          .Build();
+
+  auto overlayable_second = std::make_shared<Overlayable>("CustomizableResources",
+                                                          "overlay://customization");
+  OverlayableItem overlayable_item_second(overlayable_second);
+  overlayable_item_second.policies |= OverlayableItem::Policy::kSignature;
+  std::unique_ptr<ResourceTable> 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, SameResourceSameOverlayable) {
   auto overlayable = std::make_shared<Overlayable>("CustomizableResources",
                                                   "overlay://customization");
 
@@ -551,7 +609,7 @@ TEST_F(TableMergerTest, SameResourceSameNameFail) {
           .Build();
 
   OverlayableItem overlayable_item_second(overlayable);
-  overlayable_item_second.policies |= OverlayableItem::Policy::kSystem;
+  overlayable_item_second.policies |= OverlayableItem::Policy::kProduct;
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
@@ -563,7 +621,7 @@ TEST_F(TableMergerTest, SameResourceSameNameFail) {
   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*/));
+  ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/));
 }
 
 }  // namespace aapt