Refactor policy parsing

This change removes the ability for an overlayable resource to be
defined in multiple policy blocks within the same overlayable. This
change also changes aapt2 to use a bit mask to keep track of the parsed
policies.

Bug: 110869880
Bug: 120298168
Test: aapt2_tests
Change-Id: Ie26cd913f94a16c0b312f222bccfa48f62feceaa

3 files changed, 56 insertions, 48 deletions

diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 1b6626a8dfe9..8cbc03738677 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -374,8 +374,8 @@ bool ReferenceLinker::Consume(IAaptContext* context, ResourceTable* table) {
         }
 
         // Ensure that definitions for values declared as overlayable exist
-        if (!entry->overlayable_declarations.empty() && entry->values.empty()) {
-          context->GetDiagnostics()->Error(DiagMessage(entry->overlayable_declarations[0].source)
+        if (entry->overlayable && entry->values.empty()) {
+          context->GetDiagnostics()->Error(DiagMessage(entry->overlayable.value().source)
                                            << "no definition for overlayable symbol '"
                                            << name << "'");
           error = true;
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index d777e22fa4b7..22e1723591a8 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -134,35 +134,21 @@ static bool MergeEntry(IAaptContext* context, const Source& src,
     dst_entry->allow_new = std::move(src_entry->allow_new);
   }
 
-  for (auto& src_overlayable : src_entry->overlayable_declarations) {
-    for (auto& dst_overlayable : dst_entry->overlayable_declarations) {
-      // An overlayable resource cannot be declared twice with the same policy
-      if (src_overlayable.policy == dst_overlayable.policy) {
-        context->GetDiagnostics()->Error(DiagMessage(src_overlayable.source)
-                                             << "duplicate overlayable declaration for resource '"
-                                             << src_entry->name << "'");
-        context->GetDiagnostics()->Error(DiagMessage(dst_overlayable.source)
-                                             << "previous declaration here");
-        return false;
-      }
-
-      // An overlayable resource cannot be declared once with a policy and without a policy because
-      // the policy becomes unused
-      if (!src_overlayable.policy || !dst_overlayable.policy) {
-        context->GetDiagnostics()->Error(DiagMessage(src_overlayable.source)
-                                             << "overlayable resource '" << src_entry->name
-                                             << "' declared once with a policy and once with no "
-                                             << "policy");
-        context->GetDiagnostics()->Error(DiagMessage(dst_overlayable.source)
-                                             << "previous declaration here");
-        return false;
-      }
+  if (src_entry->overlayable) {
+    if (dst_entry->overlayable) {
+      // Do not allow a resource with an overlayable declaration to have that overlayable
+      // declaration redefined
+      context->GetDiagnostics()->Error(DiagMessage(src_entry->overlayable.value().source)
+                                       << "duplicate overlayable declaration for resource '"
+                                       << src_entry->name << "'");
+      context->GetDiagnostics()->Error(DiagMessage(dst_entry->overlayable.value().source)
+                                       << "previous declaration here");
+      return false;
+    } else {
+      dst_entry->overlayable = std::move(src_entry->overlayable);
     }
   }
 
-  dst_entry->overlayable_declarations.insert(dst_entry->overlayable_declarations.end(),
-                                             src_entry->overlayable_declarations.begin(),
-                                             src_entry->overlayable_declarations.end());
   return true;
 }
 
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index d6579d37b452..17b2a83bad04 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/tools/aapt2/link/TableMerger_test.cpp
@@ -436,17 +436,21 @@ TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) {
               Eq(make_value(Reference(test::ParseNameOrDie("com.app.a:style/OverlayParent")))));
 }
 
-TEST_F(TableMergerTest, AddOverlayable) {
+TEST_F(TableMergerTest, SetOverlayable) {
+  Overlayable overlayable{};
+  overlayable.policies |= Overlayable::Policy::kProduct;
+  overlayable.policies |= Overlayable::Policy::kVendor;
+
   std::unique_ptr<ResourceTable> table_a =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProduct)
+          .SetOverlayable("bool/foo", overlayable)
           .Build();
 
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProductServices)
+          .AddSimple("bool/foo")
           .Build();
 
   ResourceTable final_table;
@@ -457,26 +461,28 @@ TEST_F(TableMergerTest, AddOverlayable) {
   ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/));
 
   const ResourceName name = test::ParseNameOrDie("com.app.a:bool/foo");
-  Maybe<ResourceTable::SearchResult> result = final_table.FindResource(name);
-  ASSERT_TRUE(result);
-  ASSERT_THAT(result.value().entry->overlayable_declarations.size(), Eq(2));
-  ASSERT_THAT(result.value().entry->overlayable_declarations[0].policy,
-              Eq(Overlayable::Policy::kProduct));
-  ASSERT_THAT(result.value().entry->overlayable_declarations[1].policy,
-              Eq(Overlayable::Policy::kProductServices));
+  Maybe<ResourceTable::SearchResult> search_result = final_table.FindResource(name);
+  ASSERT_TRUE(search_result);
+  ASSERT_TRUE(search_result.value().entry->overlayable);
+  Overlayable& result_overlayable = search_result.value().entry->overlayable.value();
+  EXPECT_THAT(result_overlayable.policies, Eq(Overlayable::Policy::kProduct
+                                              | Overlayable::Policy::kVendor));
 }
 
