Recognize dynamic res ids as valid

Shared libraries are assigned package id 0. Resource ids that start
with 0x00 are not invalid. This change changes is_valid to accept
dynamic resource ids and adds an is_valid_static method for when
an id must have a non-zero package id.

This also fixes an issue that made layouts in shared libraries that
use internal attributes exclude compiled resource ids from the binay
xml output.

Bug: 146491000
Test: Build a shared library with a layout that uses app attributes
      as well as android attribute and verify that all attributes
      have assigned resource ids using `aapt2 dump xmltree`

Change-Id: Ibc0407c610ffc98d7aaf233c37c065912ab0d516

5 files changed, 19 insertions, 18 deletions

diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index 67ba895e51d1..c49c370bcc44 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -147,10 +147,11 @@ struct ResourceId {
   ResourceId(uint32_t res_id);  // NOLINT(google-explicit-constructor)
   ResourceId(uint8_t p, uint8_t t, uint16_t e);
 
-  bool is_valid() const;
+  // Returns true if the ID is a valid ID that is not dynamic (package ID cannot be 0)
+  bool is_valid_static() const;
 
   // Returns true if the ID is a valid ID or dynamic ID (package ID can be 0).
-  bool is_valid_dynamic() const;
+  bool is_valid() const;
 
   uint8_t package_id() const;
   uint8_t type_id() const;
@@ -233,11 +234,11 @@ inline ResourceId::ResourceId(uint32_t res_id) : id(res_id) {}
 inline ResourceId::ResourceId(uint8_t p, uint8_t t, uint16_t e)
     : id((p << 24) | (t << 16) | e) {}
 
-inline bool ResourceId::is_valid() const {
+inline bool ResourceId::is_valid_static() const {
   return (id & 0xff000000u) != 0 && (id & 0x00ff0000u) != 0;
 }
 
-inline bool ResourceId::is_valid_dynamic() const {
+inline bool ResourceId::is_valid() const {
   return (id & 0x00ff0000u) != 0;
 }
 
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 1773b5a8addf..e0a9a31eee8b 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -398,7 +398,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
 
   // Check for package names appearing twice with two different package ids
   ResourceTablePackage* package = FindOrCreatePackage(name.package);
-  if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {
+  if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) {
     diag->Error(DiagMessage(source)
                     << "trying to add resource '" << name << "' with ID " << res_id
                     << " but package '" << package->name << "' already has ID "
@@ -407,9 +407,9 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
   }
 
   // Whether or not to error on duplicate resources
-  bool check_id = validate_resources_ && res_id.is_valid_dynamic();
+  bool check_id = validate_resources_ && res_id.is_valid();
   // Whether or not to create a duplicate resource if the id does not match
-  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();
+  bool use_id = !validate_resources_ && res_id.is_valid();
 
   ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()
                                                                         : Maybe<uint8_t>());
@@ -463,7 +463,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI
     }
   }
 
-  if (res_id.is_valid_dynamic()) {
+  if (res_id.is_valid()) {
     package->id = res_id.package_id();
     type->id = res_id.type_id();
     entry->id = res_id.entry_id();
@@ -504,7 +504,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil
 
   // Check for package names appearing twice with two different package ids
   ResourceTablePackage* package = FindOrCreatePackage(name.package);
-  if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) {
+  if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) {
     diag->Error(DiagMessage(source)
                     << "trying to add resource '" << name << "' with ID " << res_id
                     << " but package '" << package->name << "' already has ID "
@@ -513,9 +513,9 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil
   }
 
   // Whether or not to error on duplicate resources
-  bool check_id = validate_resources_ && res_id.is_valid_dynamic();
+  bool check_id = validate_resources_ && res_id.is_valid();
   // Whether or not to create a duplicate resource if the id does not match
-  bool use_id = !validate_resources_ && res_id.is_valid_dynamic();
+  bool use_id = !validate_resources_ && res_id.is_valid();
 
   ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id()
                                                                         : Maybe<uint8_t>());
@@ -541,7 +541,7 @@ bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibil
     return false;
   }
 
-  if (res_id.is_valid_dynamic()) {
+  if (res_id.is_valid()) {
     package->id = res_id.package_id();
     type->id = res_id.type_id();
     entry->id = res_id.entry_id();
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 03009aaeead2..3623b1112bc6 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -516,7 +516,7 @@ Maybe<ResourceId> ParseResourceId(const StringPiece& str) {
   if (android::ResTable::stringToInt(str16.data(), str16.size(), &value)) {
     if (value.dataType == android::Res_value::TYPE_INT_HEX) {
       ResourceId id(value.data);
-      if (id.is_valid_dynamic()) {
+      if (id.is_valid()) {
         return id;
       }
     }
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 34b46c552e0c..4f0fa8ae29ba 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -117,7 +117,7 @@ bool Reference::Equals(const Value* value) const {
 
 bool Reference::Flatten(android::Res_value* out_value) const {
   const ResourceId resid = id.value_or_default(ResourceId(0));
-  const bool dynamic = resid.is_valid_dynamic() && is_dynamic;
+  const bool dynamic = resid.is_valid() && is_dynamic;
 
   if (reference_type == Reference::Type::kResource) {
     if (dynamic) {
@@ -159,7 +159,7 @@ void Reference::Print(std::ostream* out) const {
     *out << name.value();
   }
 
-  if (id && id.value().is_valid_dynamic()) {
+  if (id && id.value().is_valid()) {
     if (name) {
       *out << " ";
     }
@@ -196,7 +196,7 @@ static void PrettyPrintReferenceImpl(const Reference& ref, bool print_package, P
       printer->Print("/");
       printer->Print(name.entry);
     }
-  } else if (ref.id && ref.id.value().is_valid_dynamic()) {
+  } else if (ref.id && ref.id.value().is_valid()) {
     printer->Print(ref.id.value().to_string());
   }
 }
diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp
index bc09f19b94d5..83e20b5833b9 100644
--- a/tools/aapt2/process/SymbolTable.cpp
+++ b/tools/aapt2/process/SymbolTable.cpp
@@ -340,7 +340,7 @@ std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindByName(
     }
 
     res_id = asset_manager_.GetResourceId(real_name.to_string());
-    if (res_id.is_valid() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) {
+    if (res_id.is_valid_static() && asset_manager_.GetResourceFlags(res_id.id, &type_spec_flags)) {
       found = true;
       return false;
     }
@@ -379,7 +379,7 @@ static Maybe<ResourceName> GetResourceName(android::AssetManager2& am,
 
 std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindById(
     ResourceId id) {
-  if (!id.is_valid()) {
+  if (!id.is_valid_static()) {
     // Exit early and avoid the error logs from AssetManager.
     return {};
   }