diff options
author | 2021-03-29 20:04:00 +0000 | |
---|---|---|
committer | 2021-03-29 20:04:00 +0000 | |
commit | 0f97905040606c3bba2f90d9dba33c16cb76873a (patch) | |
tree | e05d77ff63ccf67680d0c58ae6ce1de6be11a645 | |
parent | 74d4d0297792a42aca2cd11cab3cae851d2b0401 (diff) | |
parent | 1d008d1d2a73a8b796add4e18924fcc99220a839 (diff) |
Merge changes I851f9fb9,I60dbcb24 into sc-dev
* changes:
Refactor aapt2 tests ResourceTable changes
Prepare aapt2 for multiple ids per type
38 files changed, 1308 insertions, 1386 deletions
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index 82da24959521..351e4202af65 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -256,53 +256,35 @@ class ValueBodyPrinter : public ConstValueVisitor { void Debug::PrintTable(const ResourceTable& table, const DebugPrintTableOptions& options, Printer* printer) { - for (const auto& package : table.packages) { - ValueHeadlinePrinter headline_printer(package->name, printer); - ValueBodyPrinter body_printer(package->name, printer); + const auto table_view = table.GetPartitionedView(); + for (const auto& package : table_view.packages) { + ValueHeadlinePrinter headline_printer(package.name, printer); + ValueBodyPrinter body_printer(package.name, printer); printer->Print("Package name="); - printer->Print(package->name); - if (package->id) { - printer->Print(StringPrintf(" id=%02x", package->id.value())); + printer->Print(package.name); + if (package.id) { + printer->Print(StringPrintf(" id=%02x", package.id.value())); } printer->Println(); printer->Indent(); - for (const auto& type : package->types) { + for (const auto& type : package.types) { printer->Print("type "); - printer->Print(to_string(type->type)); - if (type->id) { - printer->Print(StringPrintf(" id=%02x", type->id.value())); - } - printer->Println(StringPrintf(" entryCount=%zd", type->entries.size())); - - std::vector<const ResourceEntry*> sorted_entries; - for (const auto& entry : type->entries) { - auto iter = std::lower_bound( - sorted_entries.begin(), sorted_entries.end(), entry.get(), - [](const ResourceEntry* a, const ResourceEntry* b) -> bool { - if (a->id && b->id) { - return a->id.value() < b->id.value(); - } else if (a->id) { - return true; - } else { - return false; - } - }); - sorted_entries.insert(iter, entry.get()); + printer->Print(to_string(type.type)); + if (type.id) { + printer->Print(StringPrintf(" id=%02x", type.id.value())); } + printer->Println(StringPrintf(" entryCount=%zd", type.entries.size())); printer->Indent(); - for (const ResourceEntry* entry : sorted_entries) { - const ResourceId id(package->id.value_or_default(0), type->id.value_or_default(0), - entry->id.value_or_default(0)); - + for (const ResourceEntry* entry : type.entries) { printer->Print("resource "); - printer->Print(id.to_string()); + printer->Print(entry->id.value_or_default(0).to_string()); printer->Print(" "); // Write the name without the package (this is obvious and too verbose). - printer->Print(to_string(type->type)); + printer->Print(to_string(type.type)); printer->Print("/"); printer->Print(entry->name); diff --git a/tools/aapt2/LoadedApk.cpp b/tools/aapt2/LoadedApk.cpp index e930b47f2901..830bc5fa36aa 100644 --- a/tools/aapt2/LoadedApk.cpp +++ b/tools/aapt2/LoadedApk.cpp @@ -113,7 +113,7 @@ std::unique_ptr<LoadedApk> LoadedApk::LoadProtoApkFromFileCollection( } std::string error; - table = util::make_unique<ResourceTable>(/** validate_resources **/ false); + table = util::make_unique<ResourceTable>(ResourceTable::Validation::kDisabled); if (!DeserializeTableFromPb(pb_table, collection.get(), table.get(), &error)) { diag->Error(DiagMessage(source) << "failed to deserialize " << kProtoResourceTablePath << ": " << error); @@ -157,7 +157,7 @@ std::unique_ptr<LoadedApk> LoadedApk::LoadBinaryApkFromFileCollection( io::IFile* table_file = collection->FindFile(kApkResourceTablePath); if (table_file != nullptr) { - table = util::make_unique<ResourceTable>(/** validate_resources **/ false); + table = util::make_unique<ResourceTable>(ResourceTable::Validation::kDisabled); std::unique_ptr<io::IData> data = table_file->OpenAsData(); if (data == nullptr) { diag->Error(DiagMessage(source) << "failed to open " << kApkResourceTablePath); diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index 8fe0eb3b51bf..cf938703e1e9 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -138,7 +138,7 @@ struct ResourceId { uint32_t id; ResourceId(); - ResourceId(const ResourceId& rhs); + ResourceId(const ResourceId& rhs) = default; ResourceId(uint32_t res_id); // NOLINT(google-explicit-constructor) ResourceId(uint8_t p, uint8_t t, uint16_t e); @@ -222,8 +222,6 @@ bool operator<(const ResourceKeyRef& a, const ResourceKeyRef& b); inline ResourceId::ResourceId() : id(0) {} -inline ResourceId::ResourceId(const ResourceId& rhs) : id(rhs.id) {} - inline ResourceId::ResourceId(uint32_t res_id) : id(res_id) {} inline ResourceId::ResourceId(uint8_t p, uint8_t t, uint16_t e) diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 3d9be59dd960..a447cef6dbbb 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -118,43 +118,43 @@ static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag, Parsed res->comment = trimmed_comment.to_string(); } + NewResourceBuilder res_builder(res->name); if (res->visibility_level != Visibility::Level::kUndefined) { Visibility visibility; visibility.level = res->visibility_level; visibility.source = res->source; visibility.comment = res->comment; - if (!table->SetVisibilityWithId(res->name, visibility, res->id, diag)) { - return false; - } + res_builder.SetVisibility(visibility); + } + + if (res->id.is_valid()) { + res_builder.SetId(res->id); } if (res->allow_new) { AllowNew allow_new; allow_new.source = res->source; allow_new.comment = res->comment; - if (!table->SetAllowNew(res->name, allow_new, diag)) { - return false; - } + res_builder.SetAllowNew(allow_new); } if (res->overlayable_item) { - if (!table->SetOverlayable(res->name, res->overlayable_item.value(), diag)) { - return false; - } + res_builder.SetOverlayable(res->overlayable_item.value()); } if (res->value != nullptr) { // Attach the comment, source and config to the value. res->value->SetComment(std::move(res->comment)); res->value->SetSource(std::move(res->source)); + res_builder.SetValue(std::move(res->value), res->config, res->product); + } - if (!table->AddResourceWithId(res->name, res->id, res->config, res->product, - std::move(res->value), diag)) { + bool error = false; + if (!res->name.entry.empty()) { + if (!table->AddResource(res_builder.Build(), diag)) { return false; } } - - bool error = false; for (ParsedResource& child : res->child_resources) { error |= !AddResourcesToTable(table, diag, &child); } @@ -751,7 +751,7 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, // table. std::unique_ptr<Id> id = util::make_unique<Id>(); id->SetSource(source_.WithLine(begin_xml_line)); - table_->AddResource(name, {}, {}, std::move(id), diag_); + table_->AddResource(NewResourceBuilder(name).SetValue(std::move(id)).Build(), diag_); }; // Process the raw value. diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 9b70079a98c9..53bfa0b37475 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -831,25 +831,13 @@ TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) { Maybe<ResourceTable::SearchResult> result = table_.FindResource(test::ParseNameOrDie("attr/foo")); ASSERT_TRUE(result); - - ASSERT_TRUE(result.value().package->id); - ASSERT_TRUE(result.value().type->id); ASSERT_TRUE(result.value().entry->id); - ResourceId actual_id(result.value().package->id.value(), - result.value().type->id.value(), - result.value().entry->id.value()); - EXPECT_THAT(actual_id, Eq(ResourceId(0x01010040))); + EXPECT_THAT(result.value().entry->id.value(), Eq(ResourceId(0x01010040))); result = table_.FindResource(test::ParseNameOrDie("attr/bar")); ASSERT_TRUE(result); - - ASSERT_TRUE(result.value().package->id); - ASSERT_TRUE(result.value().type->id); ASSERT_TRUE(result.value().entry->id); - actual_id = ResourceId(result.value().package->id.value(), - result.value().type->id.value(), - result.value().entry->id.value()); - EXPECT_THAT(actual_id, Eq(ResourceId(0x01010041))); + EXPECT_THAT(result.value().entry->id.value(), Eq(ResourceId(0x01010041))); } TEST_F(ResourceParserTest, StrongestSymbolVisibilityWins) { diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index e0a9a31eee8b..cff98728c325 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -18,20 +18,18 @@ #include <algorithm> #include <memory> -#include <string> #include <tuple> #include "android-base/logging.h" -#include "android-base/stringprintf.h" #include "androidfw/ConfigDescription.h" #include "androidfw/ResourceTypes.h" -#include "Debug.h" #include "NameMangler.h" +#include "ResourceUtils.h" #include "ResourceValues.h" #include "ValueVisitor.h" -#include "trace/TraceBuffer.h" #include "text/Unicode.h" +#include "trace/TraceBuffer.h" #include "util/Util.h" using ::aapt::text::IsValidResourceEntryName; @@ -43,135 +41,108 @@ namespace aapt { const char* Overlayable::kActorScheme = "overlay"; -static bool less_than_type_and_id(const std::unique_ptr<ResourceTableType>& lhs, - const std::pair<ResourceType, Maybe<uint8_t>>& rhs) { - return lhs->type < rhs.first || (lhs->type == rhs.first && rhs.second && lhs->id < rhs.second); +namespace { +bool less_than_type(const std::unique_ptr<ResourceTableType>& lhs, ResourceType rhs) { + return lhs->type < rhs; } template <typename T> -static bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, const StringPiece& rhs) { - return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0; +bool less_than_type_and_id(const T& lhs, const std::pair<ResourceType, Maybe<uint8_t>>& rhs) { + return lhs.id != rhs.second ? lhs.id < rhs.second : lhs.type < rhs.first; } template <typename T> -static bool less_than_struct_with_name_and_id(const std::unique_ptr<T>& lhs, - const std::pair<StringPiece, Maybe<uint16_t>>& rhs) { - int name_cmp = lhs->name.compare(0, lhs->name.size(), rhs.first.data(), rhs.first.size()); - return name_cmp < 0 || (name_cmp == 0 && rhs.second && lhs->id < rhs.second); -} - -ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) const { - const auto last = packages.end(); - auto iter = std::lower_bound(packages.begin(), last, name, - less_than_struct_with_name<ResourceTablePackage>); - if (iter != last && name == (*iter)->name) { - return iter->get(); - } - return nullptr; +bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, const StringPiece& rhs) { + return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0; } -ResourceTablePackage* ResourceTable::FindPackageById(uint8_t id) const { - for (auto& package : packages) { - if (package->id && package->id.value() == id) { - return package.get(); - } - } - return nullptr; +template <typename T> +bool greater_than_struct_with_name(const StringPiece& lhs, const std::unique_ptr<T>& rhs) { + return rhs->name.compare(0, rhs->name.size(), lhs.data(), lhs.size()) > 0; } -ResourceTablePackage* ResourceTable::CreatePackage(const StringPiece& name, Maybe<uint8_t> id) { - TRACE_CALL(); - ResourceTablePackage* package = FindOrCreatePackage(name); - if (id && !package->id) { - package->id = id; - return package; +template <typename T> +struct NameEqualRange { + bool operator()(const std::unique_ptr<T>& lhs, const StringPiece& rhs) const { + return less_than_struct_with_name<T>(lhs, rhs); } + bool operator()(const StringPiece& lhs, const std::unique_ptr<T>& rhs) const { + return greater_than_struct_with_name<T>(lhs, rhs); + } +}; - if (id && package->id && package->id.value() != id.value()) { - return nullptr; +template <typename T, typename U> +bool less_than_struct_with_name_and_id(const T& lhs, + const std::pair<std::string_view, Maybe<U>>& rhs) { + if (lhs.id != rhs.second) { + return lhs.id < rhs.second; } - return package; + return lhs.name.compare(0, lhs.name.size(), rhs.first.data(), rhs.first.size()) < 0; } -ResourceTablePackage* ResourceTable::CreatePackageAllowingDuplicateNames(const StringPiece& name, - const Maybe<uint8_t> id) { - const auto last = packages.end(); - auto iter = std::lower_bound(packages.begin(), last, std::make_pair(name, id), - less_than_struct_with_name_and_id<ResourceTablePackage>); - - if (iter != last && name == (*iter)->name && id == (*iter)->id) { - return iter->get(); - } +template <typename T, typename U> +bool less_than_struct_with_name_and_id_pointer(const T* lhs, + const std::pair<std::string_view, Maybe<U>>& rhs) { + return less_than_struct_with_name_and_id(*lhs, rhs); +} - std::unique_ptr<ResourceTablePackage> new_package = util::make_unique<ResourceTablePackage>(); - new_package->name = name.to_string(); - new_package->id = id; - return packages.emplace(iter, std::move(new_package))->get(); +template <typename T, typename Func, typename Elements> +T* FindElementsRunAction(const android::StringPiece& name, Elements& entries, Func action) { + const auto iter = + std::lower_bound(entries.begin(), entries.end(), name, less_than_struct_with_name<T>); + const bool found = iter != entries.end() && name == (*iter)->name; + return action(found, iter); } -ResourceTablePackage* ResourceTable::FindOrCreatePackage(const StringPiece& name) { - const auto last = packages.end(); - auto iter = std::lower_bound(packages.begin(), last, name, - less_than_struct_with_name<ResourceTablePackage>); - if (iter != last && name == (*iter)->name) { - return iter->get(); - } +} // namespace - std::unique_ptr<ResourceTablePackage> new_package = util::make_unique<ResourceTablePackage>(); - new_package->name = name.to_string(); - return packages.emplace(iter, std::move(new_package))->get(); +ResourceTable::ResourceTable(ResourceTable::Validation validation) : validation_(validation) { } -ResourceTableType* ResourceTablePackage::FindType(ResourceType type, const Maybe<uint8_t> id) { - const auto last = types.end(); - auto iter = std::lower_bound(types.begin(), last, std::make_pair(type, id), - less_than_type_and_id); - if (iter != last && (*iter)->type == type && (!id || id == (*iter)->id)) { - return iter->get(); - } - return nullptr; +ResourceTablePackage* ResourceTable::FindPackage(const android::StringPiece& name) const { + return FindElementsRunAction<ResourceTablePackage>( + name, packages, [&](bool found, auto& iter) { return found ? iter->get() : nullptr; }); } -ResourceTableType* ResourceTablePackage::FindOrCreateType(ResourceType type, - const Maybe<uint8_t> id) { - const auto last = types.end(); - auto iter = std::lower_bound(types.begin(), last, std::make_pair(type, id), - less_than_type_and_id); - if (iter != last && (*iter)->type == type && (!id || id == (*iter)->id)) { - return iter->get(); - } +ResourceTablePackage* ResourceTable::FindOrCreatePackage(const android::StringPiece& name) { + return FindElementsRunAction<ResourceTablePackage>(name, packages, [&](bool found, auto& iter) { + return found ? iter->get() : packages.emplace(iter, new ResourceTablePackage(name))->get(); + }); +} - auto new_type = new ResourceTableType(type); - new_type->id = id; - return types.emplace(iter, std::move(new_type))->get(); +template <typename Func, typename Elements> +static ResourceTableType* FindTypeRunAction(ResourceType type, Elements& entries, Func action) { + const auto iter = std::lower_bound(entries.begin(), entries.end(), type, less_than_type); + const bool found = iter != entries.end() && type == (*iter)->type; + return action(found, iter); } -ResourceEntry* ResourceTableType::FindEntry(const StringPiece& name, const Maybe<uint16_t> id) { - const auto last = entries.end(); - auto iter = std::lower_bound(entries.begin(), last, std::make_pair(name, id), - less_than_struct_with_name_and_id<ResourceEntry>); - if (iter != last && name == (*iter)->name && (!id || id == (*iter)->id)) { - return iter->get(); - } - return nullptr; +ResourceTableType* ResourceTablePackage::FindType(ResourceType type) const { + return FindTypeRunAction(type, types, + [&](bool found, auto& iter) { return found ? iter->get() : nullptr; }); } -ResourceEntry* ResourceTableType::FindOrCreateEntry(const StringPiece& name, - const Maybe<uint16_t > id) { - auto last = entries.end(); - auto iter = std::lower_bound(entries.begin(), last, std::make_pair(name, id), - less_than_struct_with_name_and_id<ResourceEntry>); - if (iter != last && name == (*iter)->name && (!id || id == (*iter)->id)) { - return iter->get(); - } +ResourceTableType* ResourceTablePackage::FindOrCreateType(ResourceType type) { + return FindTypeRunAction(type, types, [&](bool found, auto& iter) { + return found ? iter->get() : types.emplace(iter, new ResourceTableType(type))->get(); + }); +} - auto new_entry = new ResourceEntry(name); - new_entry->id = id; - return entries.emplace(iter, std::move(new_entry))->get(); +ResourceEntry* ResourceTableType::CreateEntry(const android::StringPiece& name) { + return FindElementsRunAction<ResourceEntry>(name, entries, [&](bool found, auto& iter) { + return entries.emplace(iter, new ResourceEntry(name))->get(); + }); } -ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config) { - return FindValue(config, StringPiece()); +ResourceEntry* ResourceTableType::FindEntry(const android::StringPiece& name) const { + return FindElementsRunAction<ResourceEntry>( + name, entries, [&](bool found, auto& iter) { return found ? iter->get() : nullptr; }); +} + +ResourceEntry* ResourceTableType::FindOrCreateEntry(const android::StringPiece& name) { + return FindElementsRunAction<ResourceEntry>(name, entries, [&](bool found, auto& iter) { + return found ? iter->get() : entries.emplace(iter, new ResourceEntry(name))->get(); + }); } struct ConfigKey { @@ -188,7 +159,20 @@ bool lt_config_key_ref(const std::unique_ptr<ResourceConfigValue>& lhs, const Co } ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config, - const StringPiece& product) { + android::StringPiece product) { + auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product}, + lt_config_key_ref); + if (iter != values.end()) { + ResourceConfigValue* value = iter->get(); + if (value->config == config && StringPiece(value->product) == product) { + return value; + } + } + return nullptr; +} + +const ResourceConfigValue* ResourceEntry::FindValue(const android::ConfigDescription& config, + android::StringPiece product) const { auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product}, lt_config_key_ref); if (iter != values.end()) { @@ -323,307 +307,197 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist return CollisionResult::kConflict; } -ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /** existing **/, - Value* /** incoming **/) { - return CollisionResult::kKeepBoth; -} - -static StringPiece ResourceNameValidator(const StringPiece& name) { - if (!IsValidResourceEntryName(name)) { - return name; - } - return {}; -} - -static StringPiece SkipNameValidator(const StringPiece& /*name*/) { - return {}; -} - -bool ResourceTable::AddResource(const ResourceNameRef& name, - const ConfigDescription& config, - const StringPiece& product, - std::unique_ptr<Value> value, - IDiagnostics* diag) { - return AddResourceImpl(name, ResourceId{}, config, product, std::move(value), - (validate_resources_ ? ResourceNameValidator : SkipNameValidator), - (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag); -} +ResourceTableView ResourceTable::GetPartitionedView() const { + ResourceTableView view; + for (const auto& package : packages) { + for (const auto& type : package->types) { + for (const auto& entry : type->entries) { + std::pair<std::string_view, Maybe<uint8_t>> package_key(package->name, {}); + std::pair<std::string_view, Maybe<ResourceId>> entry_key(entry->name, {}); + std::pair<ResourceType, Maybe<uint8_t>> type_key(type->type, {}); + if (entry->id) { + // If the entry has a defined id, use the id to determine insertion position. + package_key.second = entry->id.value().package_id(); + type_key.second = entry->id.value().type_id(); + entry_key.second = entry->id.value(); + } -bool ResourceTable::AddResourceWithId(const ResourceNameRef& name, const ResourceId& res_id, - const ConfigDescription& config, const StringPiece& product, - std::unique_ptr<Value> value, IDiagnostics* diag) { - return AddResourceImpl(name, res_id, config, product, std::move(value), - (validate_resources_ ? ResourceNameValidator : SkipNameValidator), - (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag); -} + auto package_it = + std::lower_bound(view.packages.begin(), view.packages.end(), package_key, + less_than_struct_with_name_and_id<ResourceTablePackageView, uint8_t>); + if (package_it == view.packages.end() || package_it->name != package_key.first || + package_it->id != package_key.second) { + ResourceTablePackageView new_package{std::string(package_key.first), package_key.second}; + package_it = view.packages.insert(package_it, new_package); + } -bool ResourceTable::AddResourceMangled(const ResourceNameRef& name, const ConfigDescription& config, - const StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag) { - return AddResourceImpl(name, ResourceId{}, config, product, std::move(value), SkipNameValidator, - (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag); -} + auto type_it = std::lower_bound(package_it->types.begin(), package_it->types.end(), + type_key, less_than_type_and_id<ResourceTableTypeView>); + if (type_it == package_it->types.end() || type_key.first != type_it->type || + type_it->id != type_key.second) { + ResourceTableTypeView new_type{type_key.first, type_key.second}; + type_it = package_it->types.insert(type_it, new_type); + } -bool ResourceTable::AddResourceWithIdMangled(const ResourceNameRef& name, const ResourceId& id, - const ConfigDescription& config, - const StringPiece& product, - std::unique_ptr<Value> value, IDiagnostics* diag) { - return AddResourceImpl(name, id, config, product, std::move(value), SkipNameValidator, - (validate_resources_ ? ResolveValueCollision : IgnoreCollision), diag); -} + if (entry->visibility.level == Visibility::Level::kPublic) { + // Only mark the type visibility level as public, it doesn't care about being private. + type_it->visibility_level = Visibility::Level::kPublic; + } -bool ResourceTable::ValidateName(NameValidator name_validator, const ResourceNameRef& name, - const Source& source, IDiagnostics* diag) { - const StringPiece bad_char = name_validator(name.entry); - if (!bad_char.empty()) { - diag->Error(DiagMessage(source) << "resource '" << name << "' has invalid entry name '" - << name.entry << "'. Invalid character '" << bad_char << "'"); - return false; + auto entry_it = + std::lower_bound(type_it->entries.begin(), type_it->entries.end(), entry_key, + less_than_struct_with_name_and_id_pointer<ResourceEntry, ResourceId>); + type_it->entries.insert(entry_it, entry.get()); + } + } } - return true; + + return view; } -bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceId& res_id, - const ConfigDescription& config, const StringPiece& product, - std::unique_ptr<Value> value, NameValidator name_validator, - const CollisionResolverFunc& conflict_resolver, - IDiagnostics* diag) { - CHECK(value != nullptr); - CHECK(diag != nullptr); +bool ResourceTable::AddResource(NewResource&& res, IDiagnostics* diag) { + CHECK(diag != nullptr) << "Diagnostic pointer is null"; - const Source& source = value->GetSource(); - if (!ValidateName(name_validator, name, source, diag)) { - return false; - } - - // Check for package names appearing twice with two different package ids - ResourceTablePackage* package = FindOrCreatePackage(name.package); - if (res_id.is_valid() && package->id && package->id.value() != res_id.package_id()) { + const bool validate = validation_ == Validation::kEnabled; + const Source source = res.value ? res.value->GetSource() : Source{}; + if (validate && !res.allow_mangled && !IsValidResourceEntryName(res.name.entry)) { diag->Error(DiagMessage(source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but package '" << package->name << "' already has ID " - << StringPrintf("%02x", package->id.value())); + << "resource '" << res.name << "' has invalid entry name '" << res.name.entry); return false; } - // Whether or not to error on duplicate resources - 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(); - - ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id() - : Maybe<uint8_t>()); - - // Check for types appearing twice with two different type ids - if (check_id && type->id && type->id.value() != res_id.type_id()) { - diag->Error(DiagMessage(source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but type '" << type->type << "' already has ID " - << StringPrintf("%02x", type->id.value())); + if (res.id.has_value() && !res.id->first.is_valid()) { + diag->Error(DiagMessage(source) << "trying to add resource '" << res.name << "' with ID " + << res.id->first << " but that ID is invalid"); return false; } - ResourceEntry* entry = type->FindOrCreateEntry(name.entry, use_id ? res_id.entry_id() - : Maybe<uint16_t>()); - - // Check for entries appearing twice with two different entry ids - if (check_id && entry->id && entry->id.value() != res_id.entry_id()) { - diag->Error(DiagMessage(source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but resource already has ID " - << ResourceId(package->id.value(), type->id.value(), entry->id.value())); - return false; - } + auto package = FindOrCreatePackage(res.name.package); + auto type = package->FindOrCreateType(res.name.type); + auto entry_it = std::equal_range(type->entries.begin(), type->entries.end(), res.name.entry, + NameEqualRange<ResourceEntry>{}); + const size_t entry_count = std::distance(entry_it.first, entry_it.second); - ResourceConfigValue* config_value = entry->FindOrCreateValue(config, product); - if (!config_value->value) { - // Resource does not exist, add it now. - config_value->value = std::move(value); + ResourceEntry* entry; + if (entry_count == 0) { + // Adding a new resource + entry = type->CreateEntry(res.name.entry); + } else if (entry_count == 1) { + // Assume that the existing resource is being modified + entry = entry_it.first->get(); } else { - switch (conflict_resolver(config_value->value.get(), value.get())) { - case CollisionResult::kKeepBoth: - // Insert the value ignoring for duplicate configurations - entry->values.push_back(util::make_unique<ResourceConfigValue>(config, product)); - entry->values.back()->value = std::move(value); - break; - - case CollisionResult::kTakeNew: - // Take the incoming value. - config_value->value = std::move(value); - break; - - case CollisionResult::kConflict: - diag->Error(DiagMessage(source) << "duplicate value for resource '" << name << "' " - << "with config '" << config << "'"); - diag->Error(DiagMessage(source) << "resource previously defined here"); - return false; - - case CollisionResult::kKeepOriginal: + // Multiple resources with the same name exist in the resource table. The only way to + // distinguish between them is using resource id since each resource should have a unique id. + CHECK(res.id.has_value()) << "ambiguous modification of resource entry '" << res.name + << "' without specifying a resource id."; + entry = entry_it.first->get(); + for (auto it = entry_it.first; it != entry_it.second; ++it) { + CHECK((bool)(*it)->id) << "ambiguous modification of resource entry '" << res.name + << "' with multiple entries without resource ids"; + if ((*it)->id == res.id->first) { + entry = it->get(); break; + } } } - if (res_id.is_valid()) { - package->id = res_id.package_id(); - type->id = res_id.type_id(); - entry->id = res_id.entry_id(); - } - - return true; -} - -bool ResourceTable::GetValidateResources() { - return validate_resources_; -} - -bool ResourceTable::SetVisibility(const ResourceNameRef& name, const Visibility& visibility, - IDiagnostics* diag) { - return SetVisibilityImpl(name, visibility, {}, ResourceNameValidator, diag); -} - -bool ResourceTable::SetVisibilityWithId(const ResourceNameRef& name, const Visibility& visibility, - const ResourceId& res_id, IDiagnostics* diag) { - return SetVisibilityImpl(name, visibility, res_id, ResourceNameValidator, diag); -} - -bool ResourceTable::SetVisibilityWithIdMangled(const ResourceNameRef& name, - const Visibility& visibility, - const ResourceId& res_id, IDiagnostics* diag) { - return SetVisibilityImpl(name, visibility, res_id, SkipNameValidator, diag); -} - -bool ResourceTable::SetVisibilityImpl(const ResourceNameRef& name, const Visibility& visibility, - const ResourceId& res_id, NameValidator name_validator, - IDiagnostics* diag) { - CHECK(diag != nullptr); - - const Source& source = visibility.source; - if (!ValidateName(name_validator, name, source, diag)) { - return false; - } - - // Check for package names appearing twice with two different package ids - ResourceTablePackage* package = FindOrCreatePackage(name.package); - 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 " - << StringPrintf("%02x", package->id.value())); - return false; - } - - // Whether or not to error on duplicate resources - 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(); - - ResourceTableType* type = package->FindOrCreateType(name.type, use_id ? res_id.type_id() - : Maybe<uint8_t>()); - - // Check for types appearing twice with two different type ids - if (check_id && type->id && type->id.value() != res_id.type_id()) { - diag->Error(DiagMessage(source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but type '" << type->type << "' already has ID " - << StringPrintf("%02x", type->id.value())); - return false; - } - - ResourceEntry* entry = type->FindOrCreateEntry(name.entry, use_id ? res_id.entry_id() - : Maybe<uint16_t>()); - - // Check for entries appearing twice with two different entry ids - if (check_id && entry->id && entry->id.value() != res_id.entry_id()) { - diag->Error(DiagMessage(source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but resource already has ID " - << ResourceId(package->id.value(), type->id.value(), entry->id.value())); - return false; - } - - if (res_id.is_valid()) { - package->id = res_id.package_id(); - type->id = res_id.type_id(); - entry->id = res_id.entry_id(); + if (res.id.has_value()) { + if (entry->id && entry->id.value() != res.id->first) { + if (res.id->second != OnIdConflict::CREATE_ENTRY) { + diag->Error(DiagMessage(source) + << "trying to add resource '" << res.name << "' with ID " << res.id->first + << " but resource already has ID " << entry->id.value()); + return false; + } + entry = type->CreateEntry(res.name.entry); + } + entry->id = res.id->first; } - // Only mark the type visibility level as public, it doesn't care about being private. - if (visibility.level == Visibility::Level::kPublic) { - type->visibility_level = Visibility::Level::kPublic; - } + if (res.visibility.has_value()) { + // Only mark the type visibility level as public, it doesn't care about being private. + if (res.visibility->level == Visibility::Level::kPublic) { + type->visibility_level = Visibility::Level::kPublic; + } - if (visibility.level == Visibility::Level::kUndefined && - entry->visibility.level != Visibility::Level::kUndefined) { - // We can't undefine a symbol (remove its visibility). Ignore. - return true; + if (res.visibility->level > entry->visibility.level) { + // This symbol definition takes precedence, replace. + entry->visibility = res.visibility.value(); + } } - if (visibility.level < entry->visibility.level) { - // We can't downgrade public to private. Ignore. - return true; + if (res.overlayable.has_value()) { + if (entry->overlayable_item) { + diag->Error(DiagMessage(res.overlayable->source) + << "duplicate overlayable declaration for resource '" << res.name << "'"); + diag->Error(DiagMessage(entry->overlayable_item.value().source) + << "previous declaration here"); + return false; + } + entry->overlayable_item = res.overlayable.value(); + } + + if (res.allow_new.has_value()) { + entry->allow_new = res.allow_new.value(); + } + + if (res.value != nullptr) { + auto config_value = entry->FindOrCreateValue(res.config, res.product); + if (!config_value->value) { + // Resource does not exist, add it now. + config_value->value = std::move(res.value); + } else { + // When validation is enabled, ensure that a resource cannot have multiple values defined for + // the same configuration. + auto result = validate ? ResolveValueCollision(config_value->value.get(), res.value.get()) + : CollisionResult::kKeepBoth; + switch (result) { + case CollisionResult::kKeepBoth: + // Insert the value ignoring for duplicate configurations + entry->values.push_back(util::make_unique<ResourceConfigValue>(res.config, res.product)); + entry->values.back()->value = std::move(res.value); + break; + + case CollisionResult::kTakeNew: + // Take the incoming value. + config_value->value = std::move(res.value); + break; + + case CollisionResult::kConflict: + diag->Error(DiagMessage(source) << "duplicate value for resource '" << res.name << "' " + << "with config '" << res.config << "'"); + diag->Error(DiagMessage(source) << "resource previously defined here"); + return false; + + case CollisionResult::kKeepOriginal: + break; + } + } } - // This symbol definition takes precedence, replace. - entry->visibility = visibility; return true; } -bool ResourceTable::SetAllowNew(const ResourceNameRef& name, const AllowNew& allow_new, - IDiagnostics* diag) { - return SetAllowNewImpl(name, allow_new, ResourceNameValidator, diag); -} - -bool ResourceTable::SetAllowNewMangled(const ResourceNameRef& name, const AllowNew& allow_new, - IDiagnostics* diag) { - return SetAllowNewImpl(name, allow_new, SkipNameValidator, diag); -} - -bool ResourceTable::SetAllowNewImpl(const ResourceNameRef& name, const AllowNew& allow_new, - NameValidator name_validator, IDiagnostics* diag) { - CHECK(diag != nullptr); - - if (!ValidateName(name_validator, name, allow_new.source, diag)) { - return false; +Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name) const { + ResourceTablePackage* package = FindPackage(name.package); + if (package == nullptr) { + return {}; } - ResourceTablePackage* package = FindOrCreatePackage(name.package); - ResourceTableType* type = package->FindOrCreateType(name.type); - ResourceEntry* entry = type->FindOrCreateEntry(name.entry); - entry->allow_new = allow_new; - return true; -} - -bool ResourceTable::SetOverlayable(const ResourceNameRef& name, const OverlayableItem& overlayable, - IDiagnostics* diag) { - return SetOverlayableImpl(name, overlayable, ResourceNameValidator, diag); -} - -bool ResourceTable::SetOverlayableImpl(const ResourceNameRef& name, - const OverlayableItem& overlayable, - NameValidator name_validator, IDiagnostics *diag) { - CHECK(diag != nullptr); - - if (!ValidateName(name_validator, name, overlayable.source, diag)) { - return false; + ResourceTableType* type = package->FindType(name.type); + if (type == nullptr) { + return {}; } - ResourceTablePackage* package = FindOrCreatePackage(name.package); - ResourceTableType* type = package->FindOrCreateType(name.type); - ResourceEntry* entry = type->FindOrCreateEntry(name.entry); - - if (entry->overlayable_item) { - diag->Error(DiagMessage(overlayable.source) - << "duplicate overlayable declaration for resource '" << name << "'"); - diag->Error(DiagMessage(entry->overlayable_item.value().source) - << "previous declaration here"); - return false; + ResourceEntry* entry = type->FindEntry(name.entry); + if (entry == nullptr) { + return {}; } - - entry->overlayable_item = overlayable; - return true; + return SearchResult{package, type, entry}; } -Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name) const { +Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name, + ResourceId id) const { ResourceTablePackage* package = FindPackage(name.package); if (package == nullptr) { return {}; @@ -634,24 +508,26 @@ Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNam return {}; } - ResourceEntry* entry = type->FindEntry(name.entry); - if (entry == nullptr) { - return {}; + auto entry_it = std::equal_range(type->entries.begin(), type->entries.end(), name.entry, + NameEqualRange<ResourceEntry>{}); + for (auto it = entry_it.first; it != entry_it.second; ++it) { + if ((*it)->id == id) { + return SearchResult{package, type, it->get()}; + } } - return SearchResult{package, type, entry}; + return {}; } std::unique_ptr<ResourceTable> ResourceTable::Clone() const { std::unique_ptr<ResourceTable> new_table = util::make_unique<ResourceTable>(); for (const auto& pkg : packages) { - ResourceTablePackage* new_pkg = new_table->CreatePackage(pkg->name, pkg->id); + ResourceTablePackage* new_pkg = new_table->FindOrCreatePackage(pkg->name); for (const auto& type : pkg->types) { ResourceTableType* new_type = new_pkg->FindOrCreateType(type->type); - new_type->id = type->id; new_type->visibility_level = type->visibility_level; for (const auto& entry : type->entries) { - ResourceEntry* new_entry = new_type->FindOrCreateEntry(entry->name); + ResourceEntry* new_entry = new_type->CreateEntry(entry->name); new_entry->id = entry->id; new_entry->visibility = entry->visibility; new_entry->allow_new = entry->allow_new; @@ -668,4 +544,51 @@ std::unique_ptr<ResourceTable> ResourceTable::Clone() const { return new_table; } +NewResourceBuilder::NewResourceBuilder(const ResourceNameRef& name) { + res_.name = name.ToResourceName(); +} + +NewResourceBuilder::NewResourceBuilder(const std::string& name) { + ResourceNameRef ref; + CHECK(ResourceUtils::ParseResourceName(name, &ref)) << "invalid resource name: " << name; + res_.name = ref.ToResourceName(); +} + +NewResourceBuilder& NewResourceBuilder::SetValue(std::unique_ptr<Value> value, + android::ConfigDescription config, + std::string product) { + res_.value = std::move(value); + res_.config = std::move(config); + res_.product = std::move(product); + return *this; +} + +NewResourceBuilder& NewResourceBuilder::SetId(ResourceId id, OnIdConflict on_conflict) { + res_.id = std::make_pair(id, on_conflict); + return *this; +} + +NewResourceBuilder& NewResourceBuilder::SetVisibility(Visibility visibility) { + res_.visibility = std::move(visibility); + return *this; +} + +NewResourceBuilder& NewResourceBuilder::SetOverlayable(OverlayableItem overlayable) { + res_.overlayable = std::move(overlayable); + return *this; +} +NewResourceBuilder& NewResourceBuilder::SetAllowNew(AllowNew allow_new) { + res_.allow_new = std::move(allow_new); + return *this; +} + +NewResourceBuilder& NewResourceBuilder::SetAllowMangled(bool allow_mangled) { + res_.allow_mangled = allow_mangled; + return *this; +} + +NewResource NewResourceBuilder::Build() { + return std::move(res_); +} + } // namespace aapt diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 93a7a314d6ba..49392a5db830 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -109,7 +109,7 @@ class ResourceEntry { const std::string name; // The entry ID for this resource (the EEEE in 0xPPTTEEEE). - Maybe<uint16_t> id; + Maybe<ResourceId> id; // Whether this resource is public (and must maintain the same entry ID across builds). Visibility visibility; @@ -124,10 +124,10 @@ class ResourceEntry { explicit ResourceEntry(const android::StringPiece& name) : name(name.to_string()) {} - ResourceConfigValue* FindValue(const android::ConfigDescription& config); - ResourceConfigValue* FindValue(const android::ConfigDescription& config, - const android::StringPiece& product); + android::StringPiece product = {}); + const ResourceConfigValue* FindValue(const android::ConfigDescription& config, + android::StringPiece product = {}) const; ResourceConfigValue* FindOrCreateValue(const android::ConfigDescription& config, const android::StringPiece& product); @@ -156,9 +156,6 @@ class ResourceTableType { // The logical type of resource (string, drawable, layout, etc.). const ResourceType type; - // The type ID for this resource (the TT in 0xPPTTEEEE). - Maybe<uint8_t> id; - // Whether this type is public (and must maintain the same type ID across builds). Visibility::Level visibility_level = Visibility::Level::kUndefined; @@ -167,10 +164,9 @@ class ResourceTableType { explicit ResourceTableType(const ResourceType type) : type(type) {} - ResourceEntry* FindEntry(const android::StringPiece& name, - Maybe<uint16_t> id = Maybe<uint16_t>()); - ResourceEntry* FindOrCreateEntry(const android::StringPiece& name, - Maybe<uint16_t> id = Maybe<uint16_t>()); + ResourceEntry* CreateEntry(const android::StringPiece& name); + ResourceEntry* FindEntry(const android::StringPiece& name) const; + ResourceEntry* FindOrCreateEntry(const android::StringPiece& name); private: DISALLOW_COPY_AND_ASSIGN(ResourceTableType); @@ -180,70 +176,99 @@ class ResourceTablePackage { public: std::string name; - // The package ID (the PP in 0xPPTTEEEE). - Maybe<uint8_t> id; - std::vector<std::unique_ptr<ResourceTableType>> types; + explicit ResourceTablePackage(const android::StringPiece& name) : name(name.to_string()) { + } + ResourceTablePackage() = default; - ResourceTableType* FindType(ResourceType type, Maybe<uint8_t> id = Maybe<uint8_t>()); - ResourceTableType* FindOrCreateType(const ResourceType type, - Maybe<uint8_t> id = Maybe<uint8_t>()); + ResourceTableType* FindType(ResourceType type) const; + ResourceTableType* FindOrCreateType(ResourceType type); private: DISALLOW_COPY_AND_ASSIGN(ResourceTablePackage); }; -// The container and index for all resources defined for an app. -class ResourceTable { - public: - ResourceTable() = default; - explicit ResourceTable(bool validate_resources) : validate_resources_(validate_resources) {} +struct ResourceTableTypeView { + ResourceType type; + Maybe<uint8_t> id; + Visibility::Level visibility_level = Visibility::Level::kUndefined; - enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew }; + // Entries sorted in ascending entry id order. If ids have not been assigned, the entries are + // // sorted lexicographically. + std::vector<const ResourceEntry*> entries; +}; - using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>; +struct ResourceTablePackageView { + std::string name; + Maybe<uint8_t> id; + // Types sorted in ascending type id order. If ids have not been assigned, the types are sorted by + // their declaration order in the ResourceType enum. + std::vector<ResourceTableTypeView> types; +}; - // When a collision of resources occurs, this method decides which value to keep. - static CollisionResult ResolveValueCollision(Value* existing, Value* incoming); +struct ResourceTableView { + // Packages sorted in ascending package id order. If ids have not been assigned, the packages are + // sorted lexicographically. + std::vector<ResourceTablePackageView> packages; +}; + +enum class OnIdConflict { + // If the resource entry already exists but has a different resource id, the resource value will + // not be added to the table. + ERROR, - // When a collision of resources occurs, this method keeps both values - static CollisionResult IgnoreCollision(Value* existing, Value* incoming); + // If the resource entry already exists but has a different resource id, create a new resource + // with this resource name and id combination. + CREATE_ENTRY, +}; - bool AddResource(const ResourceNameRef& name, const android::ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag); +struct NewResource { + ResourceName name; + std::unique_ptr<Value> value; + android::ConfigDescription config; + std::string product; + std::optional<std::pair<ResourceId, OnIdConflict>> id; + std::optional<Visibility> visibility; + std::optional<OverlayableItem> overlayable; + std::optional<AllowNew> allow_new; + bool allow_mangled = false; +}; - bool AddResourceWithId(const ResourceNameRef& name, const ResourceId& res_id, - const android::ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag); +struct NewResourceBuilder { + explicit NewResourceBuilder(const ResourceNameRef& name); + explicit NewResourceBuilder(const std::string& name); + NewResourceBuilder& SetValue(std::unique_ptr<Value> value, android::ConfigDescription config = {}, + std::string product = {}); + NewResourceBuilder& SetId(ResourceId id, OnIdConflict on_conflict = OnIdConflict::ERROR); + NewResourceBuilder& SetVisibility(Visibility id); + NewResourceBuilder& SetOverlayable(OverlayableItem overlayable); + NewResourceBuilder& SetAllowNew(AllowNew allow_new); + NewResourceBuilder& SetAllowMangled(bool allow_mangled); + NewResource Build(); - // Same as AddResource, but doesn't verify the validity of the name. This is used - // when loading resources from an existing binary resource table that may have mangled names. - bool AddResourceMangled(const ResourceNameRef& name, const android::ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag); + private: + NewResource res_; +}; - bool AddResourceWithIdMangled(const ResourceNameRef& name, const ResourceId& id, - const android::ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag); +// The container and index for all resources defined for an app. +class ResourceTable { + public: + enum class Validation { + kEnabled, + kDisabled, + }; - bool GetValidateResources(); + enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew }; - bool SetVisibility(const ResourceNameRef& name, const Visibility& visibility, IDiagnostics* diag); - bool SetVisibilityWithId(const ResourceNameRef& name, const Visibility& visibility, - const ResourceId& res_id, IDiagnostics* diag); - bool SetVisibilityWithIdMangled(const ResourceNameRef& name, const Visibility& visibility, - const ResourceId& res_id, IDiagnostics* diag); + ResourceTable() = default; + explicit ResourceTable(Validation validation); - bool SetOverlayable(const ResourceNameRef& name, const OverlayableItem& overlayable, - IDiagnostics *diag); + bool AddResource(NewResource&& res, IDiagnostics* diag); - bool SetAllowNew(const ResourceNameRef& name, const AllowNew& allow_new, IDiagnostics* diag); - bool SetAllowNewMangled(const ResourceNameRef& name, const AllowNew& allow_new, - IDiagnostics* diag); + // Retrieves a sorted a view of the packages, types, and entries sorted in ascending resource id + // order. + ResourceTableView GetPartitionedView() const; struct SearchResult { ResourceTablePackage* package; @@ -252,23 +277,19 @@ class ResourceTable { }; Maybe<SearchResult> FindResource(const ResourceNameRef& name) const; + Maybe<SearchResult> FindResource(const ResourceNameRef& name, ResourceId id) const; // Returns the package struct with the given name, or nullptr if such a package does not // exist. The empty string is a valid package and typically is used to represent the // 'current' package before it is known to the ResourceTable. ResourceTablePackage* FindPackage(const android::StringPiece& name) const; - - ResourceTablePackage* FindPackageById(uint8_t id) const; - - ResourceTablePackage* CreatePackage(const android::StringPiece& name, Maybe<uint8_t> id = {}); - - // Attempts to find a package having the specified name and ID. If not found, a new package - // of the specified parameters is created and returned. - ResourceTablePackage* CreatePackageAllowingDuplicateNames(const android::StringPiece& name, - const Maybe<uint8_t> id); + ResourceTablePackage* FindOrCreatePackage(const android::StringPiece& name); std::unique_ptr<ResourceTable> Clone() const; + // When a collision of resources occurs, this method decides which value to keep. + static CollisionResult ResolveValueCollision(Value* existing, Value* incoming); + // The string pool used by this resource table. Values that reference strings must use // this pool to create their strings. // NOTE: `string_pool` must come before `packages` so that it is destroyed after. @@ -286,36 +307,9 @@ class ResourceTable { std::map<size_t, std::string> included_packages_; private: - // The function type that validates a symbol name. Returns a non-empty StringPiece representing - // the offending character (which may be more than one byte in UTF-8). Returns an empty string - // if the name was valid. - using NameValidator = android::StringPiece(const android::StringPiece&); - - ResourceTablePackage* FindOrCreatePackage(const android::StringPiece& name); - - bool ValidateName(NameValidator validator, const ResourceNameRef& name, const Source& source, - IDiagnostics* diag); - - bool AddResourceImpl(const ResourceNameRef& name, const ResourceId& res_id, - const android::ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - NameValidator name_validator, const CollisionResolverFunc& conflict_resolver, - IDiagnostics* diag); - - bool SetVisibilityImpl(const ResourceNameRef& name, const Visibility& visibility, - const ResourceId& res_id, NameValidator name_validator, - IDiagnostics* diag); - - bool SetAllowNewImpl(const ResourceNameRef& name, const AllowNew& allow_new, - NameValidator name_validator, IDiagnostics* diag); - - bool SetOverlayableImpl(const ResourceNameRef &name, const OverlayableItem& overlayable, - NameValidator name_validator, IDiagnostics *diag); - - // Controls whether the table validates resource names and prevents duplicate resource names - bool validate_resources_ = true; - DISALLOW_COPY_AND_ASSIGN(ResourceTable); + + Validation validation_ = Validation::kEnabled; }; } // namespace aapt diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp index 9271a7e6bae1..38391c99f55a 100644 --- a/tools/aapt2/ResourceTable_test.cpp +++ b/tools/aapt2/ResourceTable_test.cpp @@ -37,31 +37,32 @@ namespace aapt { TEST(ResourceTableTest, FailToAddResourceWithBadName) { ResourceTable table; - EXPECT_FALSE(table.AddResource( - test::ParseNameOrDie("android:id/hey,there"), ConfigDescription{}, "", - test::ValueBuilder<Id>().SetSource("test.xml", 21u).Build(), - test::GetDiagnostics())); + EXPECT_FALSE( + table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:id/hey,there")).Build(), + test::GetDiagnostics())); - EXPECT_FALSE(table.AddResource( - test::ParseNameOrDie("android:id/hey:there"), ConfigDescription{}, "", - test::ValueBuilder<Id>().SetSource("test.xml", 21u).Build(), - test::GetDiagnostics())); + EXPECT_FALSE( + table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:id/hey:there")).Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, AddResourceWithWeirdNameWhenAddingMangledResources) { ResourceTable table; - EXPECT_TRUE(table.AddResourceMangled( - test::ParseNameOrDie("android:id/heythere "), ConfigDescription{}, "", - test::ValueBuilder<Id>().SetSource("test.xml", 21u).Build(), test::GetDiagnostics())); + EXPECT_TRUE( + table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:id/heythere ")) + .SetAllowMangled(true) + .Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, AddOneResource) { ResourceTable table; EXPECT_TRUE(table.AddResource( - test::ParseNameOrDie("android:attr/id"), ConfigDescription{}, "", - test::ValueBuilder<Id>().SetSource("test/path/file.xml", 23u).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:attr/id")) + .SetValue(test::ValueBuilder<Id>().SetSource("test/path/file.xml", 23u).Build()) + .Build(), test::GetDiagnostics())); EXPECT_THAT(test::GetValue<Id>(&table, "android:attr/id"), NotNull()); @@ -70,32 +71,36 @@ TEST(ResourceTableTest, AddOneResource) { TEST(ResourceTableTest, AddMultipleResources) { ResourceTable table; - ConfigDescription config; ConfigDescription language_config; memcpy(language_config.language, "pl", sizeof(language_config.language)); EXPECT_TRUE(table.AddResource( - test::ParseNameOrDie("android:attr/layout_width"), config, "", - test::ValueBuilder<Id>().SetSource("test/path/file.xml", 10u).Build(), - test::GetDiagnostics())); - - EXPECT_TRUE(table.AddResource( - test::ParseNameOrDie("android:attr/id"), config, "", - test::ValueBuilder<Id>().SetSource("test/path/file.xml", 12u).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:attr/layout_width")) + .SetValue(test::ValueBuilder<Id>().SetSource("test/path/file.xml", 10u).Build()) + .Build(), test::GetDiagnostics())); EXPECT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/ok"), config, "", - test::ValueBuilder<Id>().SetSource("test/path/file.xml", 14u).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:attr/id")) + .SetValue(test::ValueBuilder<Id>().SetSource("test/path/file.xml", 12u).Build()) + .Build(), test::GetDiagnostics())); EXPECT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/ok"), language_config, "", - test::ValueBuilder<BinaryPrimitive>(android::Res_value{}) - .SetSource("test/path/file.xml", 20u) + NewResourceBuilder(test::ParseNameOrDie("android:string/ok")) + .SetValue(test::ValueBuilder<Id>().SetSource("test/path/file.xml", 14u).Build()) .Build(), test::GetDiagnostics())); + EXPECT_TRUE( + table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:string/ok")) + .SetValue(test::ValueBuilder<BinaryPrimitive>(android::Res_value{}) + .SetSource("test/path/file.xml", 20u) + .Build(), + language_config) + .Build(), + test::GetDiagnostics())); + EXPECT_THAT(test::GetValue<Id>(&table, "android:attr/layout_width"), NotNull()); EXPECT_THAT(test::GetValue<Id>(&table, "android:attr/id"), NotNull()); EXPECT_THAT(test::GetValue<Id>(&table, "android:string/ok"), NotNull()); @@ -104,17 +109,19 @@ TEST(ResourceTableTest, AddMultipleResources) { TEST(ResourceTableTest, OverrideWeakResourceValue) { ResourceTable table; - - ASSERT_TRUE(table.AddResource(test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "", - test::AttributeBuilder().SetWeak(true).Build(), + ASSERT_TRUE(table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:attr/foo")) + .SetValue(test::AttributeBuilder().SetWeak(true).Build()) + .Build(), test::GetDiagnostics())); Attribute* attr = test::GetValue<Attribute>(&table, "android:attr/foo"); ASSERT_THAT(attr, NotNull()); EXPECT_TRUE(attr->IsWeak()); - ASSERT_TRUE(table.AddResource(test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "", - util::make_unique<Attribute>(), test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:attr/foo")) + .SetValue(util::make_unique<Attribute>()) + .Build(), + test::GetDiagnostics())); attr = test::GetValue<Attribute>(&table, "android:attr/foo"); ASSERT_THAT(attr, NotNull()); @@ -130,23 +137,27 @@ TEST(ResourceTableTest, AllowCompatibleDuplicateAttributes) { Attribute attr_two(android::ResTable_map::TYPE_STRING | android::ResTable_map::TYPE_REFERENCE); attr_two.SetWeak(true); - ASSERT_TRUE(table.AddResource(name, ConfigDescription{}, "", - util::make_unique<Attribute>(attr_one), test::GetDiagnostics())); - ASSERT_TRUE(table.AddResource(name, ConfigDescription{}, "", - util::make_unique<Attribute>(attr_two), test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(name).SetValue(util::make_unique<Attribute>(attr_one)).Build(), + test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(name).SetValue(util::make_unique<Attribute>(attr_two)).Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, ProductVaryingValues) { ResourceTable table; + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(test::ParseNameOrDie("android:string/foo")) + .SetValue(util::make_unique<Id>(), test::ParseConfigOrDie("land"), "tablet") + .Build(), + test::GetDiagnostics())); - EXPECT_TRUE(table.AddResource(test::ParseNameOrDie("android:string/foo"), - test::ParseConfigOrDie("land"), "tablet", - util::make_unique<Id>(), - test::GetDiagnostics())); - EXPECT_TRUE(table.AddResource(test::ParseNameOrDie("android:string/foo"), - test::ParseConfigOrDie("land"), "phone", - util::make_unique<Id>(), - test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(test::ParseNameOrDie("android:string/foo")) + .SetValue(util::make_unique<Id>(), test::ParseConfigOrDie("land"), "phone") + .Build(), + test::GetDiagnostics())); EXPECT_THAT(test::GetValueForConfigAndProduct<Id>(&table, "android:string/foo",test::ParseConfigOrDie("land"), "tablet"), NotNull()); EXPECT_THAT(test::GetValueForConfigAndProduct<Id>(&table, "android:string/foo",test::ParseConfigOrDie("land"), "phone"), NotNull()); @@ -203,22 +214,26 @@ TEST(ResourceTableTest, SetVisibility) { Visibility visibility; visibility.level = Visibility::Level::kPrivate; visibility.comment = "private"; - ASSERT_TRUE(table.SetVisibility(name, visibility, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetVisibility(visibility).Build(), + test::GetDiagnostics())); ASSERT_TRUE(VisibilityOfResource(table, name, Level::kPrivate, "private")); visibility.level = Visibility::Level::kUndefined; visibility.comment = "undefined"; - ASSERT_TRUE(table.SetVisibility(name, visibility, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetVisibility(visibility).Build(), + test::GetDiagnostics())); ASSERT_TRUE(VisibilityOfResource(table, name, Level::kPrivate, "private")); visibility.level = Visibility::Level::kPublic; visibility.comment = "public"; - ASSERT_TRUE(table.SetVisibility(name, visibility, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetVisibility(visibility).Build(), + test::GetDiagnostics())); ASSERT_TRUE(VisibilityOfResource(table, name, Level::kPublic, "public")); visibility.level = Visibility::Level::kPrivate; visibility.comment = "private"; - ASSERT_TRUE(table.SetVisibility(name, visibility, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetVisibility(visibility).Build(), + test::GetDiagnostics())); ASSERT_TRUE(VisibilityOfResource(table, name, Level::kPublic, "public")); } @@ -230,14 +245,16 @@ TEST(ResourceTableTest, SetAllowNew) { Maybe<ResourceTable::SearchResult> result; allow_new.comment = "first"; - ASSERT_TRUE(table.SetAllowNew(name, allow_new, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetAllowNew(allow_new).Build(), + test::GetDiagnostics())); result = table.FindResource(name); ASSERT_TRUE(result); ASSERT_TRUE(result.value().entry->allow_new); ASSERT_THAT(result.value().entry->allow_new.value().comment, StrEq("first")); allow_new.comment = "second"; - ASSERT_TRUE(table.SetAllowNew(name, allow_new, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetAllowNew(allow_new).Build(), + test::GetDiagnostics())); result = table.FindResource(name); ASSERT_TRUE(result); ASSERT_TRUE(result.value().entry->allow_new); @@ -255,7 +272,8 @@ TEST(ResourceTableTest, SetOverlayable) { overlayable_item.source = Source("res/values/overlayable.xml", 42); const ResourceName name = test::ParseNameOrDie("android:string/foo"); - ASSERT_TRUE(table.SetOverlayable(name, overlayable_item, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetOverlayable(overlayable_item).Build(), + test::GetDiagnostics())); Maybe<ResourceTable::SearchResult> search_result = table.FindResource(name); ASSERT_TRUE(search_result); @@ -280,17 +298,20 @@ TEST(ResourceTableTest, SetMultipleOverlayableResources) { auto group = std::make_shared<Overlayable>("Name", "overlay://theme"); OverlayableItem overlayable(group); overlayable.policies = PolicyFlags::PRODUCT_PARTITION; - ASSERT_TRUE(table.SetOverlayable(foo, overlayable, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(foo).SetOverlayable(overlayable).Build(), + test::GetDiagnostics())); const ResourceName bar = test::ParseNameOrDie("android:string/bar"); OverlayableItem overlayable2(group); overlayable2.policies = PolicyFlags::PRODUCT_PARTITION; - ASSERT_TRUE(table.SetOverlayable(bar, overlayable2, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(bar).SetOverlayable(overlayable2).Build(), + test::GetDiagnostics())); const ResourceName baz = test::ParseNameOrDie("android:string/baz"); OverlayableItem overlayable3(group); overlayable3.policies = PolicyFlags::VENDOR_PARTITION; - ASSERT_TRUE(table.SetOverlayable(baz, overlayable3, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(baz).SetOverlayable(overlayable3).Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, SetOverlayableDifferentResourcesDifferentName) { @@ -299,12 +320,14 @@ TEST(ResourceTableTest, SetOverlayableDifferentResourcesDifferentName) { const ResourceName foo = test::ParseNameOrDie("android:string/foo"); OverlayableItem overlayable_item(std::make_shared<Overlayable>("Name", "overlay://theme")); overlayable_item.policies = PolicyFlags::PRODUCT_PARTITION; - ASSERT_TRUE(table.SetOverlayable(foo, overlayable_item, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(foo).SetOverlayable(overlayable_item).Build(), + test::GetDiagnostics())); const ResourceName bar = test::ParseNameOrDie("android:string/bar"); OverlayableItem overlayable_item2(std::make_shared<Overlayable>("Name2", "overlay://theme")); overlayable_item2.policies = PolicyFlags::PRODUCT_PARTITION; - ASSERT_TRUE(table.SetOverlayable(bar, overlayable_item2, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(bar).SetOverlayable(overlayable_item2).Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, SetOverlayableSameResourcesFail) { @@ -313,10 +336,12 @@ TEST(ResourceTableTest, SetOverlayableSameResourcesFail) { auto overlayable = std::make_shared<Overlayable>("Name", "overlay://theme"); OverlayableItem overlayable_item(overlayable); - ASSERT_TRUE(table.SetOverlayable(name, overlayable_item, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetOverlayable(overlayable_item).Build(), + test::GetDiagnostics())); OverlayableItem overlayable_item2(overlayable); - ASSERT_FALSE(table.SetOverlayable(name, overlayable_item2, test::GetDiagnostics())); + ASSERT_FALSE(table.AddResource(NewResourceBuilder(name).SetOverlayable(overlayable_item2).Build(), + test::GetDiagnostics())); } TEST(ResourceTableTest, SetOverlayableSameResourcesDifferentNameFail) { @@ -325,51 +350,38 @@ TEST(ResourceTableTest, SetOverlayableSameResourcesDifferentNameFail) { auto overlayable = std::make_shared<Overlayable>("Name", "overlay://theme"); OverlayableItem overlayable_item(overlayable); - ASSERT_TRUE(table.SetOverlayable(name, overlayable_item, test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetOverlayable(overlayable_item).Build(), + test::GetDiagnostics())); auto overlayable2 = std::make_shared<Overlayable>("Other", "overlay://theme"); OverlayableItem overlayable_item2(overlayable2); - ASSERT_FALSE(table.SetOverlayable(name, overlayable_item2, test::GetDiagnostics())); + ASSERT_FALSE(table.AddResource(NewResourceBuilder(name).SetOverlayable(overlayable_item2).Build(), + test::GetDiagnostics())); } -TEST(ResourceTableTest, AllowDuplictaeResourcesNames) { - ResourceTable table(/* validate_resources */ false); - - const ResourceName foo_name = test::ParseNameOrDie("android:bool/foo"); - ASSERT_TRUE(table.AddResourceWithId(foo_name, ResourceId(0x7f0100ff), ConfigDescription{} , "", - test::BuildPrimitive(android::Res_value::TYPE_INT_BOOLEAN, 0), - test::GetDiagnostics())); - ASSERT_TRUE(table.AddResourceWithId(foo_name, ResourceId(0x7f010100), ConfigDescription{} , "", - test::BuildPrimitive(android::Res_value::TYPE_INT_BOOLEAN, 1), - test::GetDiagnostics())); - - ASSERT_TRUE(table.SetVisibilityWithId(foo_name, Visibility{Visibility::Level::kPublic}, - ResourceId(0x7f0100ff), test::GetDiagnostics())); - ASSERT_TRUE(table.SetVisibilityWithId(foo_name, Visibility{Visibility::Level::kPrivate}, - ResourceId(0x7f010100), test::GetDiagnostics())); - - auto package = table.FindPackageById(0x7f); - ASSERT_THAT(package, NotNull()); - auto type = package->FindType(ResourceType::kBool); - ASSERT_THAT(type, NotNull()); - - auto entry1 = type->FindEntry("foo", 0x00ff); - ASSERT_THAT(entry1, NotNull()); - ASSERT_THAT(entry1->id, Eq(0x00ff)); - ASSERT_THAT(entry1->values[0], NotNull()); - ASSERT_THAT(entry1->values[0]->value, NotNull()); - ASSERT_THAT(ValueCast<BinaryPrimitive>(entry1->values[0]->value.get()), NotNull()); - ASSERT_THAT(ValueCast<BinaryPrimitive>(entry1->values[0]->value.get())->value.data, Eq(0u)); - ASSERT_THAT(entry1->visibility.level, Visibility::Level::kPublic); - - auto entry2 = type->FindEntry("foo", 0x0100); - ASSERT_THAT(entry2, NotNull()); - ASSERT_THAT(entry2->id, Eq(0x0100)); - ASSERT_THAT(entry2->values[0], NotNull()); - ASSERT_THAT(entry1->values[0]->value, NotNull()); - ASSERT_THAT(ValueCast<BinaryPrimitive>(entry2->values[0]->value.get()), NotNull()); - ASSERT_THAT(ValueCast<BinaryPrimitive>(entry2->values[0]->value.get())->value.data, Eq(1u)); - ASSERT_THAT(entry2->visibility.level, Visibility::Level::kPrivate); +TEST(ResourceTableTest, ConflictingIds) { + ResourceTable table; + const ResourceName name = test::ParseNameOrDie("android:string/foo"); + ASSERT_TRUE(table.AddResource(NewResourceBuilder(name).SetId(0x01010000).Build(), + test::GetDiagnostics())); + ASSERT_FALSE(table.AddResource(NewResourceBuilder(name).SetId(0x01010001).Build(), + test::GetDiagnostics())); +} + +TEST(ResourceTableTest, ConflictingIdsCreateEntry) { + ResourceTable table; + const ResourceName name = test::ParseNameOrDie("android:string/foo"); + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(name).SetId(0x01010000, OnIdConflict::CREATE_ENTRY).Build(), + test::GetDiagnostics())); + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(name).SetId(0x01010001, OnIdConflict::CREATE_ENTRY).Build(), + test::GetDiagnostics())); + + // Non-ambiguous query + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(name).SetId(0x01010001).SetValue(std::make_unique<Id>()).Build(), + test::GetDiagnostics())); } } // namespace aapt diff --git a/tools/aapt2/cmd/Compile.cpp b/tools/aapt2/cmd/Compile.cpp index ff54fccda767..cd5015e81203 100644 --- a/tools/aapt2/cmd/Compile.cpp +++ b/tools/aapt2/cmd/Compile.cpp @@ -190,17 +190,6 @@ static bool CompileTable(IAaptContext* context, const CompileOptions& options, } } - // Ensure we have the compilation package at least. - table.CreatePackage(context->GetCompilationPackage()); - - // Assign an ID to any package that has resources. - for (auto& pkg : table.packages) { - if (!pkg->id) { - // If no package ID was set while parsing (public identifiers), auto assign an ID. - pkg->id = context->GetPackageId(); - } - } - // Create the file/zip entry. if (!writer->StartEntry(output_path, 0)) { context->GetDiagnostics()->Error(DiagMessage(output_path) << "failed to open"); diff --git a/tools/aapt2/cmd/Compile_test.cpp b/tools/aapt2/cmd/Compile_test.cpp index 8cbd998f2ae2..89757134f3a7 100644 --- a/tools/aapt2/cmd/Compile_test.cpp +++ b/tools/aapt2/cmd/Compile_test.cpp @@ -217,6 +217,7 @@ static void AssertTranslations(CommandTestFixture *ctf, std::string file_name, }, compiled_files_dir, &diag)); std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + ASSERT_NE(apk, nullptr); ResourceTable* table = apk->GetResourceTable(); ASSERT_NE(table, nullptr); diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp index d56994e3ae24..0bb044e8c6cb 100644 --- a/tools/aapt2/cmd/Diff.cpp +++ b/tools/aapt2/cmd/Diff.cpp @@ -95,17 +95,17 @@ static bool IsIdDiff(const Visibility::Level& level_a, const Maybe<Id>& id_a, return false; } -static bool EmitResourceConfigValueDiff(IAaptContext* context, LoadedApk* apk_a, - ResourceTablePackage* pkg_a, ResourceTableType* type_a, - ResourceEntry* entry_a, ResourceConfigValue* config_value_a, - LoadedApk* apk_b, ResourceTablePackage* pkg_b, - ResourceTableType* type_b, ResourceEntry* entry_b, - ResourceConfigValue* config_value_b) { +static bool EmitResourceConfigValueDiff( + IAaptContext* context, LoadedApk* apk_a, const ResourceTablePackageView& pkg_a, + const ResourceTableTypeView& type_a, const ResourceEntry* entry_a, + const ResourceConfigValue* config_value_a, LoadedApk* apk_b, + const ResourceTablePackageView& pkg_b, const ResourceTableTypeView& type_b, + const ResourceEntry* entry_b, const ResourceConfigValue* config_value_b) { Value* value_a = config_value_a->value.get(); Value* value_b = config_value_b->value.get(); if (!value_a->Equals(value_b)) { std::stringstream str_stream; - str_stream << "value " << pkg_a->name << ":" << type_a->type << "/" << entry_a->name + str_stream << "value " << pkg_a.name << ":" << type_a.type << "/" << entry_a->name << " config=" << config_value_a->config << " does not match:\n"; value_a->Print(&str_stream); str_stream << "\n vs \n"; @@ -117,16 +117,17 @@ static bool EmitResourceConfigValueDiff(IAaptContext* context, LoadedApk* apk_a, } static bool EmitResourceEntryDiff(IAaptContext* context, LoadedApk* apk_a, - ResourceTablePackage* pkg_a, ResourceTableType* type_a, - ResourceEntry* entry_a, LoadedApk* apk_b, - ResourceTablePackage* pkg_b, ResourceTableType* type_b, - ResourceEntry* entry_b) { + const ResourceTablePackageView& pkg_a, + const ResourceTableTypeView& type_a, const ResourceEntry* entry_a, + LoadedApk* apk_b, const ResourceTablePackageView& pkg_b, + const ResourceTableTypeView& type_b, + const ResourceEntry* entry_b) { bool diff = false; - for (std::unique_ptr<ResourceConfigValue>& config_value_a : entry_a->values) { - ResourceConfigValue* config_value_b = entry_b->FindValue(config_value_a->config); + for (const std::unique_ptr<ResourceConfigValue>& config_value_a : entry_a->values) { + auto config_value_b = entry_b->FindValue(config_value_a->config); if (!config_value_b) { std::stringstream str_stream; - str_stream << "missing " << pkg_a->name << ":" << type_a->type << "/" << entry_a->name + str_stream << "missing " << pkg_a.name << ":" << type_a.type << "/" << entry_a->name << " config=" << config_value_a->config; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; @@ -138,35 +139,47 @@ static bool EmitResourceEntryDiff(IAaptContext* context, LoadedApk* apk_a, } // Check for any newly added config values. - for (std::unique_ptr<ResourceConfigValue>& config_value_b : entry_b->values) { - ResourceConfigValue* config_value_a = entry_a->FindValue(config_value_b->config); + for (const std::unique_ptr<ResourceConfigValue>& config_value_b : entry_b->values) { + auto config_value_a = entry_a->FindValue(config_value_b->config); if (!config_value_a) { std::stringstream str_stream; - str_stream << "new config " << pkg_b->name << ":" << type_b->type << "/" << entry_b->name + str_stream << "new config " << pkg_b.name << ":" << type_b.type << "/" << entry_b->name << " config=" << config_value_b->config; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } } - return false; + return diff; } static bool EmitResourceTypeDiff(IAaptContext* context, LoadedApk* apk_a, - ResourceTablePackage* pkg_a, ResourceTableType* type_a, - LoadedApk* apk_b, ResourceTablePackage* pkg_b, - ResourceTableType* type_b) { + const ResourceTablePackageView& pkg_a, + const ResourceTableTypeView& type_a, LoadedApk* apk_b, + const ResourceTablePackageView& pkg_b, + const ResourceTableTypeView& type_b) { bool diff = false; - for (std::unique_ptr<ResourceEntry>& entry_a : type_a->entries) { - ResourceEntry* entry_b = type_b->FindEntry(entry_a->name); - if (!entry_b) { + auto entry_a_iter = type_a.entries.begin(); + auto entry_b_iter = type_b.entries.begin(); + while (entry_a_iter != type_a.entries.end() || entry_b_iter != type_b.entries.end()) { + if (entry_b_iter == type_b.entries.end()) { + // Type A contains a type that type B does not have. std::stringstream str_stream; - str_stream << "missing " << pkg_a->name << ":" << type_a->type << "/" << entry_a->name; + str_stream << "missing " << pkg_a.name << ":" << type_a.type << "/" << (*entry_a_iter)->name; + EmitDiffLine(apk_a->GetSource(), str_stream.str()); + diff = true; + } else if (entry_a_iter == type_a.entries.end()) { + // Type B contains a type that type A does not have. + std::stringstream str_stream; + str_stream << "new entry " << pkg_b.name << ":" << type_b.type << "/" + << (*entry_b_iter)->name; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } else { + const auto& entry_a = *entry_a_iter; + const auto& entry_b = *entry_a_iter; if (IsSymbolVisibilityDifferent(entry_a->visibility, entry_b->visibility)) { std::stringstream str_stream; - str_stream << pkg_a->name << ":" << type_a->type << "/" << entry_a->name + str_stream << pkg_a.name << ":" << type_a.type << "/" << entry_a->name << " has different visibility ("; if (entry_b->visibility.level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; @@ -185,7 +198,7 @@ static bool EmitResourceTypeDiff(IAaptContext* context, LoadedApk* apk_a, } else if (IsIdDiff(entry_a->visibility.level, entry_a->id, entry_b->visibility.level, entry_b->id)) { std::stringstream str_stream; - str_stream << pkg_a->name << ":" << type_a->type << "/" << entry_a->name + str_stream << pkg_a.name << ":" << type_a.type << "/" << entry_a->name << " has different public ID ("; if (entry_b->id) { str_stream << "0x" << std::hex << entry_b->id.value(); @@ -202,46 +215,43 @@ static bool EmitResourceTypeDiff(IAaptContext* context, LoadedApk* apk_a, EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } - diff |= EmitResourceEntryDiff(context, apk_a, pkg_a, type_a, entry_a.get(), apk_b, pkg_b, - type_b, entry_b); - } - } - - // Check for any newly added entries. - for (std::unique_ptr<ResourceEntry>& entry_b : type_b->entries) { - ResourceEntry* entry_a = type_a->FindEntry(entry_b->name); - if (!entry_a) { - std::stringstream str_stream; - str_stream << "new entry " << pkg_b->name << ":" << type_b->type << "/" << entry_b->name; - EmitDiffLine(apk_b->GetSource(), str_stream.str()); - diff = true; + diff |= EmitResourceEntryDiff(context, apk_a, pkg_a, type_a, entry_a, apk_b, pkg_b, type_b, + entry_b); } } return diff; } static bool EmitResourcePackageDiff(IAaptContext* context, LoadedApk* apk_a, - ResourceTablePackage* pkg_a, LoadedApk* apk_b, - ResourceTablePackage* pkg_b) { + const ResourceTablePackageView& pkg_a, LoadedApk* apk_b, + const ResourceTablePackageView& pkg_b) { bool diff = false; - for (std::unique_ptr<ResourceTableType>& type_a : pkg_a->types) { - ResourceTableType* type_b = pkg_b->FindType(type_a->type); - if (!type_b) { + auto type_a_iter = pkg_a.types.begin(); + auto type_b_iter = pkg_b.types.begin(); + while (type_a_iter != pkg_a.types.end() || type_b_iter != pkg_b.types.end()) { + if (type_b_iter == pkg_b.types.end()) { + // Type A contains a type that type B does not have. std::stringstream str_stream; - str_stream << "missing " << pkg_a->name << ":" << type_a->type; + str_stream << "missing " << pkg_a.name << ":" << type_a_iter->type; EmitDiffLine(apk_a->GetSource(), str_stream.str()); diff = true; + } else if (type_a_iter == pkg_a.types.end()) { + // Type B contains a type that type A does not have. + std::stringstream str_stream; + str_stream << "new type " << pkg_b.name << ":" << type_b_iter->type; + EmitDiffLine(apk_b->GetSource(), str_stream.str()); + diff = true; } else { - if (type_a->visibility_level != type_b->visibility_level) { + if (type_a_iter->visibility_level != type_b_iter->visibility_level) { std::stringstream str_stream; - str_stream << pkg_a->name << ":" << type_a->type << " has different visibility ("; - if (type_b->visibility_level == Visibility::Level::kPublic) { + str_stream << pkg_a.name << ":" << type_a_iter->type << " has different visibility ("; + if (type_b_iter->visibility_level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; } str_stream << " vs "; - if (type_a->visibility_level == Visibility::Level::kPublic) { + if (type_a_iter->visibility_level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; @@ -249,18 +259,18 @@ static bool EmitResourcePackageDiff(IAaptContext* context, LoadedApk* apk_a, str_stream << ")"; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; - } else if (IsIdDiff(type_a->visibility_level, type_a->id, type_b->visibility_level, - type_b->id)) { + } else if (IsIdDiff(type_a_iter->visibility_level, type_a_iter->id, + type_b_iter->visibility_level, type_b_iter->id)) { std::stringstream str_stream; - str_stream << pkg_a->name << ":" << type_a->type << " has different public ID ("; - if (type_b->id) { - str_stream << "0x" << std::hex << type_b->id.value(); + str_stream << pkg_a.name << ":" << type_a_iter->type << " has different public ID ("; + if (type_b_iter->id) { + str_stream << "0x" << std::hex << type_b_iter->id.value(); } else { str_stream << "none"; } str_stream << " vs "; - if (type_a->id) { - str_stream << "0x " << std::hex << type_a->id.value(); + if (type_a_iter->id) { + str_stream << "0x " << std::hex << type_a_iter->id.value(); } else { str_stream << "none"; } @@ -268,47 +278,44 @@ static bool EmitResourcePackageDiff(IAaptContext* context, LoadedApk* apk_a, EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } - diff |= EmitResourceTypeDiff(context, apk_a, pkg_a, type_a.get(), apk_b, pkg_b, type_b); - } - } - - // Check for any newly added types. - for (std::unique_ptr<ResourceTableType>& type_b : pkg_b->types) { - ResourceTableType* type_a = pkg_a->FindType(type_b->type); - if (!type_a) { - std::stringstream str_stream; - str_stream << "new type " << pkg_b->name << ":" << type_b->type; - EmitDiffLine(apk_b->GetSource(), str_stream.str()); - diff = true; + diff |= EmitResourceTypeDiff(context, apk_a, pkg_a, *type_a_iter, apk_b, pkg_b, *type_b_iter); } } return diff; } static bool EmitResourceTableDiff(IAaptContext* context, LoadedApk* apk_a, LoadedApk* apk_b) { - ResourceTable* table_a = apk_a->GetResourceTable(); - ResourceTable* table_b = apk_b->GetResourceTable(); + const auto table_a = apk_a->GetResourceTable()->GetPartitionedView(); + const auto table_b = apk_b->GetResourceTable()->GetPartitionedView(); bool diff = false; - for (std::unique_ptr<ResourceTablePackage>& pkg_a : table_a->packages) { - ResourceTablePackage* pkg_b = table_b->FindPackage(pkg_a->name); - if (!pkg_b) { + auto package_a_iter = table_a.packages.begin(); + auto package_b_iter = table_b.packages.begin(); + while (package_a_iter != table_a.packages.end() || package_b_iter != table_b.packages.end()) { + if (package_b_iter == table_b.packages.end()) { + // Table A contains a package that table B does not have. + std::stringstream str_stream; + str_stream << "missing package " << package_a_iter->name; + EmitDiffLine(apk_a->GetSource(), str_stream.str()); + diff = true; + } else if (package_a_iter == table_a.packages.end()) { + // Table B contains a package that table A does not have. std::stringstream str_stream; - str_stream << "missing package " << pkg_a->name; + str_stream << "new package " << package_b_iter->name; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } else { - if (pkg_a->id != pkg_b->id) { + if (package_a_iter->id != package_b_iter->id) { std::stringstream str_stream; - str_stream << "package '" << pkg_a->name << "' has different id ("; - if (pkg_b->id) { - str_stream << "0x" << std::hex << pkg_b->id.value(); + str_stream << "package '" << package_a_iter->name << "' has different id ("; + if (package_b_iter->id) { + str_stream << "0x" << std::hex << package_b_iter->id.value(); } else { str_stream << "none"; } str_stream << " vs "; - if (pkg_a->id) { - str_stream << "0x" << std::hex << pkg_a->id.value(); + if (package_a_iter->id) { + str_stream << "0x" << std::hex << package_b_iter->id.value(); } else { str_stream << "none"; } @@ -316,20 +323,10 @@ static bool EmitResourceTableDiff(IAaptContext* context, LoadedApk* apk_a, Loade EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } - diff |= EmitResourcePackageDiff(context, apk_a, pkg_a.get(), apk_b, pkg_b); + diff |= EmitResourcePackageDiff(context, apk_a, *package_a_iter, apk_b, *package_b_iter); } } - // Check for any newly added packages. - for (std::unique_ptr<ResourceTablePackage>& pkg_b : table_b->packages) { - ResourceTablePackage* pkg_a = table_a->FindPackage(pkg_b->name); - if (!pkg_a) { - std::stringstream str_stream; - str_stream << "new package " << pkg_b->name; - EmitDiffLine(apk_b->GetSource(), str_stream.str()); - diff = true; - } - } return diff; } diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 13e090d9d843..ee6a764966f0 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -25,6 +25,7 @@ #include <vector> #include "android-base/errors.h" +#include "android-base/expected.h" #include "android-base/file.h" #include "android-base/stringprintf.h" #include "androidfw/Locale.h" @@ -74,10 +75,25 @@ using ::aapt::io::FileInputStream; using ::android::ConfigDescription; using ::android::StringPiece; +using ::android::base::expected; using ::android::base::StringPrintf; +using ::android::base::unexpected; namespace aapt { +namespace { + +expected<ResourceTablePackage*, const char*> GetStaticLibraryPackage(ResourceTable* table) { + // Resource tables built by aapt2 always contain one package. This is a post condition of + // VerifyNoExternalPackages. + if (table->packages.size() != 1u) { + return unexpected("static library contains more than one package"); + } + return table->packages.back().get(); +} + +} // namespace + constexpr uint8_t kAndroidPackageId = 0x01; class LinkContext : public IAaptContext { @@ -633,13 +649,18 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv const ResourceFile& file = doc->file; dst_path = ResourceUtils::BuildResourceFileName(file, context_->GetNameMangler()); - std::unique_ptr<FileReference> file_ref = + auto file_ref = util::make_unique<FileReference>(table->string_pool.MakeRef(dst_path)); file_ref->SetSource(doc->file.source); + // Update the output format of this XML file. file_ref->type = XmlFileTypeForOutputFormat(options_.output_format); - if (!table->AddResourceMangled(file.name, file.config, {}, std::move(file_ref), - context_->GetDiagnostics())) { + bool result = table->AddResource(NewResourceBuilder(file.name) + .SetValue(std::move(file_ref), file.config) + .SetAllowMangled(true) + .Build(), + context_->GetDiagnostics()); + if (!result) { return false; } } @@ -842,18 +863,15 @@ class Linker { ResourceTable* table = static_apk->GetResourceTable(); // If we are using --no-static-lib-packages, we need to rename the package of this table to - // our compilation package. - if (options_.no_static_lib_packages) { - // Since package names can differ, and multiple packages can exist in a ResourceTable, - // we place the requirement that all static libraries are built with the package - // ID 0x7f. So if one is not found, this is an error. - if (ResourceTablePackage* pkg = table->FindPackageById(kAppPackageId)) { - pkg->name = context_->GetCompilationPackage(); - } else { - context_->GetDiagnostics()->Error(DiagMessage(path) - << "no package with ID 0x7f found in static library"); + // our compilation package so the symbol package name does not get mangled into the entry + // name. + if (options_.no_static_lib_packages && !table->packages.empty()) { + auto lib_package_result = GetStaticLibraryPackage(table); + if (!lib_package_result.has_value()) { + context_->GetDiagnostics()->Error(DiagMessage(path) << lib_package_result.error()); return false; } + lib_package_result.value()->name = context_->GetCompilationPackage(); } context_->GetExternalSymbols()->AppendSource( @@ -982,8 +1000,7 @@ class Linker { // stripped, or there is an error and false is returned. bool VerifyNoExternalPackages() { auto is_ext_package_func = [&](const std::unique_ptr<ResourceTablePackage>& pkg) -> bool { - return context_->GetCompilationPackage() != pkg->name || !pkg->id || - pkg->id.value() != context_->GetPackageId(); + return context_->GetCompilationPackage() != pkg->name; }; bool error = false; @@ -1027,19 +1044,11 @@ class Linker { bool VerifyNoIdsSet() { for (const auto& package : final_table_.packages) { for (const auto& type : package->types) { - if (type->id) { - context_->GetDiagnostics()->Error(DiagMessage() << "type " << type->type << " has ID " - << StringPrintf("%02x", type->id.value()) - << " assigned"); - return false; - } - for (const auto& entry : type->entries) { if (entry->id) { ResourceNameRef res_name(package->name, type->type, entry->name); - context_->GetDiagnostics()->Error( - DiagMessage() << "entry " << res_name << " has ID " - << StringPrintf("%02x", entry->id.value()) << " assigned"); + context_->GetDiagnostics()->Error(DiagMessage() << "resource " << res_name << " has ID " + << entry->id.value() << " assigned"); return false; } } @@ -1313,12 +1322,17 @@ class Linker { } ResourceTable* table = apk->GetResourceTable(); - ResourceTablePackage* pkg = table->FindPackageById(kAppPackageId); - if (!pkg) { - context_->GetDiagnostics()->Error(DiagMessage(input) << "static library has no package"); + if (table->packages.empty()) { + return true; + } + + auto lib_package_result = GetStaticLibraryPackage(table); + if (!lib_package_result.has_value()) { + context_->GetDiagnostics()->Error(DiagMessage(input) << lib_package_result.error()); return false; } + ResourceTablePackage* pkg = lib_package_result.value(); bool result; if (options_.no_static_lib_packages) { // Merge all resources as if they were in the compilation package. This is the old behavior @@ -1365,11 +1379,11 @@ class Linker { res_name = mangled_name.value(); } - std::unique_ptr<Id> id = util::make_unique<Id>(); + auto id = util::make_unique<Id>(); id->SetSource(source.WithLine(exported_symbol.line)); - bool result = - final_table_.AddResourceMangled(res_name, ConfigDescription::DefaultConfig(), - std::string(), std::move(id), context_->GetDiagnostics()); + bool result = final_table_.AddResource( + NewResourceBuilder(res_name).SetValue(std::move(id)).SetAllowMangled(true).Build(), + context_->GetDiagnostics()); if (!result) { return false; } @@ -1750,7 +1764,7 @@ class Linker { // anything else being generated, which includes the Java classes. // If required, the package name is modifed before flattening, and then modified back // to its original name. - ResourceTablePackage* package_to_rewrite = nullptr; + std::optional<ResourceTablePackageView> package_to_rewrite; // Pre-O, the platform treats negative resource IDs [those with a package ID of 0x80 // or higher] as invalid. In order to work around this limitation, we allow the use // of traditionally reserved resource IDs [those between 0x02 and 0x7E]. Allow the @@ -1764,10 +1778,11 @@ class Linker { // The base APK is included, and this is a feature split. If the base package is // the same as this package, then we are building an old style Android Instant Apps feature // split and must apply this workaround to avoid requiring namespaces support. - package_to_rewrite = table->FindPackage(context_->GetCompilationPackage()); - if (package_to_rewrite != nullptr) { + auto table_view = table->GetPartitionedView(); + if (!table_view.packages.empty() && + table_view.packages.back().name == context_->GetCompilationPackage()) { + package_to_rewrite = std::move(table_view.packages.back()); CHECK_EQ(1u, table->packages.size()) << "can't change name of package when > 1 package"; - std::string new_package_name = StringPrintf("%s.%s", package_to_rewrite->name.c_str(), app_info_.split_name.value_or_default("feature").c_str()); @@ -1783,7 +1798,7 @@ class Linker { bool success = FlattenTable(table, options_.output_format, writer); - if (package_to_rewrite != nullptr) { + if (package_to_rewrite.has_value()) { // Change the name back. package_to_rewrite->name = context_->GetCompilationPackage(); if (package_to_rewrite->id) { @@ -1925,8 +1940,7 @@ class Linker { for (auto& entry : type->entries) { ResourceName name(package->name, type->type, entry->name); // The IDs are guaranteed to exist. - options_.stable_id_map[std::move(name)] = - ResourceId(package->id.value(), type->id.value(), entry->id.value()); + options_.stable_id_map[std::move(name)] = entry->id.value(); } } } diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp index 73072a963d09..27cbe88fde38 100644 --- a/tools/aapt2/cmd/Link_test.cpp +++ b/tools/aapt2/cmd/Link_test.cpp @@ -47,6 +47,8 @@ TEST_F(LinkTest, RemoveRawXmlStrings) { // Load the binary xml tree android::ResXMLTree tree; std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + ASSERT_THAT(apk, Ne(nullptr)); + std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml"); ASSERT_THAT(data, Ne(nullptr)); AssertLoadXml(apk.get(), data.get(), &tree); @@ -73,6 +75,8 @@ TEST_F(LinkTest, KeepRawXmlStrings) { // Load the binary xml tree android::ResXMLTree tree; std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + ASSERT_THAT(apk, Ne(nullptr)); + std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml"); ASSERT_THAT(data, Ne(nullptr)); AssertLoadXml(apk.get(), data.get(), &tree); @@ -208,6 +212,8 @@ TEST_F(LinkTest, OverlayStyles) { ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag)); std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + ASSERT_THAT(apk, Ne(nullptr)); + const Style* actual_style = test::GetValue<Style>( apk->GetResourceTable(), std::string(kDefaultPackageName) + ":style/MyStyle"); ASSERT_NE(actual_style, nullptr); @@ -250,6 +256,8 @@ TEST_F(LinkTest, OverrideStylesInsteadOfOverlaying) { ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag)); std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag); + ASSERT_THAT(apk, Ne(nullptr)); + const Style* actual_style = test::GetValue<Style>( apk->GetResourceTable(), std::string(kDefaultPackageName) + ":style/MyStyle"); ASSERT_NE(actual_style, nullptr); diff --git a/tools/aapt2/compile/IdAssigner.cpp b/tools/aapt2/compile/IdAssigner.cpp index 17c22c5aac6b..07db73dff2c5 100644 --- a/tools/aapt2/compile/IdAssigner.cpp +++ b/tools/aapt2/compile/IdAssigner.cpp @@ -17,76 +17,109 @@ #include "compile/IdAssigner.h" #include <map> +#include <unordered_map> +#include "android-base/expected.h" #include "android-base/logging.h" #include "ResourceTable.h" #include "process/IResourceTableConsumer.h" #include "util/Util.h" +using android::base::expected; +using android::base::unexpected; + namespace aapt { -/** - * Assigns the intended ID to the ResourceTablePackage, ResourceTableType, and - * ResourceEntry, - * as long as there is no existing ID or the ID is the same. - */ -static bool AssignId(IDiagnostics* diag, const ResourceId& id, - const ResourceName& name, ResourceTablePackage* pkg, - ResourceTableType* type, ResourceEntry* entry) { - if (pkg->id.value() == id.package_id()) { - if (!type->id || type->id.value() == id.type_id()) { - type->id = id.type_id(); - - if (!entry->id || entry->id.value() == id.entry_id()) { - entry->id = id.entry_id(); - return true; - } - } +namespace { +template <typename T> +using Result = expected<T, std::string>; + +template <typename Id, typename Key> +struct NextIdFinder { + explicit NextIdFinder(Id start_id = 0u) : next_id_(start_id){}; + + // Attempts to reserve an identifier for the specified key. + // If the identifier is already reserved by a different key, an error message is returned. + // Reserving identifiers must be completed before `NextId` is called for the first time. + Result<Id> ReserveId(Key key, Id id); + + // Retrieves the next available identifier that has not been reserved. + std::optional<Id> NextId(); + + private: + // Attempts to set `next_id_` to the next available identifier that has not been reserved. + // Returns whether there were any available identifiers. + std::optional<Id> SkipToNextAvailableId(); + + Id next_id_; + bool next_id_called_ = false; + bool exhausted_ = false; + std::map<Id, Key> pre_assigned_ids_; + typename std::map<Id, Key>::iterator next_preassigned_id_; +}; + +struct TypeGroup { + explicit TypeGroup(uint8_t package_id, uint8_t type_id) + : package_id_(package_id), type_id_(type_id){}; + + // Attempts to reserve the resource id for the specified resource name. + // If the id is already reserved by a different name, an error message is returned. + // Reserving identifiers must be completed before `NextId` is called for the first time. + Result<std::monostate> ReserveId(const ResourceName& name, ResourceId id); + + // Retrieves the next available resource id that has not been reserved. + Result<ResourceId> NextId(); + + private: + uint8_t package_id_; + uint8_t type_id_; + NextIdFinder<uint16_t, ResourceName> next_entry_id_; +}; + +struct IdAssignerContext { + IdAssignerContext(std::string package_name, uint8_t package_id) + : package_name_(std::move(package_name)), package_id_(package_id) { } - const ResourceId existing_id(pkg->id.value(), type->id ? type->id.value() : 0, - entry->id ? entry->id.value() : 0); - diag->Error(DiagMessage() << "can't assign ID " << id << " to resource " - << name << " with conflicting ID " << existing_id); - return false; -} + // Attempts to reserve the resource id for the specified resource name. + // Returns whether the id was reserved successfully. + // Reserving identifiers must be completed before `NextId` is called for the first time. + bool ReserveId(const ResourceName& name, ResourceId id, IDiagnostics* diag); -bool IdAssigner::Consume(IAaptContext* context, ResourceTable* table) { - std::map<ResourceId, ResourceName> assigned_ids; + // Retrieves the next available resource id that has not been reserved. + std::optional<ResourceId> NextId(const ResourceName& name, IDiagnostics* diag); - for (auto& package : table->packages) { - CHECK(bool(package->id)) << "packages must have manually assigned IDs"; + private: + std::string package_name_; + uint8_t package_id_; + std::map<ResourceType, TypeGroup> types_; + NextIdFinder<uint8_t, ResourceType> type_id_finder_ = NextIdFinder<uint8_t, ResourceType>(1); +}; +} // namespace + +bool IdAssigner::Consume(IAaptContext* context, ResourceTable* table) { + IdAssignerContext assigned_ids(context->GetCompilationPackage(), context->GetPackageId()); + for (auto& package : table->packages) { for (auto& type : package->types) { for (auto& entry : type->entries) { const ResourceName name(package->name, type->type, entry->name); + if (entry->id) { + if (!assigned_ids.ReserveId(name, entry->id.value(), context->GetDiagnostics())) { + return false; + } + } if (assigned_id_map_) { // Assign the pre-assigned stable ID meant for this resource. const auto iter = assigned_id_map_->find(name); if (iter != assigned_id_map_->end()) { const ResourceId assigned_id = iter->second; - const bool result = - AssignId(context->GetDiagnostics(), assigned_id, name, - package.get(), type.get(), entry.get()); - if (!result) { + if (!assigned_ids.ReserveId(name, assigned_id, context->GetDiagnostics())) { return false; } - } - } - - if (package->id && type->id && entry->id) { - // If the ID is set for this resource, then reserve it. - ResourceId resource_id(package->id.value(), type->id.value(), - entry->id.value()); - auto result = assigned_ids.insert({resource_id, name}); - const ResourceName& existing_name = result.first->second; - if (!result.second) { - context->GetDiagnostics()->Error( - DiagMessage() << "resource " << name << " has same ID " - << resource_id << " as " << existing_name); - return false; + entry->id = assigned_id; } } } @@ -94,125 +127,162 @@ bool IdAssigner::Consume(IAaptContext* context, ResourceTable* table) { } if (assigned_id_map_) { - // Reserve all the IDs mentioned in the stable ID map. That way we won't - // assign - // IDs that were listed in the map if they don't exist in the table. + // Reserve all the IDs mentioned in the stable ID map. That way we won't assig IDs that were + // listed in the map if they don't exist in the table. for (const auto& stable_id_entry : *assigned_id_map_) { const ResourceName& pre_assigned_name = stable_id_entry.first; const ResourceId& pre_assigned_id = stable_id_entry.second; - auto result = assigned_ids.insert({pre_assigned_id, pre_assigned_name}); - const ResourceName& existing_name = result.first->second; - if (!result.second && existing_name != pre_assigned_name) { - context->GetDiagnostics()->Error( - DiagMessage() << "stable ID " << pre_assigned_id << " for resource " - << pre_assigned_name - << " is already taken by resource " << existing_name); + if (!assigned_ids.ReserveId(pre_assigned_name, pre_assigned_id, context->GetDiagnostics())) { return false; } } } - // Assign any resources without IDs the next available ID. Gaps will be filled - // if possible, + // Assign any resources without IDs the next available ID. Gaps will be filled if possible, // unless those IDs have been reserved. - - const auto assigned_ids_iter_end = assigned_ids.end(); for (auto& package : table->packages) { - CHECK(bool(package->id)) << "packages must have manually assigned IDs"; - - // Build a half filled ResourceId object, which will be used to find the - // closest matching - // reserved ID in the assignedId map. From that point the next available - // type ID can be - // found. - ResourceId resource_id(package->id.value(), 0, 0); - uint8_t next_expected_type_id = 1; - - // Find the closest matching ResourceId that is <= the one with only the - // package set. - auto next_type_iter = assigned_ids.lower_bound(resource_id); for (auto& type : package->types) { - if (!type->id) { - // We need to assign a type ID. Iterate over the reserved IDs until we - // find - // some type ID that is a distance of 2 greater than the last one we've - // seen. - // That means there is an available type ID between these reserved IDs. - while (next_type_iter != assigned_ids_iter_end) { - if (next_type_iter->first.package_id() != package->id.value()) { - break; - } - - const uint8_t type_id = next_type_iter->first.type_id(); - if (type_id > next_expected_type_id) { - // There is a gap in the type IDs, so use the missing one. - type->id = next_expected_type_id++; - break; - } - - // Set our expectation to be the next type ID after the reserved one - // we - // just saw. - next_expected_type_id = type_id + 1; - - // Move to the next reserved ID. - ++next_type_iter; + for (auto& entry : type->entries) { + const ResourceName name(package->name, type->type, entry->name); + if (entry->id) { + continue; } - - if (!type->id) { - // We must have hit the end of the reserved IDs and not found a gap. - // That means the next ID is available. - type->id = next_expected_type_id++; + auto id = assigned_ids.NextId(name, context->GetDiagnostics()); + if (!id.has_value()) { + return false; } + entry->id = id.value(); } + } + } + return true; +} - resource_id = ResourceId(package->id.value(), type->id.value(), 0); - uint16_t next_expected_entry_id = 0; +namespace { +template <typename Id, typename Key> +Result<Id> NextIdFinder<Id, Key>::ReserveId(Key key, Id id) { + CHECK(!next_id_called_) << "ReserveId cannot be called after NextId"; + auto assign_result = pre_assigned_ids_.emplace(id, key); + if (!assign_result.second && assign_result.first->second != key) { + std::stringstream error; + error << "ID " << id << " is already assigned to " << assign_result.first->second; + return unexpected(error.str()); + } + return id; +} - // Find the closest matching ResourceId that is <= the one with only the - // package - // and type set. - auto next_entry_iter = assigned_ids.lower_bound(resource_id); - for (auto& entry : type->entries) { - if (!entry->id) { - // We need to assign an entry ID. Iterate over the reserved IDs until - // we find - // some entry ID that is a distance of 2 greater than the last one - // we've seen. - // That means there is an available entry ID between these reserved - // IDs. - while (next_entry_iter != assigned_ids_iter_end) { - if (next_entry_iter->first.package_id() != package->id.value() || - next_entry_iter->first.type_id() != type->id.value()) { - break; - } +template <typename Id, typename Key> +std::optional<Id> NextIdFinder<Id, Key>::NextId() { + if (!next_id_called_) { + next_id_called_ = true; + next_preassigned_id_ = pre_assigned_ids_.begin(); + } + return SkipToNextAvailableId(); +} - const uint16_t entry_id = next_entry_iter->first.entry_id(); - if (entry_id > next_expected_entry_id) { - // There is a gap in the entry IDs, so use the missing one. - entry->id = next_expected_entry_id++; - break; - } +template <typename Id, typename Key> +std::optional<Id> NextIdFinder<Id, Key>::SkipToNextAvailableId() { + if (exhausted_) { + return {}; + } + while (next_preassigned_id_ != pre_assigned_ids_.end()) { + if (next_preassigned_id_->first == next_id_) { + if (next_id_ == std::numeric_limits<Id>::max()) { + // The last identifier was reserved so there are no more available identifiers. + exhausted_ = true; + return {}; + } + ++next_id_; + ++next_preassigned_id_; + continue; + } + CHECK(next_preassigned_id_->first > next_id_) << "Preassigned IDs are not in sorted order"; + break; + } + if (next_id_ == std::numeric_limits<Id>::max()) { + // There are no more identifiers after this one, but this one is still available so return it. + exhausted_ = true; + } + return next_id_++; +} - // Set our expectation to be the next type ID after the reserved one - // we - // just saw. - next_expected_entry_id = entry_id + 1; +Result<std::monostate> TypeGroup::ReserveId(const ResourceName& name, ResourceId id) { + if (type_id_ != id.type_id()) { + // Currently there cannot be multiple type ids for a single type. + std::stringstream error; + error << "type '" << name.type << "' already has ID " << id.type_id(); + return unexpected(error.str()); + } - // Move to the next reserved entry ID. - ++next_entry_iter; - } + auto assign_result = next_entry_id_.ReserveId(name, id.entry_id()); + if (!assign_result.has_value()) { + std::stringstream error; + error << "entry " << assign_result.error(); + return unexpected(error.str()); + } + return {}; +} - if (!entry->id) { - // We must have hit the end of the reserved IDs and not found a gap. - // That means the next ID is available. - entry->id = next_expected_entry_id++; - } - } - } +Result<ResourceId> TypeGroup::NextId() { + auto entry_id = next_entry_id_.NextId(); + if (!entry_id.has_value()) { + std::stringstream error; + error << "resource type ID has exceeded the maximum number of resource entries (" + << (std::numeric_limits<uint16_t>::max() + 1u) << ")"; + return unexpected(error.str()); + } + return ResourceId(package_id_, type_id_, entry_id.value()); +} + +bool IdAssignerContext::ReserveId(const ResourceName& name, ResourceId id, IDiagnostics* diag) { + if (package_id_ != id.package_id()) { + diag->Error(DiagMessage() << "can't assign ID " << id << " to resource " << name + << " because package already has ID " << id.package_id()); + return false; + } + + auto type = types_.find(name.type); + if (type == types_.end()) { + // The type has not been assigned an id yet. Ensure that the specified id is not being used by + // another type. + auto assign_result = type_id_finder_.ReserveId(name.type, id.type_id()); + if (!assign_result.has_value()) { + diag->Error(DiagMessage() << "can't assign ID " << id << " to resource " << name + << " because type " << assign_result.error()); + return false; } + type = types_.emplace(name.type, TypeGroup(package_id_, id.type_id())).first; + } + + auto assign_result = type->second.ReserveId(name, id); + if (!assign_result.has_value()) { + diag->Error(DiagMessage() << "can't assign ID " << id << " to resource " << name << " because " + << assign_result.error()); + return false; } + return true; } +std::optional<ResourceId> IdAssignerContext::NextId(const ResourceName& name, IDiagnostics* diag) { + // The package name is not known during the compile stage. + // Resources without a package name are considered a part of the app being linked. + CHECK(name.package.empty() || name.package == package_name_); + auto type = types_.find(name.type); + if (type == types_.end()) { + auto next_type_id = type_id_finder_.NextId(); + CHECK(next_type_id.has_value()) << "resource type IDs allocated have exceeded maximum (256)"; + type = types_.emplace(name.type, TypeGroup(package_id_, next_type_id.value())).first; + } + + auto assign_result = type->second.NextId(); + if (!assign_result.has_value()) { + diag->Error(DiagMessage() << "can't assign resource ID to resource " << name << " because " + << assign_result.error()); + return {}; + } + return assign_result.value(); +} +} // namespace + } // namespace aapt diff --git a/tools/aapt2/compile/IdAssigner_test.cpp b/tools/aapt2/compile/IdAssigner_test.cpp index 5cff0048c062..0065a224bfa7 100644 --- a/tools/aapt2/compile/IdAssigner_test.cpp +++ b/tools/aapt2/compile/IdAssigner_test.cpp @@ -20,42 +20,40 @@ namespace aapt { -::testing::AssertionResult VerifyIds(ResourceTable* table); +struct IdAssignerTests : public ::testing::Test { + void SetUp() override { + context = test::ContextBuilder().SetCompilationPackage("android").SetPackageId(0x01).Build(); + } + std::unique_ptr<IAaptContext> context; +}; -TEST(IdAssignerTest, AssignIds) { - std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .AddSimple("android:attr/foo") - .AddSimple("android:attr/bar") - .AddSimple("android:id/foo") - .SetPackageId("android", 0x01) - .Build(); +::testing::AssertionResult VerifyIds(ResourceTable* table); - std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); +TEST_F(IdAssignerTests, AssignIds) { + auto table = test::ResourceTableBuilder() + .AddSimple("android:attr/foo") + .AddSimple("android:attr/bar") + .AddSimple("android:id/foo") + .Build(); IdAssigner assigner; ASSERT_TRUE(assigner.Consume(context.get(), table.get())); ASSERT_TRUE(VerifyIds(table.get())); } -TEST(IdAssignerTest, AssignIdsWithReservedIds) { - std::unique_ptr<ResourceTable> table = - test::ResourceTableBuilder() - .AddSimple("android:id/foo", ResourceId(0x01010000)) - .AddSimple("android:dimen/two") - .AddSimple("android:integer/three") - .AddSimple("android:string/five") - .AddSimple("android:attr/fun", ResourceId(0x01040000)) - .AddSimple("android:attr/foo", ResourceId(0x01040006)) - .AddSimple("android:attr/bar") - .AddSimple("android:attr/baz") - .AddSimple("app:id/biz") - .SetPackageId("android", 0x01) - .SetPackageId("app", 0x7f) - .Build(); - - std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); - IdAssigner assigner; +TEST_F(IdAssignerTests, AssignIdsWithReservedIds) { + auto table = test::ResourceTableBuilder() + .AddSimple("android:id/foo", ResourceId(0x01010000)) + .AddSimple("android:dimen/two") + .AddSimple("android:integer/three") + .AddSimple("android:string/five") + .AddSimple("android:attr/fun", ResourceId(0x01040000)) + .AddSimple("android:attr/foo", ResourceId(0x01040006)) + .AddSimple("android:attr/bar") + .AddSimple("android:attr/baz") + .Build(); + IdAssigner assigner; ASSERT_TRUE(assigner.Consume(context.get(), table.get())); ASSERT_TRUE(VerifyIds(table.get())); @@ -65,12 +63,12 @@ TEST(IdAssignerTest, AssignIdsWithReservedIds) { maybe_result = table->FindResource(test::ParseNameOrDie("android:dimen/two")); ASSERT_TRUE(maybe_result); - EXPECT_EQ(make_value<uint8_t>(2), maybe_result.value().type->id); + EXPECT_EQ(make_value<ResourceId>(0x01020000), maybe_result.value().entry->id); maybe_result = table->FindResource(test::ParseNameOrDie("android:integer/three")); ASSERT_TRUE(maybe_result); - EXPECT_EQ(make_value<uint8_t>(3), maybe_result.value().type->id); + EXPECT_EQ(make_value<ResourceId>(0x01030000), maybe_result.value().entry->id); // Expect to bypass the reserved 0x0104XXXX IDs and use the next 0x0105XXXX // IDs. @@ -78,103 +76,126 @@ TEST(IdAssignerTest, AssignIdsWithReservedIds) { maybe_result = table->FindResource(test::ParseNameOrDie("android:string/five")); ASSERT_TRUE(maybe_result); - EXPECT_EQ(make_value<uint8_t>(5), maybe_result.value().type->id); + EXPECT_EQ(make_value<ResourceId>(0x01050000), maybe_result.value().entry->id); // Expect to fill in the gaps between 0x01040000 and 0x01040006. maybe_result = table->FindResource(test::ParseNameOrDie("android:attr/bar")); ASSERT_TRUE(maybe_result); - EXPECT_EQ(make_value<uint16_t>(1), maybe_result.value().entry->id); + EXPECT_EQ(make_value<ResourceId>(0x01040001), maybe_result.value().entry->id); maybe_result = table->FindResource(test::ParseNameOrDie("android:attr/baz")); ASSERT_TRUE(maybe_result); - EXPECT_EQ(make_value<uint16_t>(2), maybe_result.value().entry->id); + EXPECT_EQ(make_value<ResourceId>(0x01040002), maybe_result.value().entry->id); } -TEST(IdAssignerTest, FailWhenNonUniqueIdsAssigned) { - std::unique_ptr<ResourceTable> table = - test::ResourceTableBuilder() - .AddSimple("android:attr/foo", ResourceId(0x01040006)) - .AddSimple("android:attr/bar", ResourceId(0x01040006)) - .SetPackageId("android", 0x01) - .SetPackageId("app", 0x7f) - .Build(); - - std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); +TEST_F(IdAssignerTests, FailWhenNonUniqueIdsAssigned) { + auto table = test::ResourceTableBuilder() + .AddSimple("android:attr/foo", ResourceId(0x01040006)) + .AddSimple("android:attr/bar", ResourceId(0x01040006)) + .Build(); IdAssigner assigner; - ASSERT_FALSE(assigner.Consume(context.get(), table.get())); } -TEST(IdAssignerTest, AssignIdsWithIdMap) { - std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .AddSimple("android:attr/foo") - .AddSimple("android:attr/bar") - .SetPackageId("android", 0x01) - .Build(); - - std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); +TEST_F(IdAssignerTests, AssignIdsWithIdMap) { + auto table = test::ResourceTableBuilder() + .AddSimple("android:attr/foo") + .AddSimple("android:attr/bar") + .Build(); std::unordered_map<ResourceName, ResourceId> id_map = { {test::ParseNameOrDie("android:attr/foo"), ResourceId(0x01010002)}}; IdAssigner assigner(&id_map); ASSERT_TRUE(assigner.Consume(context.get(), table.get())); ASSERT_TRUE(VerifyIds(table.get())); - Maybe<ResourceTable::SearchResult> result = - table->FindResource(test::ParseNameOrDie("android:attr/foo")); + auto result = table->FindResource(test::ParseNameOrDie("android:attr/foo")); ASSERT_TRUE(result); const ResourceTable::SearchResult& search_result = result.value(); - EXPECT_EQ(make_value<uint8_t>(0x01), search_result.package->id); - EXPECT_EQ(make_value<uint8_t>(0x01), search_result.type->id); - EXPECT_EQ(make_value<uint16_t>(0x0002), search_result.entry->id); + EXPECT_EQ(make_value<ResourceId>(0x01010002), search_result.entry->id); +} + +TEST_F(IdAssignerTests, UseAllEntryIds) { + ResourceTable table; + const size_t max_entry_id = std::numeric_limits<uint16_t>::max(); + for (size_t i = 0; i <= max_entry_id; i++) { + ASSERT_TRUE( + table.AddResource(NewResourceBuilder("android:attr/res" + std::to_string(i)).Build(), + context->GetDiagnostics())); + } + IdAssigner assigner; + ASSERT_TRUE(assigner.Consume(context.get(), &table)); +} + +TEST_F(IdAssignerTests, ExaustEntryIds) { + ResourceTable table; + const size_t max_entry_id = std::numeric_limits<uint16_t>::max() + 1u; + for (size_t i = 0; i <= max_entry_id; i++) { + ASSERT_TRUE( + table.AddResource(NewResourceBuilder("android:attr/res" + std::to_string(i)).Build(), + context->GetDiagnostics())); + } + IdAssigner assigner; + ASSERT_FALSE(assigner.Consume(context.get(), &table)); +} + +TEST_F(IdAssignerTests, ExaustEntryIdsLastIdIsPublic) { + ResourceTable table; + ASSERT_TRUE(table.AddResource(NewResourceBuilder("android:attr/res").SetId(0x0101ffff).Build(), + context->GetDiagnostics())); + const size_t max_entry_id = std::numeric_limits<uint16_t>::max(); + for (size_t i = 0; i <= max_entry_id; i++) { + ASSERT_TRUE( + table.AddResource(NewResourceBuilder("android:attr/res" + std::to_string(i)).Build(), + context->GetDiagnostics())); + } + IdAssigner assigner; + ASSERT_FALSE(assigner.Consume(context.get(), &table)); } ::testing::AssertionResult VerifyIds(ResourceTable* table) { std::set<uint8_t> package_ids; - for (auto& package : table->packages) { - if (!package->id) { - return ::testing::AssertionFailure() << "package " << package->name - << " has no ID"; + auto table_view = table->GetPartitionedView(); + for (auto& package : table_view.packages) { + if (!package.id) { + return ::testing::AssertionFailure() << "package " << package.name << " has no ID"; } - if (!package_ids.insert(package->id.value()).second) { - return ::testing::AssertionFailure() - << "package " << package->name << " has non-unique ID " << std::hex - << (int)package->id.value() << std::dec; + if (!package_ids.insert(package.id.value()).second) { + return ::testing::AssertionFailure() << "package " << package.name << " has non-unique ID " + << std::hex << (int)package.id.value() << std::dec; } } - for (auto& package : table->packages) { + for (auto& package : table_view.packages) { std::set<uint8_t> type_ids; - for (auto& type : package->types) { - if (!type->id) { - return ::testing::AssertionFailure() << "type " << type->type - << " of package " << package->name - << " has no ID"; + for (auto& type : package.types) { + if (!type.id) { + return ::testing::AssertionFailure() + << "type " << type.type << " of package " << package.name << " has no ID"; } - if (!type_ids.insert(type->id.value()).second) { + if (!type_ids.insert(type.id.value()).second) { return ::testing::AssertionFailure() - << "type " << type->type << " of package " << package->name - << " has non-unique ID " << std::hex << (int)type->id.value() - << std::dec; + << "type " << type.type << " of package " << package.name << " has non-unique ID " + << std::hex << (int)type.id.value() << std::dec; } } - for (auto& type : package->types) { - std::set<uint16_t> entry_ids; - for (auto& entry : type->entries) { + for (auto& type : package.types) { + std::set<ResourceId> entry_ids; + for (auto& entry : type.entries) { if (!entry->id) { return ::testing::AssertionFailure() - << "entry " << entry->name << " of type " << type->type - << " of package " << package->name << " has no ID"; + << "entry " << entry->name << " of type " << type.type << " of package " + << package.name << " has no ID"; } if (!entry_ids.insert(entry->id.value()).second) { return ::testing::AssertionFailure() - << "entry " << entry->name << " of type " << type->type - << " of package " << package->name << " has non-unique ID " - << std::hex << (int)entry->id.value() << std::dec; + << "entry " << entry->name << " of type " << type.type << " of package " + << package.name << " has non-unique ID " << std::hex << entry->id.value() + << std::dec; } } } diff --git a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp index e816c86e20a8..432d7bfdad49 100644 --- a/tools/aapt2/compile/PseudolocaleGenerator_test.cpp +++ b/tools/aapt2/compile/PseudolocaleGenerator_test.cpp @@ -236,13 +236,14 @@ TEST(PseudolocaleGeneratorTest, PseudolocalizeOnlyDefaultConfigs) { TEST(PseudolocaleGeneratorTest, PluralsArePseudolocalized) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); - std::unique_ptr<ResourceTable> table = - test::ResourceTableBuilder().SetPackageId("com.pkg", 0x7F).Build(); + std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder().Build(); std::unique_ptr<Plural> plural = util::make_unique<Plural>(); plural->values = {util::make_unique<String>(table->string_pool.MakeRef("zero")), util::make_unique<String>(table->string_pool.MakeRef("one"))}; - ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("com.pkg:plurals/foo"), ConfigDescription{}, - {}, std::move(plural), context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.pkg:plurals/foo")) + .SetValue(std::move(plural)) + .Build(), + context->GetDiagnostics())); std::unique_ptr<Plural> expected = util::make_unique<Plural>(); expected->values = {util::make_unique<String>(table->string_pool.MakeRef("[žéŕö one]")), util::make_unique<String>(table->string_pool.MakeRef("[öñé one]"))}; @@ -252,6 +253,7 @@ TEST(PseudolocaleGeneratorTest, PluralsArePseudolocalized) { const auto* actual = test::GetValueForConfig<Plural>(table.get(), "com.pkg:plurals/foo", test::ParseConfigOrDie("en-rXA")); + ASSERT_NE(nullptr, actual); EXPECT_TRUE(actual->Equals(expected.get())); } @@ -273,11 +275,14 @@ TEST(PseudolocaleGeneratorTest, RespectUntranslateableSections) { auto string = util::make_unique<String>(table->string_pool.MakeRef(original_style.str)); string->untranslatable_sections.push_back(UntranslatableSection{6u, 11u}); - ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/foo"), ConfigDescription{}, - {} /* product */, std::move(styled_string), + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("android:string/foo")) + .SetValue(std::move(styled_string)) + .Build(), + context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("android:string/bar")) + .SetValue(std::move(string)) + .Build(), context->GetDiagnostics())); - ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("android:string/bar"), ConfigDescription{}, - {} /* product */, std::move(string), context->GetDiagnostics())); } PseudolocaleGenerator generator; diff --git a/tools/aapt2/dump/DumpManifest.cpp b/tools/aapt2/dump/DumpManifest.cpp index cc1cf7f23246..f29c9185cafa 100644 --- a/tools/aapt2/dump/DumpManifest.cpp +++ b/tools/aapt2/dump/DumpManifest.cpp @@ -191,18 +191,14 @@ class ManifestExtractor { const ConfigDescription& config = DefaultConfig()) { if (table) { for (auto& package : table->packages) { - if (package->id && package->id.value() == res_id.package_id()) { for (auto& type : package->types) { - if (type->id && type->id.value() == res_id.type_id()) { - for (auto& entry : type->entries) { - if (entry->id && entry->id.value() == res_id.entry_id()) { - if (auto value = BestConfigValue(entry.get(), config)) { - return value; - } + for (auto& entry : type->entries) { + if (entry->id && entry->id.value() == res_id.id) { + if (auto value = BestConfigValue(entry.get(), config)) { + return value; } } } - } } } } diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index cccd9faa9b39..bfb8d5854d6d 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -192,8 +192,7 @@ bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) { std::u16string package_name = strcpy16_dtoh((const char16_t*)package_header->name, arraysize(package_header->name)); - ResourceTablePackage* package = - table_->CreatePackage(util::Utf16ToUtf8(package_name), static_cast<uint8_t>(package_id)); + ResourceTablePackage* package = table_->FindOrCreatePackage(util::Utf16ToUtf8(package_name)); if (!package) { diag_->Error(DiagMessage(source_) << "incompatible package '" << package_name << "' with ID " << package_id); @@ -232,13 +231,13 @@ bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) { break; case android::RES_TABLE_TYPE_SPEC_TYPE: - if (!ParseTypeSpec(package, parser.chunk())) { + if (!ParseTypeSpec(package, parser.chunk(), package_id)) { return false; } break; case android::RES_TABLE_TYPE_TYPE: - if (!ParseType(package, parser.chunk())) { + if (!ParseType(package, parser.chunk(), package_id)) { return false; } break; @@ -276,7 +275,7 @@ bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) { } bool BinaryResourceParser::ParseTypeSpec(const ResourceTablePackage* package, - const ResChunk_header* chunk) { + const ResChunk_header* chunk, uint8_t package_id) { if (type_pool_.getError() != NO_ERROR) { diag_->Error(DiagMessage(source_) << "missing type string pool"); return false; @@ -317,14 +316,14 @@ bool BinaryResourceParser::ParseTypeSpec(const ResourceTablePackage* package, const uint32_t* type_spec_flags = reinterpret_cast<const uint32_t*>( reinterpret_cast<uintptr_t>(type_spec) + util::DeviceToHost16(type_spec->header.headerSize)); for (size_t i = 0; i < entry_count; i++) { - ResourceId id(package->id.value_or_default(0x0), type_spec->id, static_cast<size_t>(i)); + ResourceId id(package_id, type_spec->id, static_cast<size_t>(i)); entry_type_spec_flags_[id] = util::DeviceToHost32(type_spec_flags[i]); } return true; } bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, - const ResChunk_header* chunk) { + const ResChunk_header* chunk, uint8_t package_id) { if (type_pool_.getError() != NO_ERROR) { diag_->Error(DiagMessage(source_) << "missing type string pool"); return false; @@ -354,13 +353,9 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, const std::string type_str = util::GetString(type_pool_, type->id - 1); const ResourceType* parsed_type = ParseResourceType(type_str); if (!parsed_type) { - // Be lenient on the name of the type if the table is lenient on resource validation. - bool log_error = table_->GetValidateResources(); - if (log_error) { - diag_->Error(DiagMessage(source_) << "invalid type name '" << type_str - << "' for type with ID " << type->id); - } - return !log_error; + diag_->Warn(DiagMessage(source_) + << "invalid type name '" << type_str << "' for type with ID " << type->id); + return true; } TypeVariant tv(type); @@ -372,7 +367,7 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, const ResourceName name(package->name, *parsed_type, util::GetString(key_pool_, util::DeviceToHost32(entry->key.index))); - const ResourceId res_id(package->id.value(), type->id, static_cast<uint16_t>(it.index())); + const ResourceId res_id(package_id, type->id, static_cast<uint16_t>(it.index())); std::unique_ptr<Value> resource_value; if (entry->flags & ResTable_entry::FLAG_COMPLEX) { @@ -392,17 +387,13 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, return false; } - if (!table_->AddResourceWithIdMangled(name, res_id, config, {}, std::move(resource_value), - diag_)) { - return false; - } + NewResourceBuilder res_builder(name); + res_builder.SetValue(std::move(resource_value), config) + .SetId(res_id, OnIdConflict::CREATE_ENTRY) + .SetAllowMangled(true); if (entry->flags & ResTable_entry::FLAG_PUBLIC) { - Visibility visibility; - visibility.level = Visibility::Level::kPublic; - if (!table_->SetVisibilityWithIdMangled(name, visibility, res_id, diag_)) { - return false; - } + res_builder.SetVisibility(Visibility{Visibility::Level::kPublic}); // Erase the ID from the map once processed, so that we don't mark the same symbol more than // once. @@ -415,6 +406,10 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, if (cache_iter == id_index_.end()) { id_index_.insert({res_id, name}); } + + if (!table_->AddResource(res_builder.Build(), diag_)) { + return false; + } } return true; } @@ -472,7 +467,12 @@ bool BinaryResourceParser::ParseOverlayable(const ResChunk_header* chunk) { OverlayableItem overlayable_item(overlayable); overlayable_item.policies = policy_header->policy_flags; - if (!table_->SetOverlayable(iter->second, overlayable_item, diag_)) { + if (!table_->AddResource(NewResourceBuilder(iter->second) + .SetId(res_id, OnIdConflict::CREATE_ENTRY) + .SetOverlayable(std::move(overlayable_item)) + .SetAllowMangled(true) + .Build(), + diag_)) { return false; } } diff --git a/tools/aapt2/format/binary/BinaryResourceParser.h b/tools/aapt2/format/binary/BinaryResourceParser.h index a2eee5006964..13dd9828b911 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.h +++ b/tools/aapt2/format/binary/BinaryResourceParser.h @@ -51,8 +51,10 @@ class BinaryResourceParser { bool ParseTable(const android::ResChunk_header* chunk); bool ParsePackage(const android::ResChunk_header* chunk); - bool ParseTypeSpec(const ResourceTablePackage* package, const android::ResChunk_header* chunk); - bool ParseType(const ResourceTablePackage* package, const android::ResChunk_header* chunk); + bool ParseTypeSpec(const ResourceTablePackage* package, const android::ResChunk_header* chunk, + uint8_t package_id); + bool ParseType(const ResourceTablePackage* package, const android::ResChunk_header* chunk, + uint8_t package_id); bool ParseLibrary(const android::ResChunk_header* chunk); bool ParseOverlayable(const android::ResChunk_header* chunk); diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index 4b90b4f534cc..5fea897d46e7 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -59,35 +59,35 @@ static void strcpy16_htod(uint16_t* dst, size_t len, const StringPiece16& src) { dst[i] = 0; } -static bool cmp_style_entries(const Style::Entry& a, const Style::Entry& b) { - if (a.key.id) { - if (b.key.id) { - return cmp_ids_dynamic_after_framework(a.key.id.value(), b.key.id.value()); +static bool cmp_style_entries(const Style::Entry* a, const Style::Entry* b) { + if (a->key.id) { + if (b->key.id) { + return cmp_ids_dynamic_after_framework(a->key.id.value(), b->key.id.value()); } return true; - } else if (!b.key.id) { - return a.key.name.value() < b.key.name.value(); + } else if (!b->key.id) { + return a->key.name.value() < b->key.name.value(); } return false; } struct FlatEntry { - ResourceEntry* entry; - Value* value; + const ResourceEntry* entry; + const Value* value; // The entry string pool index to the entry's name. uint32_t entry_key; }; -class MapFlattenVisitor : public ValueVisitor { +class MapFlattenVisitor : public ConstValueVisitor { public: - using ValueVisitor::Visit; + using ConstValueVisitor::Visit; MapFlattenVisitor(ResTable_entry_ext* out_entry, BigBuffer* buffer) : out_entry_(out_entry), buffer_(buffer) { } - void Visit(Attribute* attr) override { + void Visit(const Attribute* attr) override { { Reference key = Reference(ResourceId(ResTable_map::ATTR_TYPE)); BinaryPrimitive val(Res_value::TYPE_INT_DEC, attr->type_mask); @@ -106,13 +106,13 @@ class MapFlattenVisitor : public ValueVisitor { FlattenEntry(&key, &val); } - for (Attribute::Symbol& s : attr->symbols) { + for (const Attribute::Symbol& s : attr->symbols) { BinaryPrimitive val(s.type, s.value); FlattenEntry(&s.symbol, &val); } } - void Visit(Style* style) override { + void Visit(const Style* style) override { if (style->parent) { const Reference& parent_ref = style->parent.value(); CHECK(bool(parent_ref.id)) << "parent has no ID"; @@ -120,21 +120,26 @@ class MapFlattenVisitor : public ValueVisitor { } // Sort the style. - std::sort(style->entries.begin(), style->entries.end(), cmp_style_entries); + std::vector<const Style::Entry*> sorted_entries; + for (const auto& entry : style->entries) { + sorted_entries.emplace_back(&entry); + } + + std::sort(sorted_entries.begin(), sorted_entries.end(), cmp_style_entries); - for (Style::Entry& entry : style->entries) { - FlattenEntry(&entry.key, entry.value.get()); + for (const Style::Entry* entry : sorted_entries) { + FlattenEntry(&entry->key, entry->value.get()); } } - void Visit(Styleable* styleable) override { + void Visit(const Styleable* styleable) override { for (auto& attr_ref : styleable->entries) { BinaryPrimitive val(Res_value{}); FlattenEntry(&attr_ref, &val); } } - void Visit(Array* array) override { + void Visit(const Array* array) override { const size_t count = array->elements.size(); for (size_t i = 0; i < count; i++) { Reference key(android::ResTable_map::ATTR_MIN + i); @@ -142,7 +147,7 @@ class MapFlattenVisitor : public ValueVisitor { } } - void Visit(Plural* plural) override { + void Visit(const Plural* plural) override { const size_t count = plural->values.size(); for (size_t i = 0; i < count; i++) { if (!plural->values[i]) { @@ -196,16 +201,16 @@ class MapFlattenVisitor : public ValueVisitor { private: DISALLOW_COPY_AND_ASSIGN(MapFlattenVisitor); - void FlattenKey(Reference* key, ResTable_map* out_entry) { + void FlattenKey(const Reference* key, ResTable_map* out_entry) { CHECK(bool(key->id)) << "key has no ID"; out_entry->name.ident = util::HostToDevice32(key->id.value().id); } - void FlattenValue(Item* value, ResTable_map* out_entry) { + void FlattenValue(const Item* value, ResTable_map* out_entry) { CHECK(value->Flatten(&out_entry->value)) << "flatten failed"; } - void FlattenEntry(Reference* key, Item* value) { + void FlattenEntry(const Reference* key, Item* value) { ResTable_map* out_entry = buffer_->NextBlock<ResTable_map>(); FlattenKey(key, out_entry); FlattenValue(value, out_entry); @@ -226,7 +231,7 @@ struct OverlayableChunk { class PackageFlattener { public: - PackageFlattener(IAaptContext* context, ResourceTablePackage* package, + PackageFlattener(IAaptContext* context, const ResourceTablePackageView& package, const std::map<size_t, std::string>* shared_libs, bool use_sparse_entries, bool collapse_key_stringpool, const std::set<ResourceName>& name_collapse_exemptions) @@ -243,20 +248,20 @@ class PackageFlattener { TRACE_CALL(); ChunkWriter pkg_writer(buffer); ResTable_package* pkg_header = pkg_writer.StartChunk<ResTable_package>(RES_TABLE_PACKAGE_TYPE); - pkg_header->id = util::HostToDevice32(package_->id.value()); + pkg_header->id = util::HostToDevice32(package_.id.value()); // AAPT truncated the package name, so do the same. // Shared libraries require full package names, so don't truncate theirs. if (context_->GetPackageType() != PackageType::kApp && - package_->name.size() >= arraysize(pkg_header->name)) { - diag_->Error(DiagMessage() << "package name '" << package_->name + package_.name.size() >= arraysize(pkg_header->name)) { + diag_->Error(DiagMessage() << "package name '" << package_.name << "' is too long. " "Shared libraries cannot have truncated package names"); return false; } // Copy the package name in device endianness. - strcpy16_htod(pkg_header->name, arraysize(pkg_header->name), util::Utf8ToUtf16(package_->name)); + strcpy16_htod(pkg_header->name, arraysize(pkg_header->name), util::Utf8ToUtf16(package_.name)); // Serialize the types. We do this now so that our type and key strings // are populated. We write those first. @@ -273,7 +278,7 @@ class PackageFlattener { buffer->AppendBuffer(std::move(type_buffer)); // If there are libraries (or if the package ID is 0x00), encode a library chunk. - if (package_->id.value() == 0x00 || !shared_libs_->empty()) { + if (package_.id.value() == 0x00 || !shared_libs_->empty()) { FlattenLibrarySpec(buffer); } @@ -315,7 +320,7 @@ class PackageFlattener { } bool FlattenValue(FlatEntry* entry, BigBuffer* buffer) { - if (Item* item = ValueCast<Item>(entry->value)) { + if (const Item* item = ValueCast<Item>(entry->value)) { WriteEntry<ResTable_entry, true>(entry, buffer); Res_value* outValue = buffer->NextBlock<Res_value>(); CHECK(item->Flatten(outValue)) << "flatten failed"; @@ -329,7 +334,7 @@ class PackageFlattener { return true; } - bool FlattenConfig(const ResourceTableType* type, const ConfigDescription& config, + bool FlattenConfig(const ResourceTableTypeView& type, const ConfigDescription& config, const size_t num_total_entries, std::vector<FlatEntry>* entries, BigBuffer* buffer) { CHECK(num_total_entries != 0); @@ -337,7 +342,7 @@ class PackageFlattener { ChunkWriter type_writer(buffer); ResTable_type* type_header = type_writer.StartChunk<ResTable_type>(RES_TABLE_TYPE_TYPE); - type_header->id = type->id.value(); + type_header->id = type.id.value(); type_header->config = config; type_header->config.swapHtoD(); @@ -346,12 +351,12 @@ class PackageFlattener { BigBuffer values_buffer(512); for (FlatEntry& flat_entry : *entries) { - CHECK(static_cast<size_t>(flat_entry.entry->id.value()) < num_total_entries); - offsets[flat_entry.entry->id.value()] = values_buffer.size(); + CHECK(static_cast<size_t>(flat_entry.entry->id.value().entry_id()) < num_total_entries); + offsets[flat_entry.entry->id.value().entry_id()] = values_buffer.size(); if (!FlattenValue(&flat_entry, &values_buffer)) { diag_->Error(DiagMessage() << "failed to flatten resource '" - << ResourceNameRef(package_->name, type->type, flat_entry.entry->name) + << ResourceNameRef(package_.name, type.type, flat_entry.entry->name) << "' for configuration '" << config << "'"); return false; } @@ -399,55 +404,27 @@ class PackageFlattener { return true; } - std::vector<ResourceTableType*> CollectAndSortTypes() { - std::vector<ResourceTableType*> sorted_types; - for (auto& type : package_->types) { - if (type->type == ResourceType::kStyleable) { - // Styleables aren't real Resource Types, they are represented in the - // R.java file. - continue; - } - - CHECK(bool(type->id)) << "type must have an ID set"; - - sorted_types.push_back(type.get()); - } - std::sort(sorted_types.begin(), sorted_types.end(), cmp_ids<ResourceTableType>); - return sorted_types; - } - - std::vector<ResourceEntry*> CollectAndSortEntries(ResourceTableType* type) { - // Sort the entries by entry ID. - std::vector<ResourceEntry*> sorted_entries; - for (auto& entry : type->entries) { - CHECK(bool(entry->id)) << "entry must have an ID set"; - sorted_entries.push_back(entry.get()); - } - std::sort(sorted_entries.begin(), sorted_entries.end(), cmp_ids<ResourceEntry>); - return sorted_entries; - } - bool FlattenOverlayable(BigBuffer* buffer) { std::set<ResourceId> seen_ids; std::map<std::string, OverlayableChunk> overlayable_chunks; - CHECK(bool(package_->id)) << "package must have an ID set when flattening <overlayable>"; - for (auto& type : package_->types) { - CHECK(bool(type->id)) << "type must have an ID set when flattening <overlayable>"; - for (auto& entry : type->entries) { - CHECK(bool(type->id)) << "entry must have an ID set when flattening <overlayable>"; + CHECK(bool(package_.id)) << "package must have an ID set when flattening <overlayable>"; + for (auto& type : package_.types) { + CHECK(bool(type.id)) << "type must have an ID set when flattening <overlayable>"; + for (auto& entry : type.entries) { + CHECK(bool(type.id)) << "entry must have an ID set when flattening <overlayable>"; if (!entry->overlayable_item) { continue; } - OverlayableItem& item = entry->overlayable_item.value(); + const OverlayableItem& item = entry->overlayable_item.value(); // Resource ids should only appear once in the resource table - ResourceId id = android::make_resid(package_->id.value(), type->id.value(), - entry->id.value()); + ResourceId id = + android::make_resid(package_.id.value(), type.id.value(), entry->id.value().entry_id()); CHECK(seen_ids.find(id) == seen_ids.end()) << "multiple overlayable definitions found for resource " - << ResourceName(package_->name, type->type, entry->name).to_string(); + << ResourceName(package_.name, type.type, entry->name).to_string(); seen_ids.insert(id); // Find the overlayable chunk with the specified name @@ -542,14 +519,14 @@ class PackageFlattener { return true; } - bool FlattenTypeSpec(ResourceTableType* type, std::vector<ResourceEntry*>* sorted_entries, - BigBuffer* buffer) { + bool FlattenTypeSpec(const ResourceTableTypeView& type, + const std::vector<const ResourceEntry*>& sorted_entries, BigBuffer* buffer) { ChunkWriter type_spec_writer(buffer); ResTable_typeSpec* spec_header = type_spec_writer.StartChunk<ResTable_typeSpec>(RES_TABLE_TYPE_SPEC_TYPE); - spec_header->id = type->id.value(); + spec_header->id = type.id.value(); - if (sorted_entries->empty()) { + if (sorted_entries.empty()) { type_spec_writer.Finish(); return true; } @@ -557,7 +534,7 @@ class PackageFlattener { // We can't just take the size of the vector. There may be holes in the // entry ID space. // Since the entries are sorted by ID, the last one will be the biggest. - const size_t num_entries = sorted_entries->back()->id.value() + 1; + const size_t num_entries = sorted_entries.back()->id.value().entry_id() + 1; spec_header->entryCount = util::HostToDevice32(num_entries); @@ -565,21 +542,19 @@ class PackageFlattener { // show for which configuration axis the resource changes. uint32_t* config_masks = type_spec_writer.NextBlock<uint32_t>(num_entries); - const size_t actual_num_entries = sorted_entries->size(); - for (size_t entryIndex = 0; entryIndex < actual_num_entries; entryIndex++) { - ResourceEntry* entry = sorted_entries->at(entryIndex); + for (const ResourceEntry* entry : sorted_entries) { + const uint16_t entry_id = entry->id.value().entry_id(); // Populate the config masks for this entry. if (entry->visibility.level == Visibility::Level::kPublic) { - config_masks[entry->id.value()] |= util::HostToDevice32(ResTable_typeSpec::SPEC_PUBLIC); + config_masks[entry_id] |= util::HostToDevice32(ResTable_typeSpec::SPEC_PUBLIC); } const size_t config_count = entry->values.size(); for (size_t i = 0; i < config_count; i++) { const ConfigDescription& config = entry->values[i]->config; for (size_t j = i + 1; j < config_count; j++) { - config_masks[entry->id.value()] |= - util::HostToDevice32(config.diff(entry->values[j]->config)); + config_masks[entry_id] |= util::HostToDevice32(config.diff(entry->values[j]->config)); } } } @@ -590,32 +565,31 @@ class PackageFlattener { bool FlattenTypes(BigBuffer* buffer) { // Sort the types by their IDs. They will be inserted into the StringPool in // this order. - std::vector<ResourceTableType*> sorted_types = CollectAndSortTypes(); size_t expected_type_id = 1; - for (ResourceTableType* type : sorted_types) { + for (const ResourceTableTypeView& type : package_.types) { + if (type.type == ResourceType::kStyleable) { + // Styleables aren't real Resource Types, they are represented in the R.java file. + continue; + } + // If there is a gap in the type IDs, fill in the StringPool // with empty values until we reach the ID we expect. - while (type->id.value() > expected_type_id) { + while (type.id.value() > expected_type_id) { std::stringstream type_name; type_name << "?" << expected_type_id; type_pool_.MakeRef(type_name.str()); expected_type_id++; } expected_type_id++; - type_pool_.MakeRef(to_string(type->type)); - - std::vector<ResourceEntry*> sorted_entries = CollectAndSortEntries(type); - if (sorted_entries.empty()) { - continue; - } + type_pool_.MakeRef(to_string(type.type)); - if (!FlattenTypeSpec(type, &sorted_entries, buffer)) { + if (!FlattenTypeSpec(type, type.entries, buffer)) { return false; } // Since the entries are sorted by ID, the last ID will be the largest. - const size_t num_entries = sorted_entries.back()->id.value() + 1; + const size_t num_entries = type.entries.back()->id.value().entry_id() + 1; // The binary resource table lists resource entries for each // configuration. @@ -628,9 +602,9 @@ class PackageFlattener { // hardcoded string uses characters which make it an invalid resource name const std::string obfuscated_resource_name = "0_resource_name_obfuscated"; - for (ResourceEntry* entry : sorted_entries) { + for (const ResourceEntry* entry : type.entries) { uint32_t local_key_index; - ResourceName resource_name({}, type->type, entry->name); + ResourceName resource_name({}, type.type, entry->name); if (!collapse_key_stringpool_ || name_collapse_exemptions_.find(resource_name) != name_collapse_exemptions_.end()) { local_key_index = (uint32_t)key_pool_.MakeRef(entry->name).index(); @@ -660,17 +634,17 @@ class PackageFlattener { ResTable_lib_header* lib_header = lib_writer.StartChunk<ResTable_lib_header>(RES_TABLE_LIBRARY_TYPE); - const size_t num_entries = (package_->id.value() == 0x00 ? 1 : 0) + shared_libs_->size(); + const size_t num_entries = (package_.id.value() == 0x00 ? 1 : 0) + shared_libs_->size(); CHECK(num_entries > 0); lib_header->count = util::HostToDevice32(num_entries); ResTable_lib_entry* lib_entry = buffer->NextBlock<ResTable_lib_entry>(num_entries); - if (package_->id.value() == 0x00) { + if (package_.id.value() == 0x00) { // Add this package lib_entry->packageId = util::HostToDevice32(0x00); strcpy16_htod(lib_entry->packageName, arraysize(lib_entry->packageName), - util::Utf8ToUtf16(package_->name)); + util::Utf8ToUtf16(package_.name)); ++lib_entry; } @@ -685,7 +659,7 @@ class PackageFlattener { IAaptContext* context_; IDiagnostics* diag_; - ResourceTablePackage* package_; + const ResourceTablePackageView package_; const std::map<size_t, std::string>* shared_libs_; bool use_sparse_entries_; StringPool type_pool_; @@ -709,9 +683,10 @@ bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) { }); // Write the ResTable header. + const auto& table_view = table->GetPartitionedView(); ChunkWriter table_writer(buffer_); ResTable_header* table_header = table_writer.StartChunk<ResTable_header>(RES_TABLE_TYPE); - table_header->packageCount = util::HostToDevice32(table->packages.size()); + table_header->packageCount = util::HostToDevice32(table_view.packages.size()); // Flatten the values string pool. StringPool::FlattenUtf8(table_writer.buffer(), table->string_pool, @@ -720,24 +695,25 @@ bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) { BigBuffer package_buffer(1024); // Flatten each package. - for (auto& package : table->packages) { + for (auto& package : table_view.packages) { if (context->GetPackageType() == PackageType::kApp) { // Write a self mapping entry for this package if the ID is non-standard (0x7f). - const uint8_t package_id = package->id.value(); + CHECK((bool)package.id) << "Resource ids have not been assigned before flattening the table"; + const uint8_t package_id = package.id.value(); if (package_id != kFrameworkPackageId && package_id != kAppPackageId) { - auto result = table->included_packages_.insert({package_id, package->name}); - if (!result.second && result.first->second != package->name) { + auto result = table->included_packages_.insert({package_id, package.name}); + if (!result.second && result.first->second != package.name) { // A mapping for this package ID already exists, and is a different package. Error! context->GetDiagnostics()->Error( DiagMessage() << android::base::StringPrintf( "can't map package ID %02x to '%s'. Already mapped to '%s'", package_id, - package->name.c_str(), result.first->second.c_str())); + package.name.c_str(), result.first->second.c_str())); return false; } } } - PackageFlattener flattener(context, package.get(), &table->included_packages_, + PackageFlattener flattener(context, package, &table->included_packages_, options_.use_sparse_entries, options_.collapse_key_stringpool, options_.name_collapse_exemptions); if (!flattener.FlattenPackage(&package_buffer)) { diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index f8b8a1c4e35b..f5fe5e30bf0f 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -155,7 +155,6 @@ class TableFlattenerTest : public ::testing::Test { TEST_F(TableFlattenerTest, FlattenFullyLinkedTable) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(0x7f020000)) .AddSimple("com.app.test:id/two", ResourceId(0x7f020001)) .AddValue("com.app.test:id/three", ResourceId(0x7f020002), @@ -204,7 +203,6 @@ TEST_F(TableFlattenerTest, FlattenFullyLinkedTable) { TEST_F(TableFlattenerTest, FlattenEntriesWithGapsInIds) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(0x7f020001)) .AddSimple("com.app.test:id/three", ResourceId(0x7f020003)) .Build(); @@ -225,7 +223,6 @@ TEST_F(TableFlattenerTest, FlattenMinMaxAttributes) { attr.max_int = 23; std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/foo", ResourceId(0x01010000), util::make_unique<Attribute>(attr)) .Build(); @@ -248,7 +245,6 @@ TEST_F(TableFlattenerTest, FlattenArray) { 2u)); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:array/foo", ResourceId(0x01010000), std::move(array)) .Build(); @@ -300,7 +296,6 @@ static std::unique_ptr<ResourceTable> BuildTableWithSparseEntries( IAaptContext* context, const ConfigDescription& sparse_config, float load) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId(context->GetCompilationPackage(), context->GetPackageId()) .Build(); // Add regular entries. @@ -311,15 +306,20 @@ static std::unique_ptr<ResourceTable> BuildTableWithSparseEntries( const ResourceId resid(context->GetPackageId(), 0x02, static_cast<uint16_t>(i)); const auto value = util::make_unique<BinaryPrimitive>(Res_value::TYPE_INT_DEC, static_cast<uint32_t>(i)); - CHECK(table->AddResourceWithId(name, resid, ConfigDescription::DefaultConfig(), "", - std::unique_ptr<Value>(value->Clone(nullptr)), - context->GetDiagnostics())); + CHECK(table->AddResource(NewResourceBuilder(name) + .SetId(resid) + .SetValue(std::unique_ptr<Value>(value->Clone(nullptr))) + .Build(), + context->GetDiagnostics())); // Every few entries, write out a sparse_config value. This will give us the desired load. if (i % stride == 0) { - CHECK(table->AddResourceWithId(name, resid, sparse_config, "", - std::unique_ptr<Value>(value->Clone(nullptr)), - context->GetDiagnostics())); + CHECK(table->AddResource( + NewResourceBuilder(name) + .SetId(resid) + .SetValue(std::unique_ptr<Value>(value->Clone(nullptr)), sparse_config) + .Build(), + context->GetDiagnostics())); } } return table; @@ -417,7 +417,6 @@ TEST_F(TableFlattenerTest, FlattenSharedLibrary) { test::ContextBuilder().SetCompilationPackage("lib").SetPackageId(0x00).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("lib", 0x00) .AddValue("lib:id/foo", ResourceId(0x00010000), util::make_unique<Id>()) .Build(); ResourceTable result; @@ -426,7 +425,7 @@ TEST_F(TableFlattenerTest, FlattenSharedLibrary) { Maybe<ResourceTable::SearchResult> search_result = result.FindResource(test::ParseNameOrDie("lib:id/foo")); ASSERT_TRUE(search_result); - EXPECT_EQ(0x00u, search_result.value().package->id.value()); + EXPECT_EQ(0x00u, search_result.value().entry->id.value().package_id()); auto iter = result.included_packages_.find(0x00); ASSERT_NE(result.included_packages_.end(), iter); @@ -438,7 +437,6 @@ TEST_F(TableFlattenerTest, FlattenSharedLibraryWithStyle) { test::ContextBuilder().SetCompilationPackage("lib").SetPackageId(0x00).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("lib", 0x00) .AddValue("lib:style/Theme", ResourceId(0x00030001), test::StyleBuilder() @@ -458,9 +456,7 @@ TEST_F(TableFlattenerTest, FlattenSharedLibraryWithStyle) { Maybe<ResourceTable::SearchResult> search_result = result.FindResource(test::ParseNameOrDie("lib:style/Theme")); ASSERT_TRUE(search_result); - EXPECT_EQ(0x00u, search_result.value().package->id.value()); - EXPECT_EQ(0x03u, search_result.value().type->id.value()); - EXPECT_EQ(0x01u, search_result.value().entry->id.value()); + EXPECT_EQ(0x00030001u, search_result.value().entry->id.value()); ASSERT_EQ(1u, search_result.value().entry->values.size()); Value* value = search_result.value().entry->values[0]->value.get(); Style* style = ValueCast<Style>(value); @@ -479,7 +475,6 @@ TEST_F(TableFlattenerTest, FlattenTableReferencingSharedLibraries) { test::ContextBuilder().SetCompilationPackage("app").SetPackageId(0x7f).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("app", 0x7f) .AddValue("app:id/foo", ResourceId(0x7f010000), test::BuildReference("lib_one:id/foo", ResourceId(0x02010000))) .AddValue("app:id/bar", ResourceId(0x7f010001), @@ -509,7 +504,6 @@ TEST_F(TableFlattenerTest, PackageWithNonStandardIdHasDynamicRefTable) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetCompilationPackage("app").SetPackageId(0x80).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("app", 0x80) .AddSimple("app:id/foo", ResourceId(0x80010000)) .Build(); @@ -532,7 +526,6 @@ TEST_F(TableFlattenerTest, LongPackageNameIsTruncated) { test::ContextBuilder().SetCompilationPackage(kPackageName).SetPackageId(0x7f).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId(kPackageName, 0x7f) .AddSimple(kPackageName + ":id/foo", ResourceId(0x7f010000)) .Build(); @@ -553,7 +546,6 @@ TEST_F(TableFlattenerTest, LongSharedLibraryPackageNameIsIllegal) { .Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId(kPackageName, 0x7f) .AddSimple(kPackageName + ":id/foo", ResourceId(0x7f010000)) .Build(); @@ -564,7 +556,6 @@ TEST_F(TableFlattenerTest, LongSharedLibraryPackageNameIsIllegal) { TEST_F(TableFlattenerTest, ObfuscatingResourceNamesNoNameCollapseExemptionsSucceeds) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(0x7f020000)) .AddSimple("com.app.test:id/two", ResourceId(0x7f020001)) .AddValue("com.app.test:id/three", ResourceId(0x7f020002), @@ -618,7 +609,6 @@ TEST_F(TableFlattenerTest, ObfuscatingResourceNamesNoNameCollapseExemptionsSucce TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithNameCollapseExemptionsSucceeds) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(0x7f020000)) .AddSimple("com.app.test:id/two", ResourceId(0x7f020001)) .AddValue("com.app.test:id/three", ResourceId(0x7f020002), @@ -680,7 +670,6 @@ TEST_F(TableFlattenerTest, FlattenOverlayable) { std::string name = "com.app.test:integer/overlayable"; std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple(name, ResourceId(0x7f020000)) .SetOverlayable(name, overlayable_item) .Build(); @@ -717,7 +706,6 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayablePolicies) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple(name_zero, ResourceId(0x7f020000)) .SetOverlayable(name_zero, overlayable_item_zero) .AddSimple(name_one, ResourceId(0x7f020001)) @@ -780,7 +768,6 @@ TEST_F(TableFlattenerTest, FlattenMultipleOverlayable) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple(name_zero, ResourceId(0x7f020000)) .SetOverlayable(name_zero, overlayable_item_zero) .AddSimple(name_one, ResourceId(0x7f020001)) @@ -842,7 +829,6 @@ TEST_F(TableFlattenerTest, FlattenOverlayableNoPolicyFails) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple(name_zero, ResourceId(0x7f020000)) .SetOverlayable(name_zero, overlayable_item_zero) .Build(); diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index 06ac9e5dc5c4..bfb92da27e4d 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -425,15 +425,9 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr io::IFileCollection* files, const std::vector<std::shared_ptr<Overlayable>>& overlayables, ResourceTable* out_table, std::string* out_error) { - Maybe<uint8_t> id; - if (pb_package.has_package_id()) { - id = static_cast<uint8_t>(pb_package.package_id().id()); - } - std::map<ResourceId, ResourceNameRef> id_index; - ResourceTablePackage* pkg = - out_table->CreatePackageAllowingDuplicateNames(pb_package.package_name(), id); + ResourceTablePackage* pkg = out_table->FindOrCreatePackage(pb_package.package_name()); for (const pb::Type& pb_type : pb_package.type()) { const ResourceType* res_type = ParseResourceType(pb_type.name()); if (res_type == nullptr) { @@ -444,17 +438,15 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr } ResourceTableType* type = pkg->FindOrCreateType(*res_type); - if (pb_type.has_type_id()) { - type->id = static_cast<uint8_t>(pb_type.type_id().id()); - } for (const pb::Entry& pb_entry : pb_type.entry()) { - ResourceEntry* entry; - if (pb_entry.has_entry_id()) { - auto entry_id = static_cast<uint16_t>(pb_entry.entry_id().id()); - entry = type->FindOrCreateEntry(pb_entry.name(), entry_id); - } else { - entry = type->FindOrCreateEntry(pb_entry.name()); + ResourceEntry* entry = type->CreateEntry(pb_entry.name()); + const ResourceId resource_id( + pb_package.has_package_id() ? static_cast<uint8_t>(pb_package.package_id().id()) : 0u, + pb_type.has_type_id() ? static_cast<uint8_t>(pb_type.type_id().id()) : 0u, + pb_entry.has_entry_id() ? static_cast<uint16_t>(pb_entry.entry_id().id()) : 0u); + if (resource_id.id != 0u) { + entry->id = resource_id; } // Deserialize the symbol status (public/private with source and comments). diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index 98c517510028..9842e253f063 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -345,28 +345,29 @@ void SerializeTableToPb(const ResourceTable& table, pb::ResourceTable* out_table pb_fingerprint->set_version(util::GetToolFingerprint()); std::vector<Overlayable*> overlayables; - for (const std::unique_ptr<ResourceTablePackage>& package : table.packages) { + auto table_view = table.GetPartitionedView(); + for (const auto& package : table_view.packages) { pb::Package* pb_package = out_table->add_package(); - if (package->id) { - pb_package->mutable_package_id()->set_id(package->id.value()); + if (package.id) { + pb_package->mutable_package_id()->set_id(package.id.value()); } - pb_package->set_package_name(package->name); + pb_package->set_package_name(package.name); - for (const std::unique_ptr<ResourceTableType>& type : package->types) { + for (const auto& type : package.types) { pb::Type* pb_type = pb_package->add_type(); - if (type->id) { - pb_type->mutable_type_id()->set_id(type->id.value()); + if (type.id) { + pb_type->mutable_type_id()->set_id(type.id.value()); } - pb_type->set_name(to_string(type->type).to_string()); + pb_type->set_name(to_string(type.type).to_string()); // hardcoded string uses characters which make it an invalid resource name static const char* obfuscated_resource_name = "0_resource_name_obfuscated"; - for (const std::unique_ptr<ResourceEntry>& entry : type->entries) { + for (const auto& entry : type.entries) { pb::Entry* pb_entry = pb_type->add_entry(); if (entry->id) { - pb_entry->mutable_entry_id()->set_id(entry->id.value()); + pb_entry->mutable_entry_id()->set_id(entry->id.value().entry_id()); } - ResourceName resource_name({}, type->type, entry->name); + ResourceName resource_name({}, type.type, entry->name); if (options.collapse_key_stringpool && options.name_collapse_exemptions.find(resource_name) == options.name_collapse_exemptions.end()) { diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index fe4c8aa065f1..ad5ed4db55b6 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -40,18 +40,20 @@ class MockFileCollection : public io::IFileCollection { MOCK_METHOD0(GetDirSeparator, char()); }; -ResourceEntry* GetEntry(ResourceTable* table, const ResourceNameRef& res_name, - uint32_t id) { - ResourceTablePackage* package = table->FindPackage(res_name.package); - ResourceTableType* type = package->FindType(res_name.type); - return type->FindEntry(res_name.entry, id); +ResourceEntry* GetEntry(ResourceTable* table, const ResourceNameRef& res_name) { + auto result = table->FindResource(res_name); + return (result) ? result.value().entry : nullptr; +} + +ResourceEntry* GetEntry(ResourceTable* table, const ResourceNameRef& res_name, ResourceId id) { + auto result = table->FindResource(res_name, id); + return (result) ? result.value().entry : nullptr; } TEST(ProtoSerializeTest, SerializeSinglePackage) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddFileReference("com.app.a:layout/main", ResourceId(0x7f020000), "res/layout/main.xml") .AddReference("com.app.a:layout/other", ResourceId(0x7f020001), "com.app.a:layout/main") .AddString("com.app.a:string/text", {}, "hi") @@ -60,11 +62,11 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { true /*allow_new*/) .Build(); - Visibility public_symbol; - public_symbol.level = Visibility::Level::kPublic; - ASSERT_TRUE(table->SetVisibilityWithId(test::ParseNameOrDie("com.app.a:layout/main"), - public_symbol, ResourceId(0x7f020000), - context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:layout/main")) + .SetId(0x7f020000) + .SetVisibility({Visibility::Level::kPublic}) + .Build(), + context->GetDiagnostics())); Id* id = test::GetValue<Id>(table.get(), "com.app.a:id/foo"); ASSERT_THAT(id, NotNull()); @@ -72,25 +74,35 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { // Make a plural. std::unique_ptr<Plural> plural = util::make_unique<Plural>(); plural->values[Plural::One] = util::make_unique<String>(table->string_pool.MakeRef("one")); - ASSERT_TRUE(table->AddResource(test::ParseNameOrDie("com.app.a:plurals/hey"), ConfigDescription{}, - {}, std::move(plural), context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:plurals/hey")) + .SetValue(std::move(plural)) + .Build(), + context->GetDiagnostics())); // Make a styled string. StyleString style_string; style_string.str = "hello"; style_string.spans.push_back(Span{"b", 0u, 4u}); + ASSERT_TRUE(table->AddResource( + NewResourceBuilder(test::ParseNameOrDie("com.app.a:string/styled")) + .SetValue(util::make_unique<StyledString>(table->string_pool.MakeRef(style_string))) + .Build(), + context->GetDiagnostics())); + + // Make a resource with different products. ASSERT_TRUE( - table->AddResource(test::ParseNameOrDie("com.app.a:string/styled"), ConfigDescription{}, {}, - util::make_unique<StyledString>(table->string_pool.MakeRef(style_string)), + table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:integer/one")) + .SetValue(test::BuildPrimitive(android::Res_value::TYPE_INT_DEC, 123u), + test::ParseConfigOrDie("land")) + .Build(), context->GetDiagnostics())); - // Make a resource with different products. - ASSERT_TRUE(table->AddResource( - test::ParseNameOrDie("com.app.a:integer/one"), test::ParseConfigOrDie("land"), {}, - test::BuildPrimitive(android::Res_value::TYPE_INT_DEC, 123u), context->GetDiagnostics())); - ASSERT_TRUE(table->AddResource( - test::ParseNameOrDie("com.app.a:integer/one"), test::ParseConfigOrDie("land"), "tablet", - test::BuildPrimitive(android::Res_value::TYPE_INT_HEX, 321u), context->GetDiagnostics())); + ASSERT_TRUE( + table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:integer/one")) + .SetValue(test::BuildPrimitive(android::Res_value::TYPE_INT_HEX, 321u), + test::ParseConfigOrDie("land"), "tablet") + .Build(), + context->GetDiagnostics())); // Make a reference with both resource name and resource ID. // The reference should point to a resource outside of this table to test that both name and id @@ -98,16 +110,20 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { Reference expected_ref; expected_ref.name = test::ParseNameOrDie("android:layout/main"); expected_ref.id = ResourceId(0x01020000); - ASSERT_TRUE(table->AddResource( - test::ParseNameOrDie("com.app.a:layout/abc"), ConfigDescription::DefaultConfig(), {}, - util::make_unique<Reference>(expected_ref), context->GetDiagnostics())); + ASSERT_TRUE(table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:layout/abc")) + .SetValue(util::make_unique<Reference>(expected_ref)) + .Build(), + context->GetDiagnostics())); // Make an overlayable resource. OverlayableItem overlayable_item(std::make_shared<Overlayable>( "OverlayableName", "overlay://theme", Source("res/values/overlayable.xml", 40))); overlayable_item.source = Source("res/values/overlayable.xml", 42); - ASSERT_TRUE(table->SetOverlayable(test::ParseNameOrDie("com.app.a:integer/overlayable"), - overlayable_item, test::GetDiagnostics())); + ASSERT_TRUE( + table->AddResource(NewResourceBuilder(test::ParseNameOrDie("com.app.a:integer/overlayable")) + .SetOverlayable(overlayable_item) + .Build(), + context->GetDiagnostics())); pb::ResourceTable pb_table; SerializeTableToPb(*table, &pb_table, context->GetDiagnostics()); @@ -680,7 +696,6 @@ TEST(ProtoSerializeTest, CollapsingResourceNamesNoNameCollapseExemptionsSucceeds std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(id_one_id)) .AddSimple("com.app.test:id/two", ResourceId(id_two_id)) .AddValue("com.app.test:id/three", ResourceId(id_three_id), @@ -714,7 +729,7 @@ TEST(ProtoSerializeTest, CollapsingResourceNamesNoNameCollapseExemptionsSucceeds ResourceName real_id_resource( "com.app.test", ResourceType::kId, "one"); - EXPECT_THAT(GetEntry(&new_table, real_id_resource, id_one_id), IsNull()); + EXPECT_THAT(GetEntry(&new_table, real_id_resource), IsNull()); ResourceName obfuscated_id_resource( "com.app.test", ResourceType::kId, "0_resource_name_obfuscated"); @@ -767,7 +782,6 @@ TEST(ProtoSerializeTest, ObfuscatingResourceNamesWithNameCollapseExemptionsSucce std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddSimple("com.app.test:id/one", ResourceId(id_one_id)) .AddSimple("com.app.test:id/two", ResourceId(id_two_id)) .AddValue("com.app.test:id/three", ResourceId(id_three_id), diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 59dd481607e9..039448e1b3e9 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -542,8 +542,8 @@ bool JavaClassGenerator::ProcessType(const StringPiece& package_name_to_generate // Create an ID if there is one (static libraries don't need one). ResourceId id; - if (package.id && type.id && entry->id) { - id = ResourceId(package.id.value(), type.id.value(), entry->id.value()); + if (entry->id) { + id = entry->id.value(); } // We need to make sure we hide the fact that we are generating kAttrPrivate attributes. diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index ec5b4151b1a6..8bb3ee93d312 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -34,7 +34,6 @@ namespace aapt { TEST(JavaClassGeneratorTest, FailWhenEntryIsJavaKeyword) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:id/class", ResourceId(0x01020000)) .Build(); @@ -54,7 +53,6 @@ TEST(JavaClassGeneratorTest, FailWhenEntryIsJavaKeyword) { TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:id/hey-man", ResourceId(0x01020000)) .AddValue("android:attr/cool.attr", ResourceId(0x01010000), test::AttributeBuilder().Build()) @@ -84,7 +82,6 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) { TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:id/one", ResourceId(0x01020000)) .AddSimple("android:id/com.foo$two", ResourceId(0x01020001)) .Build(); @@ -110,8 +107,6 @@ TEST(JavaClassGeneratorTest, CorrectPackageNameIsUsed) { TEST(JavaClassGeneratorTest, StyleableAttributesWithDifferentPackageName) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) - .SetPackageId("app", 0x7f) .AddValue("app:attr/foo", ResourceId(0x7f010000), test::AttributeBuilder().Build()) .AddValue("app:attr/bar", ResourceId(0x7f010001), @@ -159,7 +154,6 @@ TEST(JavaClassGeneratorTest, StyleableAttributesWithDifferentPackageName) { TEST(JavaClassGeneratorTest, AttrPrivateIsWrittenAsAttr) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:attr/two", ResourceId(0x01010001)) .AddSimple("android:^attr-private/one", ResourceId(0x01010000)) .Build(); @@ -184,7 +178,6 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { StdErrDiagnostics diag; std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:id/one", ResourceId(0x01020000)) .AddSimple("android:id/two", ResourceId(0x01020001)) .AddSimple("android:id/three", ResourceId(0x01020002)) @@ -276,8 +269,6 @@ ResourceId>()), TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) - .SetPackageId("com.lib", 0x02) .AddValue("android:attr/bar", ResourceId(0x01010000), test::AttributeBuilder().Build()) .AddValue("com.lib:attr/bar", ResourceId(0x02010000), test::AttributeBuilder().Build()) .AddValue("android:styleable/foo", ResourceId(0x01030000), @@ -306,7 +297,6 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) { TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:id/foo", ResourceId(0x01010000)) .Build(); test::GetValue<Id>(table.get(), "android:id/foo") @@ -346,7 +336,6 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/one", util::make_unique<Attribute>(attr)) .AddValue("android:styleable/Container", std::unique_ptr<Styleable>(styleable.Clone(nullptr))) @@ -384,7 +373,6 @@ TEST(JavaClassGeneratorTest, CommentsForStyleableHiddenAttributesAreNotPresent) std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/one", util::make_unique<Attribute>(attr)) .AddValue("android:styleable/Container", std::unique_ptr<Styleable>(styleable.Clone(nullptr))) @@ -415,7 +403,6 @@ TEST(JavaClassGeneratorTest, CommentsForStyleableHiddenAttributesAreNotPresent) TEST(JavaClassGeneratorTest, StyleableAndIndicesAreColocated) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/layout_gravity", util::make_unique<Attribute>()) .AddValue("android:attr/background", util::make_unique<Attribute>()) .AddValue("android:styleable/ActionBar", @@ -467,7 +454,6 @@ TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/one", util::make_unique<Attribute>(attr)) .Build(); @@ -499,7 +485,6 @@ TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x00) .AddValue("android:attr/foo", ResourceId(0x00010000), util::make_unique<Attribute>()) .AddValue("android:id/foo", ResourceId(0x00020000), util::make_unique<Id>()) .AddValue( @@ -536,7 +521,6 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) TEST(JavaClassGeneratorTest, OnlyGenerateRText) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddValue("android:attr/foo", ResourceId(0x01010000), util::make_unique<Attribute>()) .AddValue("android:styleable/hey.dude", ResourceId(0x01020000), test::StyleableBuilder() @@ -554,8 +538,6 @@ TEST(JavaClassGeneratorTest, OnlyGenerateRText) { TEST(JavaClassGeneratorTest, SortsDynamicAttributesAfterFrameworkAttributes) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) - .SetPackageId("lib", 0x00) .AddValue("android:attr/framework_attr", ResourceId(0x01010000), test::AttributeBuilder().Build()) .AddValue("lib:attr/dynamic_attr", ResourceId(0x00010000), diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp index c7ae0b6a75cc..b7dfec3a6b28 100644 --- a/tools/aapt2/java/ProguardRules_test.cpp +++ b/tools/aapt2/java/ProguardRules_test.cpp @@ -247,8 +247,10 @@ TEST(ProguardRulesTest, IncludedLayoutRulesAreConditional) { ResourceTable table; StdErrDiagnostics errDiagnostics; - table.AddResource(bar_layout->file.name, ConfigDescription::DefaultConfig(), "", - util::make_unique<FileReference>(), &errDiagnostics); + table.AddResource(NewResourceBuilder(bar_layout->file.name) + .SetValue(util::make_unique<FileReference>()) + .Build(), + &errDiagnostics); std::unique_ptr<IAaptContext> context = test::ContextBuilder() diff --git a/tools/aapt2/link/AutoVersioner_test.cpp b/tools/aapt2/link/AutoVersioner_test.cpp index 1117472f104b..02fd00bba439 100644 --- a/tools/aapt2/link/AutoVersioner_test.cpp +++ b/tools/aapt2/link/AutoVersioner_test.cpp @@ -54,7 +54,6 @@ TEST(AutoVersionerTest, GenerateVersionedResourceWhenHigherVersionExists) { TEST(AutoVersionerTest, VersionStylesForTable) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("app", 0x7f) .AddValue( "app:style/Foo", test::ParseConfigOrDie("v4"), ResourceId(0x7f020000), diff --git a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp index d129c9ac8db7..3179fefc3fd0 100644 --- a/tools/aapt2/link/NoDefaultResourceRemover_test.cpp +++ b/tools/aapt2/link/NoDefaultResourceRemover_test.cpp @@ -21,10 +21,9 @@ namespace aapt { TEST(NoDefaultResourceRemoverTest, RemoveEntryWithNoDefaultAndOnlyLocales) { - std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); + std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetPackageId(0x01).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:string/foo") .AddSimple("android:string/foo", test::ParseConfigOrDie("en-rGB")) .AddSimple("android:string/foo", test::ParseConfigOrDie("fr-rFR")) @@ -47,10 +46,10 @@ TEST(NoDefaultResourceRemoverTest, RemoveEntryWithNoDefaultAndOnlyLocales) { } TEST(NoDefaultResourceRemoverTest, KeepEntryWithLocalesAndDensities) { - std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(26).Build(); + std::unique_ptr<IAaptContext> context = + test::ContextBuilder().SetPackageId(0x01).SetMinSdkVersion(26).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("mdpi")) // v4 .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) @@ -71,10 +70,10 @@ TEST(NoDefaultResourceRemoverTest, KeepEntryWithLocalesAndDensities) { } TEST(NoDefaultResourceRemoverTest, RemoveEntryWithLocalesAndDensitiesLowVersion) { - std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(3).Build(); + std::unique_ptr<IAaptContext> context = + test::ContextBuilder().SetPackageId(0x01).SetMinSdkVersion(3).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("mdpi")) // v4 .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("en-rGB")) .AddSimple("android:drawable/remove1", test::ParseConfigOrDie("fr-rFR")) @@ -87,10 +86,10 @@ TEST(NoDefaultResourceRemoverTest, RemoveEntryWithLocalesAndDensitiesLowVersion) } TEST(NoDefaultResourceRemoverTest, KeepEntryWithVersion) { - std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(8).Build(); + std::unique_ptr<IAaptContext> context = + test::ContextBuilder().SetPackageId(0x01).SetMinSdkVersion(8).Build(); std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("android", 0x01) .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("v8")) .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("en-rGB")) .AddSimple("android:drawable/keep1", test::ParseConfigOrDie("fr-rFR")) diff --git a/tools/aapt2/link/ProductFilter_test.cpp b/tools/aapt2/link/ProductFilter_test.cpp index dd4767463711..4f78bbcece33 100644 --- a/tools/aapt2/link/ProductFilter_test.cpp +++ b/tools/aapt2/link/ProductFilter_test.cpp @@ -30,21 +30,29 @@ TEST(ProductFilterTest, SelectTwoProducts) { ResourceTable table; ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), land, "", - test::ValueBuilder<Id>().SetSource(Source("land/default.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("land/default.xml")).Build(), land) + .Build(), context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), land, "tablet", - test::ValueBuilder<Id>().SetSource(Source("land/tablet.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("land/tablet.xml")).Build(), land, + "tablet") + .Build(), context->GetDiagnostics())); ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), port, "", - test::ValueBuilder<Id>().SetSource(Source("port/default.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("port/default.xml")).Build(), port) + .Build(), context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), port, "tablet", - test::ValueBuilder<Id>().SetSource(Source("port/tablet.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("port/tablet.xml")).Build(), port, + "tablet") + .Build(), context->GetDiagnostics())); ProductFilter filter({"tablet"}); @@ -65,15 +73,17 @@ TEST(ProductFilterTest, SelectDefaultProduct) { ResourceTable table; ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "", - test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build()) + .Build(), context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "tablet", - test::ValueBuilder<Id>().SetSource(Source("tablet.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("tablet.xml")).Build(), {}, "tablet") + .Build(), context->GetDiagnostics())); + ; ProductFilter filter(std::unordered_set<std::string>{}); ASSERT_TRUE(filter.Consume(context.get(), &table)); @@ -91,19 +101,22 @@ TEST(ProductFilterTest, FailOnAmbiguousProduct) { ResourceTable table; ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "", - test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build()) + .Build(), context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "tablet", - test::ValueBuilder<Id>().SetSource(Source("tablet.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("tablet.xml")).Build(), {}, "tablet") + .Build(), context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "no-sdcard", - test::ValueBuilder<Id>().SetSource(Source("no-sdcard.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("no-sdcard.xml")).Build(), {}, + "no-sdcard") + .Build(), context->GetDiagnostics())); ProductFilter filter({"tablet", "no-sdcard"}); @@ -114,15 +127,17 @@ TEST(ProductFilterTest, FailOnMultipleDefaults) { std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build(); ResourceTable table; + ASSERT_TRUE( + table.AddResource(NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source(".xml")).Build()) + .Build(), + context->GetDiagnostics())); + ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "", - test::ValueBuilder<Id>().SetSource(Source(".xml")).Build(), - context->GetDiagnostics())); - ASSERT_TRUE(table.AddResource( - test::ParseNameOrDie("android:string/one"), - ConfigDescription::DefaultConfig(), "default", - test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build(), + NewResourceBuilder(test::ParseNameOrDie("android:string/one")) + .SetValue(test::ValueBuilder<Id>().SetSource(Source("default.xml")).Build(), {}, + "default") + .Build(), context->GetDiagnostics())); ProductFilter filter(std::unordered_set<std::string>{}); diff --git a/tools/aapt2/link/ReferenceLinker_test.cpp b/tools/aapt2/link/ReferenceLinker_test.cpp index a31ce9496d0c..228c5bd743a0 100644 --- a/tools/aapt2/link/ReferenceLinker_test.cpp +++ b/tools/aapt2/link/ReferenceLinker_test.cpp @@ -28,7 +28,6 @@ namespace aapt { TEST(ReferenceLinkerTest, LinkSimpleReferences) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddReference("com.app.test:string/foo", ResourceId(0x7f020000), "com.app.test:string/bar") @@ -75,7 +74,6 @@ TEST(ReferenceLinkerTest, LinkSimpleReferences) { TEST(ReferenceLinkerTest, LinkStyleAttributes) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddValue("com.app.test:style/Theme", test::StyleBuilder() .SetParent("android:style/Theme.Material") @@ -155,7 +153,6 @@ TEST(ReferenceLinkerTest, LinkMangledReferencesAndAttributes) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddValue("com.app.test:style/Theme", ResourceId(0x7f020000), test::StyleBuilder() .AddItem("com.android.support:attr/foo", @@ -176,7 +173,6 @@ TEST(ReferenceLinkerTest, LinkMangledReferencesAndAttributes) { TEST(ReferenceLinkerTest, FailToLinkPrivateSymbols) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddReference("com.app.test:string/foo", ResourceId(0x7f020000), "android:string/hidden") .Build(); @@ -201,7 +197,6 @@ TEST(ReferenceLinkerTest, FailToLinkPrivateSymbols) { TEST(ReferenceLinkerTest, FailToLinkPrivateMangledSymbols) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddReference("com.app.test:string/foo", ResourceId(0x7f020000), "com.app.lib:string/hidden") .Build(); @@ -229,7 +224,6 @@ TEST(ReferenceLinkerTest, FailToLinkPrivateMangledSymbols) { TEST(ReferenceLinkerTest, FailToLinkPrivateStyleAttributes) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() - .SetPackageId("com.app.test", 0x7f) .AddValue("com.app.test:style/Theme", test::StyleBuilder() .AddItem("android:attr/hidden", diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index ad56092754aa..4ef2882ce347 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -33,8 +33,7 @@ TableMerger::TableMerger(IAaptContext* context, ResourceTable* out_table, const TableMergerOptions& options) : context_(context), main_table_(out_table), options_(options) { // Create the desired package that all tables will be merged into. - main_package_ = - main_table_->CreatePackage(context_->GetCompilationPackage(), context_->GetPackageId()); + main_package_ = main_table_->FindOrCreatePackage(context_->GetCompilationPackage()); CHECK(main_package_ != nullptr) << "package name or ID already taken"; } @@ -85,20 +84,9 @@ bool TableMerger::MergeAndMangle(const Source& src, const StringPiece& package_n static bool MergeType(IAaptContext* context, const Source& src, ResourceTableType* dst_type, ResourceTableType* src_type) { - if (src_type->visibility_level > dst_type->visibility_level) { + if (src_type->visibility_level >= dst_type->visibility_level) { // The incoming type's visibility is stronger, so we should override the visibility. - if (src_type->visibility_level == Visibility::Level::kPublic) { - // Only copy the ID if the source is public, or else the ID is meaningless. - dst_type->id = src_type->id; - } dst_type->visibility_level = src_type->visibility_level; - } else if (dst_type->visibility_level == Visibility::Level::kPublic && - src_type->visibility_level == Visibility::Level::kPublic && dst_type->id && - src_type->id && dst_type->id.value() != src_type->id.value()) { - // Both types are public and have different IDs. - context->GetDiagnostics()->Error(DiagMessage(src) << "cannot merge type '" << src_type->type - << "': conflicting public IDs"); - return false; } return true; } @@ -347,7 +335,7 @@ bool TableMerger::MergeFile(const ResourceFile& file_desc, bool overlay, io::IFi file_ref->type = file_desc.type; file_ref->file = file; - ResourceTablePackage* pkg = table.CreatePackage(file_desc.name.package, 0x0); + ResourceTablePackage* pkg = table.FindOrCreatePackage(file_desc.name.package); pkg->FindOrCreateType(file_desc.name.type) ->FindOrCreateEntry(file_desc.name.entry) ->FindOrCreateValue(file_desc.config, {}) diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 69cf5ee7002b..4358fb565a7d 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -55,16 +55,13 @@ struct TableMergerTest : public ::testing::Test { TEST_F(TableMergerTest, SimpleMerge) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddReference("com.app.a:id/foo", "com.app.a:id/bar") .AddReference("com.app.a:id/bar", "com.app.b:id/foo") - .AddValue( - "com.app.a:styleable/view", - test::StyleableBuilder().AddItem("com.app.b:id/foo").Build()) + .AddValue("com.app.a:styleable/view", + test::StyleableBuilder().AddItem("com.app.b:id/foo").Build()) .Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.b", 0x7f) .AddSimple("com.app.b:id/foo") .Build(); @@ -129,12 +126,10 @@ TEST_F(TableMergerTest, MergeFileReferences) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddFileReference("com.app.a:xml/file", "res/xml/file.xml", &file_a) .Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.b", 0x7f) .AddFileReference("com.app.b:xml/file", "res/xml/file.xml", &file_b) .Build(); @@ -158,12 +153,10 @@ TEST_F(TableMergerTest, MergeFileReferences) { TEST_F(TableMergerTest, OverrideResourceWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x00) .AddValue("bool/foo", ResourceUtils::TryParseBool("true")) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x00) .AddValue("bool/foo", ResourceUtils::TryParseBool("false")) .Build(); @@ -194,14 +187,12 @@ TEST_F(TableMergerTest, DoNotOverrideResourceComment) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x00) .AddValue("bool/foo", std::move(foo_original)) .AddValue("bool/bar", std::move(bar_original)) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x00) .AddValue("bool/foo", std::move(foo_overlay)) .AddValue("bool/bar", std::move(bar_overlay)) .AddValue("bool/baz", std::move(baz_overlay)) @@ -226,12 +217,10 @@ TEST_F(TableMergerTest, DoNotOverrideResourceComment) { TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); @@ -247,12 +236,10 @@ TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) { TEST_F(TableMergerTest, FailToOverrideConflictingTypeIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x02, 0x0001), Visibility::Level::kPublic) .Build(); @@ -268,12 +255,10 @@ TEST_F(TableMergerTest, FailToOverrideConflictingTypeIdsWithOverlay) { TEST_F(TableMergerTest, FailToOverrideConflictingEntryIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0002), Visibility::Level::kPublic) .Build(); @@ -289,12 +274,10 @@ TEST_F(TableMergerTest, FailToOverrideConflictingEntryIdsWithOverlay) { TEST_F(TableMergerTest, FailConflictingVisibility) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); std::unique_ptr<ResourceTable> overlay = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPrivate) .Build(); @@ -318,11 +301,9 @@ TEST_F(TableMergerTest, FailConflictingVisibility) { } TEST_F(TableMergerTest, MergeAddResourceFromOverlay) { - std::unique_ptr<ResourceTable> table_a = - test::ResourceTableBuilder().SetPackageId("", 0x7f).Build(); + std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder().Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .SetSymbolState("bool/foo", {}, Visibility::Level::kUndefined, true /*allow new overlay*/) .AddValue("bool/foo", ResourceUtils::TryParseBool("true")) .Build(); @@ -337,11 +318,9 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlay) { } TEST_F(TableMergerTest, MergeAddResourceFromOverlayWithAutoAddOverlay) { - std::unique_ptr<ResourceTable> table_a = - test::ResourceTableBuilder().SetPackageId("", 0x7f).Build(); + std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder().Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .AddValue("bool/foo", ResourceUtils::TryParseBool("true")) .Build(); @@ -355,11 +334,9 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlayWithAutoAddOverlay) { } TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) { - std::unique_ptr<ResourceTable> table_a = - test::ResourceTableBuilder().SetPackageId("", 0x7f).Build(); + std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder().Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("", 0x7f) .AddValue("bool/foo", ResourceUtils::TryParseBool("true")) .Build(); @@ -375,7 +352,6 @@ TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) { TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder() .AddItem("com.app.a:attr/bar") @@ -391,7 +367,6 @@ TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) { std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder() .AddItem("com.app.a:attr/bat") .AddItem("com.app.a:attr/foo") @@ -441,7 +416,6 @@ TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) { TEST_F(TableMergerTest, OverrideStyleInsteadOfOverlaying) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddValue( "com.app.a:styleable/MyWidget", test::StyleableBuilder().AddItem("com.app.a:attr/foo", ResourceId(0x1234)).Build()) @@ -452,7 +426,6 @@ TEST_F(TableMergerTest, OverrideStyleInsteadOfOverlaying) { .Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddValue( "com.app.a:styleable/MyWidget", test::StyleableBuilder().AddItem("com.app.a:attr/bar", ResourceId(0x5678)).Build()) @@ -494,13 +467,11 @@ TEST_F(TableMergerTest, SetOverlayable) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item) .Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddSimple("bool/foo") .Build(); @@ -527,7 +498,6 @@ TEST_F(TableMergerTest, SetOverlayableLater) { "overlay://customization"); std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .AddSimple("bool/foo") .Build(); @@ -536,7 +506,6 @@ TEST_F(TableMergerTest, SetOverlayableLater) { overlayable_item.policies |= PolicyFlags::SYSTEM_PARTITION; std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item) .Build(); @@ -565,7 +534,6 @@ TEST_F(TableMergerTest, SameResourceDifferentNameFail) { overlayable_item_first.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_first) .Build(); @@ -575,7 +543,6 @@ TEST_F(TableMergerTest, SameResourceDifferentNameFail) { overlayable_item_second.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_second) .Build(); @@ -594,7 +561,6 @@ TEST_F(TableMergerTest, SameResourceDifferentActorFail) { overlayable_item_first.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_first) .Build(); @@ -604,7 +570,6 @@ TEST_F(TableMergerTest, SameResourceDifferentActorFail) { overlayable_item_second.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_second) .Build(); @@ -623,7 +588,6 @@ TEST_F(TableMergerTest, SameResourceDifferentPoliciesFail) { overlayable_item_first.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_first) .Build(); @@ -633,7 +597,6 @@ TEST_F(TableMergerTest, SameResourceDifferentPoliciesFail) { overlayable_item_second.policies |= PolicyFlags::SIGNATURE; std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_second) .Build(); @@ -653,7 +616,6 @@ TEST_F(TableMergerTest, SameResourceSameOverlayable) { overlayable_item_first.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_first) .Build(); @@ -661,7 +623,6 @@ TEST_F(TableMergerTest, SameResourceSameOverlayable) { overlayable_item_second.policies |= PolicyFlags::PRODUCT_PARTITION; std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() - .SetPackageId("com.app.a", 0x7f) .SetOverlayable("bool/foo", overlayable_item_second) .Build(); diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 98ee63d2e5c6..de72334e65a9 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -197,9 +197,9 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName( std::unique_ptr<SymbolTable::Symbol> symbol = util::make_unique<SymbolTable::Symbol>(); symbol->is_public = (sr.entry->visibility.level == Visibility::Level::kPublic); - if (sr.package->id && sr.type->id && sr.entry->id) { - symbol->id = ResourceId(sr.package->id.value(), sr.type->id.value(), sr.entry->id.value()); - symbol->is_dynamic = (sr.package->id.value() == 0); + if (sr.entry->id) { + symbol->id = sr.entry->id.value(); + symbol->is_dynamic = (sr.entry->id.value().package_id() == 0); } if (name.type == ResourceType::kAttr || name.type == ResourceType::kAttrPrivate) { diff --git a/tools/aapt2/split/TableSplitter.cpp b/tools/aapt2/split/TableSplitter.cpp index 6a672717f38e..2f319b11e3b2 100644 --- a/tools/aapt2/split/TableSplitter.cpp +++ b/tools/aapt2/split/TableSplitter.cpp @@ -185,7 +185,7 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // Initialize all packages for splits. for (size_t idx = 0; idx < split_count; idx++) { ResourceTable* split_table = splits_[idx].get(); - split_table->CreatePackage(pkg->name, pkg->id); + split_table->FindOrCreatePackage(pkg->name); } for (auto& type : pkg->types) { @@ -241,10 +241,7 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { // not have actual values for each type/entry. ResourceTablePackage* split_pkg = split_table->FindPackage(pkg->name); ResourceTableType* split_type = split_pkg->FindOrCreateType(type->type); - if (!split_type->id) { - split_type->id = type->id; - split_type->visibility_level = type->visibility_level; - } + split_type->visibility_level = type->visibility_level; ResourceEntry* split_entry = split_type->FindOrCreateEntry(entry->name); if (!split_entry->id) { diff --git a/tools/aapt2/split/TableSplitter_test.cpp b/tools/aapt2/split/TableSplitter_test.cpp index cdf07386c70f..c6a2ff33979b 100644 --- a/tools/aapt2/split/TableSplitter_test.cpp +++ b/tools/aapt2/split/TableSplitter_test.cpp @@ -193,15 +193,23 @@ TEST(TableSplitterTest, SplitTableByConfigAndDensity) { ResourceTable table; const ResourceName foo = test::ParseNameOrDie("android:string/foo"); - ASSERT_TRUE(table.AddResource(foo, test::ParseConfigOrDie("land-hdpi"), {}, - util::make_unique<Id>(), - test::GetDiagnostics())); - ASSERT_TRUE(table.AddResource(foo, test::ParseConfigOrDie("land-xhdpi"), {}, - util::make_unique<Id>(), - test::GetDiagnostics())); - ASSERT_TRUE(table.AddResource(foo, test::ParseConfigOrDie("land-xxhdpi"), {}, - util::make_unique<Id>(), - test::GetDiagnostics())); + ASSERT_TRUE( + table.AddResource(NewResourceBuilder(foo) + .SetValue(util::make_unique<Id>(), test::ParseConfigOrDie("land-hdpi")) + .Build(), + test::GetDiagnostics())); + + ASSERT_TRUE( + table.AddResource(NewResourceBuilder(foo) + .SetValue(util::make_unique<Id>(), test::ParseConfigOrDie("land-xhdpi")) + .Build(), + test::GetDiagnostics())); + + ASSERT_TRUE(table.AddResource( + NewResourceBuilder(foo) + .SetValue(util::make_unique<Id>(), test::ParseConfigOrDie("land-xxhdpi")) + .Build(), + test::GetDiagnostics())); std::vector<SplitConstraints> constraints; constraints.push_back( diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index 9a93f2a7476c..4816596da487 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -34,13 +34,6 @@ using ::android::StringPiece; namespace aapt { namespace test { -ResourceTableBuilder& ResourceTableBuilder::SetPackageId(const StringPiece& package_name, - uint8_t id) { - ResourceTablePackage* package = table_->CreatePackage(package_name, id); - CHECK(package != nullptr); - return *this; -} - ResourceTableBuilder& ResourceTableBuilder::AddSimple(const StringPiece& name, const ResourceId& id) { return AddValue(name, id, util::make_unique<Id>()); @@ -118,8 +111,13 @@ ResourceTableBuilder& ResourceTableBuilder::AddValue(const StringPiece& name, const ResourceId& id, std::unique_ptr<Value> value) { ResourceName res_name = ParseNameOrDie(name); - CHECK(table_->AddResourceWithIdMangled(res_name, id, config, {}, std::move(value), - GetDiagnostics())); + NewResourceBuilder builder(res_name); + builder.SetValue(std::move(value), config).SetAllowMangled(true); + if (id.id != 0U) { + builder.SetId(id); + } + + CHECK(table_->AddResource(builder.Build(), GetDiagnostics())); return *this; } @@ -128,10 +126,13 @@ ResourceTableBuilder& ResourceTableBuilder::SetSymbolState(const StringPiece& na Visibility::Level level, bool allow_new) { ResourceName res_name = ParseNameOrDie(name); - Visibility visibility; - visibility.level = level; - CHECK(table_->SetVisibilityWithIdMangled(res_name, visibility, id, GetDiagnostics())); - CHECK(table_->SetAllowNewMangled(res_name, AllowNew{}, GetDiagnostics())); + NewResourceBuilder builder(res_name); + builder.SetVisibility({level}).SetAllowNew({}).SetAllowMangled(true); + if (id.id != 0U) { + builder.SetId(id); + } + + CHECK(table_->AddResource(builder.Build(), GetDiagnostics())); return *this; } @@ -139,7 +140,14 @@ ResourceTableBuilder& ResourceTableBuilder::SetOverlayable(const StringPiece& na const OverlayableItem& overlayable) { ResourceName res_name = ParseNameOrDie(name); - CHECK(table_->SetOverlayable(res_name, overlayable, GetDiagnostics())); + CHECK(table_->AddResource( + NewResourceBuilder(res_name).SetOverlayable(overlayable).SetAllowMangled(true).Build(), + GetDiagnostics())); + return *this; +} + +ResourceTableBuilder& ResourceTableBuilder::Add(NewResource&& res) { + CHECK(table_->AddResource(std::move(res), GetDiagnostics())); return *this; } diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index c971a1b47fd5..3ff955d65f24 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -39,7 +39,6 @@ class ResourceTableBuilder { public: ResourceTableBuilder() = default; - ResourceTableBuilder& SetPackageId(const android::StringPiece& package_name, uint8_t id); ResourceTableBuilder& AddSimple(const android::StringPiece& name, const ResourceId& id = {}); ResourceTableBuilder& AddSimple(const android::StringPiece& name, const android::ConfigDescription& config, @@ -75,6 +74,7 @@ class ResourceTableBuilder { Visibility::Level level, bool allow_new = false); ResourceTableBuilder& SetOverlayable(const android::StringPiece& name, const OverlayableItem& overlayable); + ResourceTableBuilder& Add(NewResource&& res); StringPool* string_pool(); std::unique_ptr<ResourceTable> Build(); |