-TEST_F(TableMergerTest, AddDuplicateOverlayableFail) {
+TEST_F(TableMergerTest, SetOverlayableLater) {
   std::unique_ptr<ResourceTable> table_a =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProduct)
+          .AddSimple("bool/foo")
           .Build();
 
+  Overlayable overlayable{};
+  overlayable.policies |= Overlayable::Policy::kPublic;
+  overlayable.policies |= Overlayable::Policy::kProductServices;
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProduct)
+          .SetOverlayable("bool/foo", overlayable)
           .Build();
 
   ResourceTable final_table;
@@ -484,20 +490,32 @@ TEST_F(TableMergerTest, AddDuplicateOverlayableFail) {
   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*/));
+
+  const ResourceName name = test::ParseNameOrDie("com.app.a:bool/foo");
+  Maybe<ResourceTable::SearchResult> search_result = final_table.FindResource(name);
+  ASSERT_TRUE(search_result);
+  ASSERT_TRUE(search_result.value().entry->overlayable);
+  Overlayable& result_overlayable = search_result.value().entry->overlayable.value();
+  EXPECT_THAT(result_overlayable.policies, Eq(Overlayable::Policy::kPublic
+                                              | Overlayable::Policy::kProductServices));
 }
 
-TEST_F(TableMergerTest, AddOverlayablePolicyAndNoneFirstFail) {
+TEST_F(TableMergerTest, SetOverlayableSamePolicesFail) {
+  Overlayable overlayable_first{};
+  overlayable_first.policies |= Overlayable::Policy::kProduct;
   std::unique_ptr<ResourceTable> table_a =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", {})
+          .SetOverlayable("bool/foo", overlayable_first)
           .Build();
 
+  Overlayable overlayable_second{};
+  overlayable_second.policies |= Overlayable::Policy::kProduct;
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProduct)
+          .SetOverlayable("bool/foo", overlayable_second)
           .Build();
 
   ResourceTable final_table;
@@ -508,17 +526,21 @@ TEST_F(TableMergerTest, AddOverlayablePolicyAndNoneFirstFail) {
   ASSERT_FALSE(merger.Merge({}, table_b.get(), false /*overlay*/));
 }
 
-TEST_F(TableMergerTest, AddOverlayablePolicyAndNoneLastFail) {
+TEST_F(TableMergerTest, SetOverlayableDifferentPolicesFail) {
+  Overlayable overlayable_first{};
+  overlayable_first.policies |= Overlayable::Policy::kVendor;
   std::unique_ptr<ResourceTable> table_a =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", Overlayable::Policy::kProduct)
+          .SetOverlayable("bool/foo",overlayable_first)
           .Build();
 
+  Overlayable overlayable_second{};
+  overlayable_second.policies |= Overlayable::Policy::kProduct;
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddOverlayable("bool/foo", {})
+          .SetOverlayable("bool/foo", overlayable_second)
           .Build();
 
   ResourceTable final_table;