diff options
31 files changed, 817 insertions, 464 deletions
diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index 20d017813cf7..8cf4de9167e5 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -1339,9 +1339,13 @@ struct ResTable_typeSpec // Number of uint32_t entry configuration masks that follow. uint32_t entryCount; - enum { + enum : uint32_t { // Additional flag indicating an entry is public. - SPEC_PUBLIC = 0x40000000 + SPEC_PUBLIC = 0x40000000u, + + // Additional flag indicating an entry is overlayable at runtime. + // Added in Android-P. + SPEC_OVERLAYABLE = 0x80000000u, }; }; diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index 0f6fb50aba48..5831875680ac 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -294,36 +294,42 @@ void Debug::PrintTable(const ResourceTable& table, const DebugPrintTableOptions& printer->Print("/"); printer->Print(entry->name); - switch (entry->symbol_status.state) { - case SymbolState::kPublic: + switch (entry->visibility.level) { + case Visibility::Level::kPublic: printer->Print(" PUBLIC"); break; - case SymbolState::kPrivate: + case Visibility::Level::kPrivate: printer->Print(" _PRIVATE_"); break; - case SymbolState::kUndefined: + case Visibility::Level::kUndefined: // Print nothing. break; } + if (entry->overlayable) { + printer->Print(" OVERLAYABLE"); + } + printer->Println(); - printer->Indent(); - for (const auto& value : entry->values) { - printer->Print("("); - printer->Print(value->config.to_string()); - printer->Print(") "); - value->value->Accept(&headline_printer); - if (options.show_sources && !value->value->GetSource().path.empty()) { - printer->Print(" src="); - printer->Print(value->value->GetSource().to_string()); - } - printer->Println(); + if (options.show_values) { printer->Indent(); - value->value->Accept(&body_printer); + for (const auto& value : entry->values) { + printer->Print("("); + printer->Print(value->config.to_string()); + printer->Print(") "); + value->value->Accept(&headline_printer); + if (options.show_sources && !value->value->GetSource().path.empty()) { + printer->Print(" src="); + printer->Print(value->value->GetSource().to_string()); + } + printer->Println(); + printer->Indent(); + value->value->Accept(&body_printer); + printer->Undent(); + } printer->Undent(); } - printer->Undent(); } printer->Undent(); } diff --git a/tools/aapt2/Debug.h b/tools/aapt2/Debug.h index 3c1ee4c0cdba..6209a04c3c2d 100644 --- a/tools/aapt2/Debug.h +++ b/tools/aapt2/Debug.h @@ -29,6 +29,7 @@ namespace aapt { struct DebugPrintTableOptions { bool show_sources = false; + bool show_values = true; }; struct Debug { diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 4cc60a8dbb07..24b28dd7d970 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -30,7 +30,7 @@ #include "util/Util.h" #include "xml/XmlPullParser.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { @@ -90,9 +90,12 @@ struct ParsedResource { ConfigDescription config; std::string product; Source source; + ResourceId id; - Maybe<SymbolState> symbol_state; + Visibility::Level visibility_level = Visibility::Level::kUndefined; bool allow_new = false; + bool overlayable = false; + std::string comment; std::unique_ptr<Value> value; std::list<ParsedResource> child_resources; @@ -106,24 +109,41 @@ static bool AddResourcesToTable(ResourceTable* table, IDiagnostics* diag, Parsed res->comment = trimmed_comment.to_string(); } - if (res->symbol_state) { - Symbol symbol; - symbol.state = res->symbol_state.value(); - symbol.source = res->source; - symbol.comment = res->comment; - symbol.allow_new = res->allow_new; - if (!table->SetSymbolState(res->name, res->id, symbol, diag)) { + 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; + } + } + + 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; } } - if (res->value) { + if (res->overlayable) { + Overlayable overlayable; + overlayable.source = res->source; + overlayable.comment = res->comment; + if (!table->SetOverlayable(res->name, overlayable, diag)) { + return false; + } + } + + 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)); - if (!table->AddResource(res->name, res->id, res->config, res->product, std::move(res->value), - diag)) { + if (!table->AddResourceWithId(res->name, res->id, res->config, res->product, + std::move(res->value), diag)) { return false; } } @@ -601,8 +621,7 @@ std::unique_ptr<Item> ResourceParser::ParseXml(xml::XmlPullParser* parser, // Process the raw value. std::unique_ptr<Item> processed_item = - ResourceUtils::TryParseItemForAttribute(raw_value, type_mask, - on_create_reference); + ResourceUtils::TryParseItemForAttribute(raw_value, type_mask, on_create_reference); if (processed_item) { // Fix up the reference. if (Reference* ref = ValueCast<Reference>(processed_item.get())) { @@ -689,8 +708,7 @@ bool ResourceParser::ParseString(xml::XmlPullParser* parser, return true; } -bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, - ParsedResource* out_resource) { +bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, ParsedResource* out_resource) { if (out_resource->config != ConfigDescription::DefaultConfig()) { diag_->Warn(DiagMessage(out_resource->source) << "ignoring configuration '" << out_resource->config << "' for <public> tag"); @@ -728,7 +746,7 @@ bool ResourceParser::ParsePublic(xml::XmlPullParser* parser, out_resource->value = util::make_unique<Id>(); } - out_resource->symbol_state = SymbolState::kPublic; + out_resource->visibility_level = Visibility::Level::kPublic; return true; } @@ -818,7 +836,7 @@ bool ResourceParser::ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource child_resource.id = next_id; child_resource.comment = std::move(comment); child_resource.source = item_source; - child_resource.symbol_state = SymbolState::kPublic; + child_resource.visibility_level = Visibility::Level::kPublic; out_resource->child_resources.push_back(std::move(child_resource)); next_id.id += 1; @@ -864,7 +882,7 @@ bool ResourceParser::ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out return false; } - out_resource->symbol_state = SymbolState::kPrivate; + out_resource->visibility_level = Visibility::Level::kPrivate; return true; } @@ -920,8 +938,12 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource continue; } - // TODO(b/64980941): Mark the symbol as overlayable and allow marking which entity can overlay - // the resource (system/app). + ParsedResource child_resource; + child_resource.name.type = *type; + child_resource.name.entry = maybe_name.value().to_string(); + child_resource.source = item_source; + child_resource.overlayable = true; + out_resource->child_resources.push_back(std::move(child_resource)); xml::XmlPullParser::SkipCurrentElement(parser); } else if (!ShouldIgnoreElement(element_namespace, element_name)) { @@ -932,10 +954,9 @@ bool ResourceParser::ParseOverlayable(xml::XmlPullParser* parser, ParsedResource return !error; } -bool ResourceParser::ParseAddResource(xml::XmlPullParser* parser, - ParsedResource* out_resource) { +bool ResourceParser::ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource) { if (ParseSymbolImpl(parser, out_resource)) { - out_resource->symbol_state = SymbolState::kUndefined; + out_resource->visibility_level = Visibility::Level::kUndefined; out_resource->allow_new = true; return true; } @@ -1395,9 +1416,8 @@ bool ResourceParser::ParseDeclareStyleable(xml::XmlPullParser* parser, ParsedResource* out_resource) { out_resource->name.type = ResourceType::kStyleable; - // Declare-styleable is kPrivate by default, because it technically only - // exists in R.java. - out_resource->symbol_state = SymbolState::kPublic; + // Declare-styleable is kPrivate by default, because it technically only exists in R.java. + out_resource->visibility_level = Visibility::Level::kPublic; // Declare-styleable only ends up in default config; if (out_resource->config != ConfigDescription::DefaultConfig()) { diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index 9a5cd3edb47f..618c8ed4afd1 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -29,8 +29,8 @@ using ::aapt::io::StringInputStream; using ::aapt::test::StrValueEq; using ::aapt::test::ValueEq; -using ::android::ResTable_map; using ::android::Res_value; +using ::android::ResTable_map; using ::android::StringPiece; using ::testing::Eq; using ::testing::IsEmpty; @@ -38,6 +38,7 @@ using ::testing::IsNull; using ::testing::NotNull; using ::testing::Pointee; using ::testing::SizeIs; +using ::testing::StrEq; namespace aapt { @@ -482,7 +483,7 @@ TEST_F(ResourceParserTest, ParseAttributesDeclareStyleable) { Maybe<ResourceTable::SearchResult> result = table_.FindResource(test::ParseNameOrDie("styleable/foo")); ASSERT_TRUE(result); - EXPECT_THAT(result.value().entry->symbol_status.state, Eq(SymbolState::kPublic)); + EXPECT_THAT(result.value().entry->visibility.level, Eq(Visibility::Level::kPublic)); Attribute* attr = test::GetValue<Attribute>(&table_, "attr/bar"); ASSERT_THAT(attr, NotNull()); @@ -718,6 +719,26 @@ TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) { EXPECT_THAT(actual_id, Eq(ResourceId(0x01010041))); } +TEST_F(ResourceParserTest, StrongestSymbolVisibilityWins) { + std::string input = R"( + <!-- private --> + <java-symbol type="string" name="foo" /> + <!-- public --> + <public type="string" name="foo" id="0x01020000" /> + <!-- private2 --> + <java-symbol type="string" name="foo" />)"; + ASSERT_TRUE(TestParse(input)); + + Maybe<ResourceTable::SearchResult> result = + table_.FindResource(test::ParseNameOrDie("string/foo")); + ASSERT_TRUE(result); + + ResourceEntry* entry = result.value().entry; + ASSERT_THAT(entry, NotNull()); + EXPECT_THAT(entry->visibility.level, Eq(Visibility::Level::kPublic)); + EXPECT_THAT(entry->visibility.comment, StrEq("public")); +} + TEST_F(ResourceParserTest, ExternalTypesShouldOnlyBeReferences) { ASSERT_TRUE(TestParse(R"(<item type="layout" name="foo">@layout/bar</item>)")); ASSERT_FALSE(TestParse(R"(<item type="layout" name="bar">"this is a string"</item>)")); @@ -731,8 +752,8 @@ TEST_F(ResourceParserTest, AddResourcesElementShouldAddEntryWithUndefinedSymbol) ASSERT_TRUE(result); const ResourceEntry* entry = result.value().entry; ASSERT_THAT(entry, NotNull()); - EXPECT_THAT(entry->symbol_status.state, Eq(SymbolState::kUndefined)); - EXPECT_TRUE(entry->symbol_status.allow_new); + EXPECT_THAT(entry->visibility.level, Eq(Visibility::Level::kUndefined)); + EXPECT_TRUE(entry->allow_new); } TEST_F(ResourceParserTest, ParseItemElementWithFormat) { @@ -833,6 +854,22 @@ TEST_F(ResourceParserTest, ParseOverlayableTagWithSystemPolicy) { <item type="string" name="bar" /> </overlayable>)"; ASSERT_TRUE(TestParse(input)); + + Maybe<ResourceTable::SearchResult> search_result = + table_.FindResource(test::ParseNameOrDie("string/bar")); + ASSERT_TRUE(search_result); + ASSERT_THAT(search_result.value().entry, NotNull()); + EXPECT_THAT(search_result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined)); + EXPECT_TRUE(search_result.value().entry->overlayable); +} + +TEST_F(ResourceParserTest, DuplicateOverlayableIsError) { + std::string input = R"( + <overlayable> + <item type="string" name="foo" /> + <item type="string" name="foo" /> + </overlayable>)"; + EXPECT_FALSE(TestParse(input)); } } // namespace aapt diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 0304e21698df..3172892d7172 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -22,6 +22,7 @@ #include <tuple> #include "android-base/logging.h" +#include "android-base/stringprintf.h" #include "androidfw/ResourceTypes.h" #include "ConfigDescription.h" @@ -33,6 +34,7 @@ using ::aapt::text::IsValidResourceEntryName; using ::android::StringPiece; +using ::android::base::StringPrintf; namespace aapt { @@ -45,7 +47,7 @@ static bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, const Stri return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0; } -ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) { +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>); @@ -55,7 +57,7 @@ ResourceTablePackage* ResourceTable::FindPackage(const StringPiece& name) { return nullptr; } -ResourceTablePackage* ResourceTable::FindPackageById(uint8_t id) { +ResourceTablePackage* ResourceTable::FindPackageById(uint8_t id) const { for (auto& package : packages) { if (package->id && package->id.value() == id) { return package.get(); @@ -206,30 +208,23 @@ std::vector<ResourceConfigValue*> ResourceEntry::FindValuesIf( return results; } -/** - * The default handler for collisions. - * - * Typically, a weak value will be overridden by a strong value. An existing - * weak - * value will not be overridden by an incoming weak value. - * - * There are some exceptions: - * - * Attributes: There are two types of Attribute values: USE and DECL. - * - * USE is anywhere an Attribute is declared without a format, and in a place - * that would - * be legal to declare if the Attribute already existed. This is typically in a - * <declare-styleable> tag. Attributes defined in a <declare-styleable> are also - * weak. - * - * DECL is an absolute declaration of an Attribute and specifies an explicit - * format. - * - * A DECL will override a USE without error. Two DECLs must match in their - * format for there to be - * no error. - */ +// The default handler for collisions. +// +// Typically, a weak value will be overridden by a strong value. An existing weak +// value will not be overridden by an incoming weak value. +// +// There are some exceptions: +// +// Attributes: There are two types of Attribute values: USE and DECL. +// +// USE is anywhere an Attribute is declared without a format, and in a place that would +// be legal to declare if the Attribute already existed. This is typically in a +// <declare-styleable> tag. Attributes defined in a <declare-styleable> are also weak. +// +// DECL is an absolute declaration of an Attribute and specifies an explicit format. +// +// A DECL will override a USE without error. Two DECLs must match in their format for there to be +// no error. ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* existing, Value* incoming) { Attribute* existing_attr = ValueCast<Attribute>(existing); @@ -287,14 +282,14 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist return CollisionResult::kConflict; } -static StringPiece ValidateName(const StringPiece& name) { +static StringPiece ResourceNameValidator(const StringPiece& name) { if (!IsValidResourceEntryName(name)) { return name; } return {}; } -static StringPiece SkipValidateName(const StringPiece& /*name*/) { +static StringPiece SkipNameValidator(const StringPiece& /*name*/) { return {}; } @@ -303,17 +298,14 @@ bool ResourceTable::AddResource(const ResourceNameRef& name, const StringPiece& product, std::unique_ptr<Value> value, IDiagnostics* diag) { - return AddResourceImpl(name, {}, config, product, std::move(value), ValidateName, + return AddResourceImpl(name, {}, config, product, std::move(value), ResourceNameValidator, ResolveValueCollision, diag); } -bool ResourceTable::AddResource(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), ValidateName, +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), ResourceNameValidator, ResolveValueCollision, diag); } @@ -322,14 +314,14 @@ bool ResourceTable::AddFileReference(const ResourceNameRef& name, const Source& source, const StringPiece& path, IDiagnostics* diag) { - return AddFileReferenceImpl(name, config, source, path, nullptr, ValidateName, diag); + return AddFileReferenceImpl(name, config, source, path, nullptr, ResourceNameValidator, diag); } -bool ResourceTable::AddFileReferenceAllowMangled( - const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const StringPiece& path, io::IFile* file, - IDiagnostics* diag) { - return AddFileReferenceImpl(name, config, source, path, file, SkipValidateName, diag); +bool ResourceTable::AddFileReferenceMangled(const ResourceNameRef& name, + const ConfigDescription& config, const Source& source, + const StringPiece& path, io::IFile* file, + IDiagnostics* diag) { + return AddFileReferenceImpl(name, config, source, path, file, SkipNameValidator, diag); } bool ResourceTable::AddFileReferenceImpl(const ResourceNameRef& name, @@ -344,88 +336,85 @@ bool ResourceTable::AddFileReferenceImpl(const ResourceNameRef& name, name_validator, ResolveValueCollision, diag); } -bool ResourceTable::AddResourceAllowMangled(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), SkipValidateName, +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, ResolveValueCollision, diag); } -bool ResourceTable::AddResourceAllowMangled(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), SkipValidateName, +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, ResolveValueCollision, diag); } +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; + } + return true; +} + 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& conflictResolver, + const CollisionResolverFunc& conflict_resolver, IDiagnostics* diag) { CHECK(value != nullptr); CHECK(diag != nullptr); - const StringPiece bad_char = name_validator(name.entry); - if (!bad_char.empty()) { - diag->Error(DiagMessage(value->GetSource()) << "resource '" << name - << "' has invalid entry name '" << name.entry - << "'. Invalid character '" << bad_char << "'"); - + const Source& source = value->GetSource(); + if (!ValidateName(name_validator, name, source, diag)) { return false; } ResourceTablePackage* package = FindOrCreatePackage(name.package); if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) { - diag->Error(DiagMessage(value->GetSource()) - << "trying to add resource '" << name << "' with ID " << res_id - << " but package '" << package->name << "' already has ID " - << std::hex << (int)package->id.value() << std::dec); + 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; } ResourceTableType* type = package->FindOrCreateType(name.type); if (res_id.is_valid_dynamic() && type->id && type->id.value() != res_id.type_id()) { - diag->Error(DiagMessage(value->GetSource()) - << "trying to add resource '" << name << "' with ID " << res_id - << " but type '" << type->type << "' already has ID " - << std::hex << (int)type->id.value() << std::dec); + 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); if (res_id.is_valid_dynamic() && entry->id && entry->id.value() != res_id.entry_id()) { - diag->Error(DiagMessage(value->GetSource()) + 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())); + << ResourceId(package->id.value(), type->id.value(), entry->id.value())); return false; } ResourceConfigValue* config_value = entry->FindOrCreateValue(config, product); - if (!config_value->value) { + if (config_value->value == nullptr) { // Resource does not exist, add it now. config_value->value = std::move(value); - } else { - switch (conflictResolver(config_value->value.get(), value.get())) { + switch (conflict_resolver(config_value->value.get(), value.get())) { case CollisionResult::kTakeNew: // Take the incoming value. config_value->value = std::move(value); break; case CollisionResult::kConflict: - diag->Error(DiagMessage(value->GetSource()) - << "duplicate value for resource '" << name << "' " - << "with config '" << config << "'"); - diag->Error(DiagMessage(config_value->value->GetSource()) - << "resource previously defined here"); + 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: @@ -441,52 +430,56 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI return true; } -bool ResourceTable::SetSymbolState(const ResourceNameRef& name, const ResourceId& res_id, - const Symbol& symbol, IDiagnostics* diag) { - return SetSymbolStateImpl(name, res_id, symbol, ValidateName, diag); +bool ResourceTable::SetVisibility(const ResourceNameRef& name, const Visibility& visibility, + IDiagnostics* diag) { + return SetVisibilityImpl(name, visibility, ResourceId{}, ResourceNameValidator, diag); } -bool ResourceTable::SetSymbolStateAllowMangled(const ResourceNameRef& name, - const ResourceId& res_id, - const Symbol& symbol, - IDiagnostics* diag) { - return SetSymbolStateImpl(name, res_id, symbol, SkipValidateName, diag); +bool ResourceTable::SetVisibilityMangled(const ResourceNameRef& name, const Visibility& visibility, + IDiagnostics* diag) { + return SetVisibilityImpl(name, visibility, ResourceId{}, SkipNameValidator, diag); } -bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const ResourceId& res_id, - const Symbol& symbol, NameValidator name_validator, - IDiagnostics* 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 StringPiece bad_char = name_validator(name.entry); - if (!bad_char.empty()) { - diag->Error(DiagMessage(symbol.source) << "resource '" << name << "' has invalid entry name '" - << name.entry << "'. Invalid character '" << bad_char - << "'"); + const Source& source = visibility.source; + if (!ValidateName(name_validator, name, source, diag)) { return false; } ResourceTablePackage* package = FindOrCreatePackage(name.package); if (res_id.is_valid_dynamic() && package->id && package->id.value() != res_id.package_id()) { - diag->Error(DiagMessage(symbol.source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but package '" << package->name << "' already has ID " - << std::hex << (int)package->id.value() << std::dec); + 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; } ResourceTableType* type = package->FindOrCreateType(name.type); if (res_id.is_valid_dynamic() && type->id && type->id.value() != res_id.type_id()) { - diag->Error(DiagMessage(symbol.source) - << "trying to add resource '" << name << "' with ID " << res_id - << " but type '" << type->type << "' already has ID " - << std::hex << (int)type->id.value() << std::dec); + 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); if (res_id.is_valid_dynamic() && entry->id && entry->id.value() != res_id.entry_id()) { - diag->Error(DiagMessage(symbol.source) + 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())); @@ -499,48 +492,96 @@ bool ResourceTable::SetSymbolStateImpl(const ResourceNameRef& name, const Resour entry->id = res_id.entry_id(); } - // Only mark the type state as public, it doesn't care about being private. - if (symbol.state == SymbolState::kPublic) { - type->symbol_status.state = SymbolState::kPublic; + // 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 (symbol.allow_new) { - // This symbol can be added as a new resource when merging (if it belongs to an overlay). - entry->symbol_status.allow_new = true; - } - - if (symbol.state == SymbolState::kUndefined && - entry->symbol_status.state != SymbolState::kUndefined) { + 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 (symbol.state == SymbolState::kPrivate && - entry->symbol_status.state == SymbolState::kPublic) { + if (visibility.level < entry->visibility.level) { // We can't downgrade public to private. Ignore. return true; } // This symbol definition takes precedence, replace. - entry->symbol_status.state = symbol.state; - entry->symbol_status.source = symbol.source; - entry->symbol_status.comment = symbol.comment; + 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; + } + + ResourceTablePackage* package = FindOrCreatePackage(name.package); + ResourceTableType* type = package->FindOrCreateType(name.type); + ResourceEntry* entry = type->FindOrCreateEntry(name.entry); + entry->allow_new = allow_new; return true; } -Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name) { +bool ResourceTable::SetOverlayable(const ResourceNameRef& name, const Overlayable& overlayable, + IDiagnostics* diag) { + return SetOverlayableImpl(name, overlayable, ResourceNameValidator, diag); +} + +bool ResourceTable::SetOverlayableMangled(const ResourceNameRef& name, + const Overlayable& overlayable, IDiagnostics* diag) { + return SetOverlayableImpl(name, overlayable, SkipNameValidator, diag); +} + +bool ResourceTable::SetOverlayableImpl(const ResourceNameRef& name, const Overlayable& overlayable, + NameValidator name_validator, IDiagnostics* diag) { + CHECK(diag != nullptr); + + if (!ValidateName(name_validator, name, overlayable.source, diag)) { + return false; + } + + ResourceTablePackage* package = FindOrCreatePackage(name.package); + ResourceTableType* type = package->FindOrCreateType(name.type); + ResourceEntry* entry = type->FindOrCreateEntry(name.entry); + if (entry->overlayable) { + diag->Error(DiagMessage(overlayable.source) + << "duplicate overlayable declaration for resource '" << name << "'"); + diag->Error(DiagMessage(entry->overlayable.value().source) << "previous declaration here"); + return false; + } + entry->overlayable = overlayable; + return true; +} + +Maybe<ResourceTable::SearchResult> ResourceTable::FindResource(const ResourceNameRef& name) const { ResourceTablePackage* package = FindPackage(name.package); - if (!package) { + if (package == nullptr) { return {}; } ResourceTableType* type = package->FindType(name.type); - if (!type) { + if (type == nullptr) { return {}; } ResourceEntry* entry = type->FindEntry(name.entry); - if (!entry) { + if (entry == nullptr) { return {}; } return SearchResult{package, type, entry}; @@ -552,23 +593,20 @@ std::unique_ptr<ResourceTable> ResourceTable::Clone() const { ResourceTablePackage* new_pkg = new_table->CreatePackage(pkg->name, pkg->id); for (const auto& type : pkg->types) { ResourceTableType* new_type = new_pkg->FindOrCreateType(type->type); - if (!new_type->id) { - new_type->id = type->id; - new_type->symbol_status = type->symbol_status; - } + 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); - if (!new_entry->id) { - new_entry->id = entry->id; - new_entry->symbol_status = entry->symbol_status; - } + new_entry->id = entry->id; + new_entry->visibility = entry->visibility; + new_entry->allow_new = entry->allow_new; + new_entry->overlayable = entry->overlayable; for (const auto& config_value : entry->values) { ResourceConfigValue* new_value = new_entry->FindOrCreateValue(config_value->config, config_value->product); - Value* value = config_value->value->Clone(&new_table->string_pool); - new_value->value = std::unique_ptr<Value>(value); + new_value->value.reset(config_value->value->Clone(&new_table->string_pool)); } } } diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index d5db67e77f51..eaa2d0b8af7d 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -38,40 +38,40 @@ namespace aapt { -enum class SymbolState { - kUndefined, - kPrivate, - kPublic, -}; +// The Public status of a resource. +struct Visibility { + enum class Level { + kUndefined, + kPrivate, + kPublic, + }; -/** - * The Public status of a resource. - */ -struct Symbol { - SymbolState state = SymbolState::kUndefined; + Level level = Level::kUndefined; Source source; + std::string comment; +}; - // Whether this entry (originating from an overlay) can be added as a new resource. - bool allow_new = false; +// Represents <add-resource> in an overlay. +struct AllowNew { + Source source; + std::string comment; +}; +// The policy dictating whether an entry is overlayable at runtime by RROs. +struct Overlayable { + Source source; std::string comment; }; class ResourceConfigValue { public: - /** - * The configuration for which this value is defined. - */ + // The configuration for which this value is defined. const ConfigDescription config; - /** - * The product for which this value is defined. - */ + // The product for which this value is defined. const std::string product; - /** - * The actual Value. - */ + // The actual Value. std::unique_ptr<Value> value; ResourceConfigValue(const ConfigDescription& config, const android::StringPiece& product) @@ -87,27 +87,21 @@ class ResourceConfigValue { */ class ResourceEntry { public: - /** - * The name of the resource. Immutable, as - * this determines the order of this resource - * when doing lookups. - */ + // The name of the resource. Immutable, as this determines the order of this resource + // when doing lookups. const std::string name; - /** - * The entry ID for this resource. - */ + // The entry ID for this resource (the EEEE in 0xPPTTEEEE). Maybe<uint16_t> id; - /** - * Whether this resource is public (and must maintain the same entry ID across - * builds). - */ - Symbol symbol_status; + // Whether this resource is public (and must maintain the same entry ID across builds). + Visibility visibility; + + Maybe<AllowNew> allow_new; - /** - * The resource's values for each configuration. - */ + Maybe<Overlayable> overlayable; + + // The resource's values for each configuration. std::vector<std::unique_ptr<ResourceConfigValue>> values; explicit ResourceEntry(const android::StringPiece& name) : name(name.to_string()) {} @@ -125,31 +119,19 @@ class ResourceEntry { DISALLOW_COPY_AND_ASSIGN(ResourceEntry); }; -/** - * Represents a resource type, which holds entries defined - * for this type. - */ +// Represents a resource type (eg. string, drawable, layout, etc.) containing resource entries. class ResourceTableType { public: - /** - * The logical type of resource (string, drawable, layout, etc.). - */ + // The logical type of resource (string, drawable, layout, etc.). const ResourceType type; - /** - * The type ID for this resource. - */ + // 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). - */ - Symbol symbol_status; + // Whether this type is public (and must maintain the same type ID across builds). + Visibility::Level visibility_level = Visibility::Level::kUndefined; - /** - * List of resources for this type. - */ + // List of resources for this type. std::vector<std::unique_ptr<ResourceEntry>> entries; explicit ResourceTableType(const ResourceType type) : type(type) {} @@ -163,9 +145,11 @@ class ResourceTableType { class ResourceTablePackage { public: - Maybe<uint8_t> id; std::string name; + // The package ID (the PP in 0xPPTTEEEE). + Maybe<uint8_t> id; + std::vector<std::unique_ptr<ResourceTableType>> types; ResourceTablePackage() = default; @@ -176,10 +160,7 @@ class ResourceTablePackage { DISALLOW_COPY_AND_ASSIGN(ResourceTablePackage); }; -/** - * The container and index for all resources defined for an app. This gets - * flattened into a binary resource table (resources.arsc). - */ +// The container and index for all resources defined for an app. class ResourceTable { public: ResourceTable() = default; @@ -188,47 +169,51 @@ class ResourceTable { using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>; - /** - * When a collision of resources occurs, this method decides which value to - * keep. - */ + // When a collision of resources occurs, this method decides which value to keep. static CollisionResult ResolveValueCollision(Value* existing, Value* incoming); bool AddResource(const ResourceNameRef& name, const ConfigDescription& config, const android::StringPiece& product, std::unique_ptr<Value> value, IDiagnostics* diag); - bool AddResource(const ResourceNameRef& name, const ResourceId& res_id, - const ConfigDescription& config, const android::StringPiece& product, - std::unique_ptr<Value> value, IDiagnostics* diag); + bool AddResourceWithId(const ResourceNameRef& name, const ResourceId& res_id, + const ConfigDescription& config, const android::StringPiece& product, + std::unique_ptr<Value> value, IDiagnostics* diag); bool AddFileReference(const ResourceNameRef& name, const ConfigDescription& config, const Source& source, const android::StringPiece& path, IDiagnostics* diag); - bool AddFileReferenceAllowMangled(const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const android::StringPiece& path, - io::IFile* file, IDiagnostics* diag); - - /** - * 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 AddResourceAllowMangled(const ResourceNameRef& name, const ConfigDescription& config, - const android::StringPiece& product, std::unique_ptr<Value> value, - IDiagnostics* diag); - - bool AddResourceAllowMangled(const ResourceNameRef& name, const ResourceId& id, - const ConfigDescription& config, const android::StringPiece& product, - std::unique_ptr<Value> value, IDiagnostics* diag); - - bool SetSymbolState(const ResourceNameRef& name, const ResourceId& res_id, - const Symbol& symbol, IDiagnostics* diag); - - bool SetSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId& res_id, - const Symbol& symbol, IDiagnostics* diag); + bool AddFileReferenceMangled(const ResourceNameRef& name, const ConfigDescription& config, + const Source& source, const android::StringPiece& path, + io::IFile* file, IDiagnostics* diag); + + // 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 ConfigDescription& config, + const android::StringPiece& product, std::unique_ptr<Value> value, + IDiagnostics* diag); + + bool AddResourceWithIdMangled(const ResourceNameRef& name, const ResourceId& id, + const ConfigDescription& config, + const android::StringPiece& product, std::unique_ptr<Value> value, + IDiagnostics* diag); + + bool SetVisibility(const ResourceNameRef& name, const Visibility& visibility, IDiagnostics* diag); + bool SetVisibilityMangled(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); + + bool SetOverlayable(const ResourceNameRef& name, const Overlayable& overlayable, + IDiagnostics* diag); + bool SetOverlayableMangled(const ResourceNameRef& name, const Overlayable& overlayable, + IDiagnostics* diag); + + bool SetAllowNew(const ResourceNameRef& name, const AllowNew& allow_new, IDiagnostics* diag); + bool SetAllowNewMangled(const ResourceNameRef& name, const AllowNew& allow_new, + IDiagnostics* diag); struct SearchResult { ResourceTablePackage* package; @@ -236,40 +221,28 @@ class ResourceTable { ResourceEntry* entry; }; - Maybe<SearchResult> FindResource(const ResourceNameRef& name); + Maybe<SearchResult> FindResource(const ResourceNameRef& name) 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); + // 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); + ResourceTablePackage* FindPackageById(uint8_t id) const; ResourceTablePackage* CreatePackage(const android::StringPiece& name, Maybe<uint8_t> id = {}); std::unique_ptr<ResourceTable> Clone() const; - /** - * 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. - * When `string_pool` references are destroyed (as they will be when - * `packages` - * is destroyed), they decrement a refCount, which would cause invalid - * memory access if the pool was already destroyed. - */ + // 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. + // When `string_pool` references are destroyed (as they will be when `packages` is destroyed), + // they decrement a refCount, which would cause invalid memory access if the pool was already + // destroyed. StringPool string_pool; - /** - * The list of packages in this table, sorted alphabetically by package name. - */ + // The list of packages in this table, sorted alphabetically by package name. std::vector<std::unique_ptr<ResourceTablePackage>> packages; // Set of dynamic packages that this table may reference. Their package names get encoded @@ -284,6 +257,9 @@ class ResourceTable { 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 ConfigDescription& config, const android::StringPiece& product, std::unique_ptr<Value> value, NameValidator name_validator, @@ -293,8 +269,19 @@ class ResourceTable { const Source& source, const android::StringPiece& path, io::IFile* file, NameValidator name_validator, 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 Overlayable& overlayable, + NameValidator name_validator, IDiagnostics* diag); + bool SetSymbolStateImpl(const ResourceNameRef& name, const ResourceId& res_id, - const Symbol& symbol, NameValidator name_validator, IDiagnostics* diag); + const Visibility& symbol, NameValidator name_validator, + IDiagnostics* diag); DISALLOW_COPY_AND_ASSIGN(ResourceTable); }; diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp index 2a3c131f7b4b..eb75f947e0be 100644 --- a/tools/aapt2/ResourceTable_test.cpp +++ b/tools/aapt2/ResourceTable_test.cpp @@ -24,7 +24,10 @@ #include <ostream> #include <string> +using ::android::StringPiece; +using ::testing::Eq; using ::testing::NotNull; +using ::testing::StrEq; namespace aapt { @@ -45,7 +48,7 @@ TEST(ResourceTableTest, FailToAddResourceWithBadName) { TEST(ResourceTableTest, AddResourceWithWeirdNameWhenAddingMangledResources) { ResourceTable table; - EXPECT_TRUE(table.AddResourceAllowMangled( + EXPECT_TRUE(table.AddResourceMangled( test::ParseNameOrDie("android:id/heythere "), ConfigDescription{}, "", test::ValueBuilder<Id>().SetSource("test.xml", 21u).Build(), test::GetDiagnostics())); } @@ -141,4 +144,104 @@ TEST(ResourceTableTest, ProductVaryingValues) { EXPECT_EQ(std::string("tablet"), values[1]->product); } +static StringPiece LevelToString(Visibility::Level level) { + switch (level) { + case Visibility::Level::kPrivate: + return "private"; + case Visibility::Level::kPublic: + return "private"; + default: + return "undefined"; + } +} + +static ::testing::AssertionResult VisibilityOfResource(const ResourceTable& table, + const ResourceNameRef& name, + Visibility::Level level, + const StringPiece& comment) { + Maybe<ResourceTable::SearchResult> result = table.FindResource(name); + if (!result) { + return ::testing::AssertionFailure() << "no resource '" << name << "' found in table"; + } + + const Visibility& visibility = result.value().entry->visibility; + if (visibility.level != level) { + return ::testing::AssertionFailure() << "expected visibility " << LevelToString(level) + << " but got " << LevelToString(visibility.level); + } + + if (visibility.comment != comment) { + return ::testing::AssertionFailure() << "expected visibility comment '" << comment + << "' but got '" << visibility.comment << "'"; + } + return ::testing::AssertionSuccess(); +} + +TEST(ResourceTableTest, SetVisibility) { + using Level = Visibility::Level; + + ResourceTable table; + const ResourceName name = test::ParseNameOrDie("android:string/foo"); + + Visibility visibility; + visibility.level = Visibility::Level::kPrivate; + visibility.comment = "private"; + ASSERT_TRUE(table.SetVisibility(name, visibility, 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(VisibilityOfResource(table, name, Level::kPrivate, "private")); + + visibility.level = Visibility::Level::kPublic; + visibility.comment = "public"; + ASSERT_TRUE(table.SetVisibility(name, visibility, 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(VisibilityOfResource(table, name, Level::kPublic, "public")); +} + +TEST(ResourceTableTest, SetAllowNew) { + ResourceTable table; + const ResourceName name = test::ParseNameOrDie("android:string/foo"); + + AllowNew allow_new; + Maybe<ResourceTable::SearchResult> result; + + allow_new.comment = "first"; + ASSERT_TRUE(table.SetAllowNew(name, allow_new, 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())); + result = table.FindResource(name); + ASSERT_TRUE(result); + ASSERT_TRUE(result.value().entry->allow_new); + ASSERT_THAT(result.value().entry->allow_new.value().comment, StrEq("second")); +} + +TEST(ResourceTableTest, SetOverlayable) { + ResourceTable table; + const ResourceName name = test::ParseNameOrDie("android:string/foo"); + + Overlayable overlayable; + + overlayable.comment = "first"; + ASSERT_TRUE(table.SetOverlayable(name, overlayable, test::GetDiagnostics())); + Maybe<ResourceTable::SearchResult> result = table.FindResource(name); + ASSERT_TRUE(result); + ASSERT_TRUE(result.value().entry->overlayable); + ASSERT_THAT(result.value().entry->overlayable.value().comment, StrEq("first")); + + overlayable.comment = "second"; + ASSERT_FALSE(table.SetOverlayable(name, overlayable, test::GetDiagnostics())); +} + } // namespace aapt diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto index 7e7c86d53d24..8552195d338d 100644 --- a/tools/aapt2/Resources.proto +++ b/tools/aapt2/Resources.proto @@ -93,11 +93,10 @@ message Type { repeated Entry entry = 3; } -// The status of a symbol/entry. This contains information like visibility (public/private), -// comments, and whether the entry can be overridden. -message SymbolStatus { +// The Visibility of a symbol/entry (public, private, undefined). +message Visibility { // The visibility of the resource outside of its package. - enum Visibility { + enum Level { // No visibility was explicitly specified. This is typically treated as private. // The distinction is important when two separate R.java files are generated: a public and // private one. An unknown visibility, in this case, would cause the resource to be omitted @@ -115,17 +114,32 @@ message SymbolStatus { PUBLIC = 2; } - Visibility visibility = 1; + Level level = 1; // The path at which this entry's visibility was defined (eg. public.xml). Source source = 2; // The comment associated with the <public> tag. string comment = 3; +} + +// Whether a resource comes from a compile-time overlay and is explicitly allowed to not overlay an +// existing resource. +message AllowNew { + // Where this was defined in source. + Source source = 1; - // Whether the symbol can be merged into another resource table without there being an existing - // definition to override. Used for overlays and set to true when <add-resource> is specified. - bool allow_new = 4; + // Any comment associated with the declaration. + string comment = 2; +} + +// Whether a resource is overlayable by runtime resource overlays (RRO). +message Overlayable { + // Where this declaration was defined in source. + Source source = 1; + + // Any comment associated with the declaration. + string comment = 2; } // An entry ID in the range [0x0000, 0xffff]. @@ -147,12 +161,19 @@ message Entry { // form package:type/entry. string name = 2; - // The symbol status of this entry, which includes visibility information. - SymbolStatus symbol_status = 3; + // The visibility of this entry (public, private, undefined). + Visibility visibility = 3; + + // Whether this resource, when originating from a compile-time overlay, is allowed to NOT overlay + // any existing resources. + AllowNew allow_new = 4; + + // Whether this resource can be overlaid by a runtime resource overlay (RRO). + Overlayable overlayable = 5; // The set of values defined for this entry, each corresponding to a different // configuration/variant. - repeated ConfigValue config_value = 4; + repeated ConfigValue config_value = 6; } // A Configuration/Value pair. diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp index fc1f1d6342ae..12113ed8a48a 100644 --- a/tools/aapt2/cmd/Diff.cpp +++ b/tools/aapt2/cmd/Diff.cpp @@ -75,14 +75,14 @@ static void EmitDiffLine(const Source& source, const StringPiece& message) { std::cerr << source << ": " << message << "\n"; } -static bool IsSymbolVisibilityDifferent(const Symbol& symbol_a, const Symbol& symbol_b) { - return symbol_a.state != symbol_b.state; +static bool IsSymbolVisibilityDifferent(const Visibility& vis_a, const Visibility& vis_b) { + return vis_a.level != vis_b.level; } template <typename Id> -static bool IsIdDiff(const Symbol& symbol_a, const Maybe<Id>& id_a, const Symbol& symbol_b, - const Maybe<Id>& id_b) { - if (symbol_a.state == SymbolState::kPublic || symbol_b.state == SymbolState::kPublic) { +static bool IsIdDiff(const Visibility::Level& level_a, const Maybe<Id>& id_a, + const Visibility::Level& level_b, const Maybe<Id>& id_b) { + if (level_a == Visibility::Level::kPublic || level_b == Visibility::Level::kPublic) { return id_a != id_b; } return false; @@ -157,17 +157,17 @@ static bool EmitResourceTypeDiff(IAaptContext* context, LoadedApk* apk_a, EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; } else { - if (IsSymbolVisibilityDifferent(entry_a->symbol_status, entry_b->symbol_status)) { + if (IsSymbolVisibilityDifferent(entry_a->visibility, entry_b->visibility)) { std::stringstream str_stream; str_stream << pkg_a->name << ":" << type_a->type << "/" << entry_a->name << " has different visibility ("; - if (entry_b->symbol_status.state == SymbolState::kPublic) { + if (entry_b->visibility.level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; } str_stream << " vs "; - if (entry_a->symbol_status.state == SymbolState::kPublic) { + if (entry_a->visibility.level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; @@ -175,7 +175,7 @@ static bool EmitResourceTypeDiff(IAaptContext* context, LoadedApk* apk_a, str_stream << ")"; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; - } else if (IsIdDiff(entry_a->symbol_status, entry_a->id, entry_b->symbol_status, + } 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 @@ -225,16 +225,16 @@ static bool EmitResourcePackageDiff(IAaptContext* context, LoadedApk* apk_a, EmitDiffLine(apk_a->GetSource(), str_stream.str()); diff = true; } else { - if (IsSymbolVisibilityDifferent(type_a->symbol_status, type_b->symbol_status)) { + if (type_a->visibility_level != type_b->visibility_level) { std::stringstream str_stream; str_stream << pkg_a->name << ":" << type_a->type << " has different visibility ("; - if (type_b->symbol_status.state == SymbolState::kPublic) { + if (type_b->visibility_level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; } str_stream << " vs "; - if (type_a->symbol_status.state == SymbolState::kPublic) { + if (type_a->visibility_level == Visibility::Level::kPublic) { str_stream << "PUBLIC"; } else { str_stream << "PRIVATE"; @@ -242,7 +242,8 @@ static bool EmitResourcePackageDiff(IAaptContext* context, LoadedApk* apk_a, str_stream << ")"; EmitDiffLine(apk_b->GetSource(), str_stream.str()); diff = true; - } else if (IsIdDiff(type_a->symbol_status, type_a->id, type_b->symbol_status, type_b->id)) { + } else if (IsIdDiff(type_a->visibility_level, type_a->id, type_b->visibility_level, + type_b->id)) { std::stringstream str_stream; str_stream << pkg_a->name << ":" << type_a->type << " has different public ID ("; if (type_b->id) { diff --git a/tools/aapt2/cmd/Dump.cpp b/tools/aapt2/cmd/Dump.cpp index bc7f5a86b043..3d2fb556cf59 100644 --- a/tools/aapt2/cmd/Dump.cpp +++ b/tools/aapt2/cmd/Dump.cpp @@ -69,15 +69,13 @@ static void DumpCompiledFile(const ResourceFile& file, const Source& source, off printer->Println(StringPrintf("Data: offset=%" PRIi64 " length=%zd", offset, len)); } -static bool TryDumpFile(IAaptContext* context, const std::string& file_path) { +static bool TryDumpFile(IAaptContext* context, const std::string& file_path, + const DebugPrintTableOptions& print_options) { // Use a smaller buffer so that there is less latency for dumping to stdout. constexpr size_t kStdOutBufferSize = 1024u; io::FileOutputStream fout(STDOUT_FILENO, kStdOutBufferSize); Printer printer(&fout); - DebugPrintTableOptions print_options; - print_options.show_sources = true; - std::string err; std::unique_ptr<io::ZipFileCollection> zip = io::ZipFileCollection::Create(file_path, &err); if (zip) { @@ -244,7 +242,12 @@ class DumpContext : public IAaptContext { // Entry point for dump command. int Dump(const std::vector<StringPiece>& args) { bool verbose = false; - Flags flags = Flags().OptionalSwitch("-v", "increase verbosity of output", &verbose); + bool no_values = false; + Flags flags = Flags() + .OptionalSwitch("--no-values", + "Suppresses output of values when displaying resource tables.", + &no_values) + .OptionalSwitch("-v", "increase verbosity of output", &verbose); if (!flags.Parse("aapt2 dump", args, &std::cerr)) { return 1; } @@ -252,8 +255,11 @@ int Dump(const std::vector<StringPiece>& args) { DumpContext context; context.SetVerbose(verbose); + DebugPrintTableOptions dump_table_options; + dump_table_options.show_sources = true; + dump_table_options.show_values = !no_values; for (const std::string& arg : flags.GetArgs()) { - if (!TryDumpFile(&context, arg)) { + if (!TryDumpFile(&context, arg, dump_table_options)) { return 1; } } diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index d782de55f66a..becbfc255ed1 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -631,9 +631,9 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv dst_path = ResourceUtils::BuildResourceFileName(doc->file, context_->GetNameMangler()); - bool result = table->AddFileReferenceAllowMangled(doc->file.name, doc->file.config, - doc->file.source, dst_path, nullptr, - context_->GetDiagnostics()); + bool result = + table->AddFileReferenceMangled(doc->file.name, doc->file.config, doc->file.source, + dst_path, nullptr, context_->GetDiagnostics()); if (!result) { return false; } @@ -1343,9 +1343,9 @@ class LinkCommand { std::unique_ptr<Id> id = util::make_unique<Id>(); id->SetSource(source.WithLine(exported_symbol.line)); - bool result = final_table_.AddResourceAllowMangled( - res_name, ConfigDescription::DefaultConfig(), std::string(), std::move(id), - context_->GetDiagnostics()); + bool result = + final_table_.AddResourceMangled(res_name, ConfigDescription::DefaultConfig(), + std::string(), std::move(id), context_->GetDiagnostics()); if (!result) { return false; } diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index 5078678e08a3..8d079ffecb63 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -223,7 +223,7 @@ bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) { break; case android::RES_TABLE_TYPE_SPEC_TYPE: - if (!ParseTypeSpec(parser.chunk())) { + if (!ParseTypeSpec(package, parser.chunk())) { return false; } break; @@ -260,7 +260,8 @@ bool BinaryResourceParser::ParsePackage(const ResChunk_header* chunk) { return true; } -bool BinaryResourceParser::ParseTypeSpec(const ResChunk_header* chunk) { +bool BinaryResourceParser::ParseTypeSpec(const ResourceTablePackage* package, + const ResChunk_header* chunk) { if (type_pool_.getError() != NO_ERROR) { diag_->Error(DiagMessage(source_) << "missing type string pool"); return false; @@ -276,6 +277,34 @@ bool BinaryResourceParser::ParseTypeSpec(const ResChunk_header* chunk) { diag_->Error(DiagMessage(source_) << "ResTable_typeSpec has invalid id: " << type_spec->id); return false; } + + // The data portion of this chunk contains entry_count 32bit entries, + // each one representing a set of flags. + const size_t entry_count = dtohl(type_spec->entryCount); + + // There can only be 2^16 entries in a type, because that is the ID + // space for entries (EEEE) in the resource ID 0xPPTTEEEE. + if (entry_count > std::numeric_limits<uint16_t>::max()) { + diag_->Error(DiagMessage(source_) + << "ResTable_typeSpec has too many entries (" << entry_count << ")"); + return false; + } + + const size_t data_size = util::DeviceToHost32(type_spec->header.size) - + util::DeviceToHost16(type_spec->header.headerSize); + if (entry_count * sizeof(uint32_t) > data_size) { + diag_->Error(DiagMessage(source_) << "ResTable_typeSpec too small to hold entries."); + return false; + } + + // Record the type_spec_flags for later. We don't know resource names yet, and we need those + // to mark resources as overlayable. + 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)); + entry_type_spec_flags_[id] = util::DeviceToHost32(type_spec_flags[i]); + } return true; } @@ -346,18 +375,34 @@ bool BinaryResourceParser::ParseType(const ResourceTablePackage* package, return false; } - if (!table_->AddResourceAllowMangled(name, res_id, config, {}, std::move(resource_value), - diag_)) { + if (!table_->AddResourceWithIdMangled(name, res_id, config, {}, std::move(resource_value), + diag_)) { return false; } - if ((entry->flags & ResTable_entry::FLAG_PUBLIC) != 0) { - Symbol symbol; - symbol.state = SymbolState::kPublic; - symbol.source = source_.WithLine(0); - if (!table_->SetSymbolStateAllowMangled(name, res_id, symbol, diag_)) { - return false; + const uint32_t type_spec_flags = entry_type_spec_flags_[res_id]; + if ((entry->flags & ResTable_entry::FLAG_PUBLIC) != 0 || + (type_spec_flags & ResTable_typeSpec::SPEC_OVERLAYABLE) != 0) { + if (entry->flags & ResTable_entry::FLAG_PUBLIC) { + Visibility visibility; + visibility.level = Visibility::Level::kPublic; + visibility.source = source_.WithLine(0); + if (!table_->SetVisibilityWithIdMangled(name, visibility, res_id, diag_)) { + return false; + } + } + + if (type_spec_flags & ResTable_typeSpec::SPEC_OVERLAYABLE) { + Overlayable overlayable; + overlayable.source = source_.WithLine(0); + if (!table_->SetOverlayableMangled(name, overlayable, diag_)) { + return false; + } } + + // Erase the ID from the map once processed, so that we don't mark the same symbol more than + // once. + entry_type_spec_flags_.erase(res_id); } // Add this resource name->id mapping to the index so diff --git a/tools/aapt2/format/binary/BinaryResourceParser.h b/tools/aapt2/format/binary/BinaryResourceParser.h index 052f806e3b95..a1f9f83edfb6 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.h +++ b/tools/aapt2/format/binary/BinaryResourceParser.h @@ -50,7 +50,7 @@ class BinaryResourceParser { bool ParseTable(const android::ResChunk_header* chunk); bool ParsePackage(const android::ResChunk_header* chunk); - bool ParseTypeSpec(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 ParseLibrary(const android::ResChunk_header* chunk); @@ -105,6 +105,9 @@ class BinaryResourceParser { // A mapping of resource ID to resource name. When we finish parsing // we use this to convert all resource IDs to symbolic references. std::map<ResourceId, ResourceName> id_index_; + + // A mapping of resource ID to type spec flags. + std::unordered_map<ResourceId, uint32_t> entry_type_spec_flags_; }; } // namespace aapt diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index a3034df91d82..24a4112e181d 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -283,7 +283,7 @@ class PackageFlattener { T* result = buffer->NextBlock<T>(); ResTable_entry* out_entry = (ResTable_entry*)result; - if (entry->entry->symbol_status.state == SymbolState::kPublic) { + if (entry->entry->visibility.level == Visibility::Level::kPublic) { out_entry->flags |= ResTable_entry::FLAG_PUBLIC; } @@ -443,10 +443,15 @@ class PackageFlattener { // Populate the config masks for this entry. - if (entry->symbol_status.state == SymbolState::kPublic) { + if (entry->visibility.level == Visibility::Level::kPublic) { config_masks[entry->id.value()] |= util::HostToDevice32(ResTable_typeSpec::SPEC_PUBLIC); } + if (entry->overlayable) { + config_masks[entry->id.value()] |= + util::HostToDevice32(ResTable_typeSpec::SPEC_OVERLAYABLE); + } + const size_t config_count = entry->values.size(); for (size_t i = 0; i < config_count; i++) { const ConfigDescription& config = entry->values[i]->config; diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index f0b80d22a9b4..51ccdc7359b2 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -26,6 +26,7 @@ using namespace android; +using ::testing::Gt; using ::testing::IsNull; using ::testing::NotNull; @@ -250,15 +251,15 @@ 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->AddResource(name, resid, ConfigDescription::DefaultConfig(), "", - std::unique_ptr<Value>(value->Clone(nullptr)), - context->GetDiagnostics())); + CHECK(table->AddResourceWithId(name, resid, ConfigDescription::DefaultConfig(), "", + std::unique_ptr<Value>(value->Clone(nullptr)), + context->GetDiagnostics())); // Every few entries, write out a sparse_config value. This will give us the desired load. if (i % stride == 0) { - CHECK(table->AddResource(name, resid, sparse_config, "", - std::unique_ptr<Value>(value->Clone(nullptr)), - context->GetDiagnostics())); + CHECK(table->AddResourceWithId(name, resid, sparse_config, "", + std::unique_ptr<Value>(value->Clone(nullptr)), + context->GetDiagnostics())); } } return table; @@ -568,4 +569,25 @@ TEST_F(TableFlattenerTest, ObfuscatingResourceNamesWithWhitelistSucceeds) { ResourceId(0x7f050000), {}, Res_value::TYPE_STRING, (uint32_t)idx, 0u)); } +TEST_F(TableFlattenerTest, FlattenOverlayable) { + std::unique_ptr<ResourceTable> table = + test::ResourceTableBuilder() + .SetPackageId("com.app.test", 0x7f) + .AddSimple("com.app.test:integer/overlayable", ResourceId(0x7f020000)) + .Build(); + + ASSERT_TRUE(table->SetOverlayable(test::ParseNameOrDie("com.app.test:integer/overlayable"), + Overlayable{}, test::GetDiagnostics())); + + ResTable res_table; + ASSERT_TRUE(Flatten(context_.get(), {}, table.get(), &res_table)); + + const StringPiece16 overlayable_name(u"com.app.test:integer/overlayable"); + uint32_t spec_flags = 0u; + ASSERT_THAT(res_table.identifierForName(overlayable_name.data(), overlayable_name.size(), nullptr, + 0u, nullptr, 0u, &spec_flags), + Gt(0u)); + EXPECT_TRUE(spec_flags & android::ResTable_typeSpec::SPEC_OVERLAYABLE); +} + } // namespace aapt diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index 0f0bce8bf5a7..3d6975d8d559 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -358,16 +358,16 @@ static void DeserializeSourceFromPb(const pb::Source& pb_source, const ResString out_source->line = static_cast<size_t>(pb_source.position().line_number()); } -static SymbolState DeserializeVisibilityFromPb(const pb::SymbolStatus_Visibility& pb_visibility) { - switch (pb_visibility) { - case pb::SymbolStatus_Visibility_PRIVATE: - return SymbolState::kPrivate; - case pb::SymbolStatus_Visibility_PUBLIC: - return SymbolState::kPublic; +static Visibility::Level DeserializeVisibilityFromPb(const pb::Visibility::Level& pb_level) { + switch (pb_level) { + case pb::Visibility::PRIVATE: + return Visibility::Level::kPrivate; + case pb::Visibility::PUBLIC: + return Visibility::Level::kPublic; default: break; } - return SymbolState::kUndefined; + return Visibility::Level::kUndefined; } static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStringPool& src_pool, @@ -402,28 +402,48 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr } // Deserialize the symbol status (public/private with source and comments). - if (pb_entry.has_symbol_status()) { - const pb::SymbolStatus& pb_status = pb_entry.symbol_status(); - if (pb_status.has_source()) { - DeserializeSourceFromPb(pb_status.source(), src_pool, &entry->symbol_status.source); + if (pb_entry.has_visibility()) { + const pb::Visibility& pb_visibility = pb_entry.visibility(); + if (pb_visibility.has_source()) { + DeserializeSourceFromPb(pb_visibility.source(), src_pool, &entry->visibility.source); } + entry->visibility.comment = pb_visibility.comment(); - entry->symbol_status.comment = pb_status.comment(); - entry->symbol_status.allow_new = pb_status.allow_new(); - - const SymbolState visibility = DeserializeVisibilityFromPb(pb_status.visibility()); - entry->symbol_status.state = visibility; - if (visibility == SymbolState::kPublic) { + const Visibility::Level level = DeserializeVisibilityFromPb(pb_visibility.level()); + entry->visibility.level = level; + if (level == Visibility::Level::kPublic) { // Propagate the public visibility up to the Type. - type->symbol_status.state = SymbolState::kPublic; - } else if (visibility == SymbolState::kPrivate) { + type->visibility_level = Visibility::Level::kPublic; + } else if (level == Visibility::Level::kPrivate) { // Only propagate if no previous state was assigned. - if (type->symbol_status.state == SymbolState::kUndefined) { - type->symbol_status.state = SymbolState::kPrivate; + if (type->visibility_level == Visibility::Level::kUndefined) { + type->visibility_level = Visibility::Level::kPrivate; } } } + if (pb_entry.has_allow_new()) { + const pb::AllowNew& pb_allow_new = pb_entry.allow_new(); + + AllowNew allow_new; + if (pb_allow_new.has_source()) { + DeserializeSourceFromPb(pb_allow_new.source(), src_pool, &allow_new.source); + } + allow_new.comment = pb_allow_new.comment(); + entry->allow_new = std::move(allow_new); + } + + if (pb_entry.has_overlayable()) { + const pb::Overlayable& pb_overlayable = pb_entry.overlayable(); + + Overlayable overlayable; + if (pb_overlayable.has_source()) { + DeserializeSourceFromPb(pb_overlayable.source(), src_pool, &overlayable.source); + } + overlayable.comment = pb_overlayable.comment(); + entry->overlayable = std::move(overlayable); + } + ResourceId resid(pb_package.package_id().id(), pb_type.type_id().id(), pb_entry.entry_id().id()); if (resid.is_valid()) { diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index 97ce01a9de13..78f12814389d 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -43,16 +43,16 @@ void SerializeSourceToPb(const Source& source, StringPool* src_pool, pb::Source* } } -static pb::SymbolStatus_Visibility SerializeVisibilityToPb(SymbolState state) { +static pb::Visibility::Level SerializeVisibilityToPb(Visibility::Level state) { switch (state) { - case SymbolState::kPrivate: - return pb::SymbolStatus_Visibility_PRIVATE; - case SymbolState::kPublic: - return pb::SymbolStatus_Visibility_PUBLIC; + case Visibility::Level::kPrivate: + return pb::Visibility::PRIVATE; + case Visibility::Level::kPublic: + return pb::Visibility::PUBLIC; default: break; } - return pb::SymbolStatus_Visibility_UNKNOWN; + return pb::Visibility::UNKNOWN; } void SerializeConfig(const ConfigDescription& config, pb::Configuration* out_pb_config) { @@ -293,12 +293,26 @@ void SerializeTableToPb(const ResourceTable& table, pb::ResourceTable* out_table } pb_entry->set_name(entry->name); - // Write the SymbolStatus struct. - pb::SymbolStatus* pb_status = pb_entry->mutable_symbol_status(); - pb_status->set_visibility(SerializeVisibilityToPb(entry->symbol_status.state)); - SerializeSourceToPb(entry->symbol_status.source, &source_pool, pb_status->mutable_source()); - pb_status->set_comment(entry->symbol_status.comment); - pb_status->set_allow_new(entry->symbol_status.allow_new); + // Write the Visibility struct. + pb::Visibility* pb_visibility = pb_entry->mutable_visibility(); + pb_visibility->set_level(SerializeVisibilityToPb(entry->visibility.level)); + SerializeSourceToPb(entry->visibility.source, &source_pool, + pb_visibility->mutable_source()); + pb_visibility->set_comment(entry->visibility.comment); + + if (entry->allow_new) { + pb::AllowNew* pb_allow_new = pb_entry->mutable_allow_new(); + SerializeSourceToPb(entry->allow_new.value().source, &source_pool, + pb_allow_new->mutable_source()); + pb_allow_new->set_comment(entry->allow_new.value().comment); + } + + if (entry->overlayable) { + pb::Overlayable* pb_overlayable = pb_entry->mutable_overlayable(); + SerializeSourceToPb(entry->overlayable.value().source, &source_pool, + pb_overlayable->mutable_source()); + pb_overlayable->set_comment(entry->overlayable.value().comment); + } for (const std::unique_ptr<ResourceConfigValue>& config_value : entry->values) { pb::ConfigValue* pb_config_value = pb_entry->add_config_value(); diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index 9649a4de2fe4..d7f83fd9ecdc 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -44,14 +44,15 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { .AddReference("com.app.a:layout/other", ResourceId(0x7f020001), "com.app.a:layout/main") .AddString("com.app.a:string/text", {}, "hi") .AddValue("com.app.a:id/foo", {}, util::make_unique<Id>()) - .SetSymbolState("com.app.a:bool/foo", {}, SymbolState::kUndefined, true /*allow_new*/) + .SetSymbolState("com.app.a:bool/foo", {}, Visibility::Level::kUndefined, + true /*allow_new*/) .Build(); - Symbol public_symbol; - public_symbol.state = SymbolState::kPublic; - ASSERT_TRUE(table->SetSymbolState(test::ParseNameOrDie("com.app.a:layout/main"), - ResourceId(0x7f020000), public_symbol, - context->GetDiagnostics())); + 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())); Id* id = test::GetValue<Id>(table.get(), "com.app.a:id/foo"); ASSERT_THAT(id, NotNull()); @@ -89,6 +90,10 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { test::ParseNameOrDie("com.app.a:layout/abc"), ConfigDescription::DefaultConfig(), {}, util::make_unique<Reference>(expected_ref), context->GetDiagnostics())); + // Make an overlayable resource. + ASSERT_TRUE(table->SetOverlayable(test::ParseNameOrDie("com.app.a:integer/overlayable"), + Overlayable{}, test::GetDiagnostics())); + pb::ResourceTable pb_table; SerializeTableToPb(*table, &pb_table); @@ -110,13 +115,13 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { new_table.FindResource(test::ParseNameOrDie("com.app.a:layout/main")); ASSERT_TRUE(result); - EXPECT_THAT(result.value().type->symbol_status.state, Eq(SymbolState::kPublic)); - EXPECT_THAT(result.value().entry->symbol_status.state, Eq(SymbolState::kPublic)); + EXPECT_THAT(result.value().type->visibility_level, Eq(Visibility::Level::kPublic)); + EXPECT_THAT(result.value().entry->visibility.level, Eq(Visibility::Level::kPublic)); result = new_table.FindResource(test::ParseNameOrDie("com.app.a:bool/foo")); ASSERT_TRUE(result); - EXPECT_THAT(result.value().entry->symbol_status.state, Eq(SymbolState::kUndefined)); - EXPECT_TRUE(result.value().entry->symbol_status.allow_new); + EXPECT_THAT(result.value().entry->visibility.level, Eq(Visibility::Level::kUndefined)); + EXPECT_TRUE(result.value().entry->allow_new); // Find the product-dependent values BinaryPrimitive* prim = test::GetValueForConfigAndProduct<BinaryPrimitive>( @@ -148,6 +153,12 @@ TEST(ProtoSerializeTest, SerializeSinglePackage) { EXPECT_THAT(*actual_styled_str->value->spans[0].name, Eq("b")); EXPECT_THAT(actual_styled_str->value->spans[0].first_char, Eq(0u)); EXPECT_THAT(actual_styled_str->value->spans[0].last_char, Eq(4u)); + + Maybe<ResourceTable::SearchResult> search_result = + new_table.FindResource(test::ParseNameOrDie("com.app.a:integer/overlayable")); + ASSERT_TRUE(search_result); + ASSERT_THAT(search_result.value().entry, NotNull()); + EXPECT_TRUE(search_result.value().entry->overlayable); } TEST(ProtoSerializeTest, SerializeAndDeserializeXml) { diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 8c8c2549609a..6b07b1e96261 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -191,14 +191,14 @@ JavaClassGenerator::JavaClassGenerator(IAaptContext* context, const JavaClassGeneratorOptions& options) : context_(context), table_(table), options_(options) {} -bool JavaClassGenerator::SkipSymbol(SymbolState state) { +bool JavaClassGenerator::SkipSymbol(Visibility::Level level) { switch (options_.types) { case JavaClassGeneratorOptions::SymbolTypes::kAll: return false; case JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate: - return state == SymbolState::kUndefined; + return level == Visibility::Level::kUndefined; case JavaClassGeneratorOptions::SymbolTypes::kPublic: - return state != SymbolState::kPublic; + return level != Visibility::Level::kPublic; } return true; } @@ -444,8 +444,8 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso AnnotationProcessor* processor = resource_member->GetCommentBuilder(); // Add the comments from any <public> tags. - if (entry.symbol_status.state != SymbolState::kUndefined) { - processor->AppendComment(entry.symbol_status.comment); + if (entry.visibility.level != Visibility::Level::kUndefined) { + processor->AppendComment(entry.visibility.comment); } // Add the comments from all configurations of this entry. @@ -484,7 +484,7 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso Maybe<std::string> JavaClassGenerator::UnmangleResource(const StringPiece& package_name, const StringPiece& package_name_to_generate, const ResourceEntry& entry) { - if (SkipSymbol(entry.symbol_status.state)) { + if (SkipSymbol(entry.visibility.level)) { return {}; } diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h index 4992f077c566..853120b3cb98 100644 --- a/tools/aapt2/java/JavaClassGenerator.h +++ b/tools/aapt2/java/JavaClassGenerator.h @@ -82,7 +82,7 @@ class JavaClassGenerator { static std::string TransformToFieldName(const android::StringPiece& symbol); private: - bool SkipSymbol(SymbolState state); + bool SkipSymbol(Visibility::Level state); bool SkipSymbol(const Maybe<SymbolTable::Symbol>& symbol); // Returns the unmangled resource entry name if the unmangled package is the same as diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 02f4cb14eb41..5beb594bd92f 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -139,8 +139,8 @@ TEST(JavaClassGeneratorTest, OnlyWritePublicResources) { .AddSimple("android:id/one", ResourceId(0x01020000)) .AddSimple("android:id/two", ResourceId(0x01020001)) .AddSimple("android:id/three", ResourceId(0x01020002)) - .SetSymbolState("android:id/one", ResourceId(0x01020000), SymbolState::kPublic) - .SetSymbolState("android:id/two", ResourceId(0x01020001), SymbolState::kPrivate) + .SetSymbolState("android:id/one", ResourceId(0x01020000), Visibility::Level::kPublic) + .SetSymbolState("android:id/two", ResourceId(0x01020001), Visibility::Level::kPrivate) .Build(); std::unique_ptr<IAaptContext> context = diff --git a/tools/aapt2/link/PrivateAttributeMover.cpp b/tools/aapt2/link/PrivateAttributeMover.cpp index eee4b60c1769..675b02a7e161 100644 --- a/tools/aapt2/link/PrivateAttributeMover.cpp +++ b/tools/aapt2/link/PrivateAttributeMover.cpp @@ -62,7 +62,7 @@ bool PrivateAttributeMover::Consume(IAaptContext* context, ResourceTable* table) continue; } - if (type->symbol_status.state != SymbolState::kPublic) { + if (type->visibility_level != Visibility::Level::kPublic) { // No public attributes, so we can safely leave these private attributes // where they are. continue; @@ -72,7 +72,7 @@ bool PrivateAttributeMover::Consume(IAaptContext* context, ResourceTable* table) move_if(type->entries, std::back_inserter(private_attr_entries), [](const std::unique_ptr<ResourceEntry>& entry) -> bool { - return entry->symbol_status.state != SymbolState::kPublic; + return entry->visibility.level != Visibility::Level::kPublic; }); if (private_attr_entries.empty()) { diff --git a/tools/aapt2/link/PrivateAttributeMover_test.cpp b/tools/aapt2/link/PrivateAttributeMover_test.cpp index 7fcf6e7ab5cd..168234b36e4a 100644 --- a/tools/aapt2/link/PrivateAttributeMover_test.cpp +++ b/tools/aapt2/link/PrivateAttributeMover_test.cpp @@ -30,9 +30,9 @@ TEST(PrivateAttributeMoverTest, MovePrivateAttributes) { .AddSimple("android:attr/publicB") .AddSimple("android:attr/privateB") .SetSymbolState("android:attr/publicA", ResourceId(0x01010000), - SymbolState::kPublic) + Visibility::Level::kPublic) .SetSymbolState("android:attr/publicB", ResourceId(0x01010000), - SymbolState::kPublic) + Visibility::Level::kPublic) .Build(); PrivateAttributeMover mover; @@ -81,7 +81,7 @@ TEST(PrivateAttributeMoverTest, DoNotCreatePrivateAttrsIfNoneExist) { std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder() .AddSimple("android:attr/pub") - .SetSymbolState("android:attr/pub", ResourceId(0x01010000), SymbolState::kPublic) + .SetSymbolState("android:attr/pub", ResourceId(0x01010000), Visibility::Level::kPublic) .Build(); ResourceTablePackage* package = table->FindPackage("android"); diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index ad7d8b65350d..b8f880427c71 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -363,8 +363,8 @@ bool ReferenceLinker::Consume(IAaptContext* context, ResourceTable* table) { NameMangler::Unmangle(&name.entry, &name.package); // Symbol state information may be lost if there is no value for the resource. - if (entry->symbol_status.state != SymbolState::kUndefined && entry->values.empty()) { - context->GetDiagnostics()->Error(DiagMessage(entry->symbol_status.source) + if (entry->visibility.level != Visibility::Level::kUndefined && entry->values.empty()) { + context->GetDiagnostics()->Error(DiagMessage(entry->visibility.source) << "no definition for declared symbol '" << name << "'"); error = true; } diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 58d0607ed7b3..e819f51a5634 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -83,44 +83,58 @@ 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 (dst_type->symbol_status.state < src_type->symbol_status.state) { + 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->symbol_status.state == SymbolState::kPublic) { + 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->symbol_status = std::move(src_type->symbol_status); - } else if (dst_type->symbol_status.state == SymbolState::kPublic && - src_type->symbol_status.state == SymbolState::kPublic && - dst_type->id && src_type->id && - dst_type->id.value() != src_type->id.value()) { + 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"); + context->GetDiagnostics()->Error(DiagMessage(src) << "cannot merge type '" << src_type->type + << "': conflicting public IDs"); return false; } return true; } -static bool MergeEntry(IAaptContext* context, const Source& src, ResourceEntry* dst_entry, - ResourceEntry* src_entry) { - if (dst_entry->symbol_status.state < src_entry->symbol_status.state) { - // The incoming type's visibility is stronger, so we should override the visibility. - if (src_entry->symbol_status.state == SymbolState::kPublic) { - // Only copy the ID if the source is public, or else the ID is meaningless. +static bool MergeEntry(IAaptContext* context, const Source& src, bool overlay, + ResourceEntry* dst_entry, ResourceEntry* src_entry) { + // Copy over the strongest visibility. + if (src_entry->visibility.level > dst_entry->visibility.level) { + // Only copy the ID if the source is public, or else the ID is meaningless. + if (src_entry->visibility.level == Visibility::Level::kPublic) { dst_entry->id = src_entry->id; } - dst_entry->symbol_status = std::move(src_entry->symbol_status); - } else if (src_entry->symbol_status.state == SymbolState::kPublic && - dst_entry->symbol_status.state == SymbolState::kPublic && - dst_entry->id && src_entry->id && - dst_entry->id.value() != src_entry->id.value()) { + dst_entry->visibility = std::move(src_entry->visibility); + } else if (src_entry->visibility.level == Visibility::Level::kPublic && + dst_entry->visibility.level == Visibility::Level::kPublic && dst_entry->id && + src_entry->id && src_entry->id != dst_entry->id) { // Both entries are public and have different IDs. context->GetDiagnostics()->Error(DiagMessage(src) << "cannot merge entry '" << src_entry->name << "': conflicting public IDs"); return false; } + + // Copy over the rest of the properties, if needed. + if (src_entry->allow_new) { + dst_entry->allow_new = std::move(src_entry->allow_new); + } + + if (src_entry->overlayable) { + if (dst_entry->overlayable && !overlay) { + context->GetDiagnostics()->Error(DiagMessage(src_entry->overlayable.value().source) + << "duplicate overlayable declaration for resource '" + << src_entry->name << "'"); + context->GetDiagnostics()->Error(DiagMessage(dst_entry->overlayable.value().source) + << "previous declaration here"); + return false; + } + dst_entry->overlayable = std::move(src_entry->overlayable); + } return true; } @@ -202,7 +216,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, } ResourceEntry* dst_entry; - if (allow_new_resources || src_entry->symbol_status.allow_new) { + if (allow_new_resources || src_entry->allow_new) { dst_entry = dst_type->FindOrCreateEntry(entry_name); } else { dst_entry = dst_type->FindEntry(entry_name); @@ -220,7 +234,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, continue; } - if (!MergeEntry(context_, src, dst_entry, src_entry.get())) { + if (!MergeEntry(context_, src, overlay, dst_entry, src_entry.get())) { error = true; continue; } diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 6aab8ded24a5..34461c6b467d 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -182,14 +182,12 @@ TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() .SetPackageId("", 0x7f) - .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), - SymbolState::kPublic) + .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), - SymbolState::kPublic) + .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), Visibility::Level::kPublic) .Build(); ResourceTable final_table; @@ -205,14 +203,12 @@ TEST_F(TableMergerTest, FailToOverrideConflictingTypeIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() .SetPackageId("", 0x7f) - .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), - SymbolState::kPublic) + .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), - SymbolState::kPublic) + .SetSymbolState("bool/foo", ResourceId(0x7f, 0x02, 0x0001), Visibility::Level::kPublic) .Build(); ResourceTable final_table; @@ -228,14 +224,12 @@ TEST_F(TableMergerTest, FailToOverrideConflictingEntryIdsWithOverlay) { std::unique_ptr<ResourceTable> base = test::ResourceTableBuilder() .SetPackageId("", 0x7f) - .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0001), - SymbolState::kPublic) + .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), - SymbolState::kPublic) + .SetSymbolState("bool/foo", ResourceId(0x7f, 0x01, 0x0002), Visibility::Level::kPublic) .Build(); ResourceTable final_table; @@ -253,7 +247,7 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlay) { std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() .SetPackageId("", 0x7f) - .SetSymbolState("bool/foo", {}, SymbolState::kUndefined, true /*allow new overlay*/) + .SetSymbolState("bool/foo", {}, Visibility::Level::kUndefined, true /*allow new overlay*/) .AddValue("bool/foo", ResourceUtils::TryParseBool("true")) .Build(); diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 0cfc0bdfaaa6..3cae0e8ae462 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -190,7 +190,7 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName( ResourceTable::SearchResult sr = result.value(); std::unique_ptr<SymbolTable::Symbol> symbol = util::make_unique<SymbolTable::Symbol>(); - symbol->is_public = (sr.entry->symbol_status.state == SymbolState::kPublic); + 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()); diff --git a/tools/aapt2/split/TableSplitter.cpp b/tools/aapt2/split/TableSplitter.cpp index 9d49ca6c0aa9..e99174302d26 100644 --- a/tools/aapt2/split/TableSplitter.cpp +++ b/tools/aapt2/split/TableSplitter.cpp @@ -233,13 +233,13 @@ void TableSplitter::SplitTable(ResourceTable* original_table) { ResourceTableType* split_type = split_pkg->FindOrCreateType(type->type); if (!split_type->id) { split_type->id = type->id; - split_type->symbol_status = type->symbol_status; + split_type->visibility_level = type->visibility_level; } ResourceEntry* split_entry = split_type->FindOrCreateEntry(entry->name); if (!split_entry->id) { split_entry->id = entry->id; - split_entry->symbol_status = entry->symbol_status; + split_entry->visibility = entry->visibility; } // Copy the selected values into the new Split Entry. diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index 88897a8afac3..a3e2f800931c 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -116,19 +116,20 @@ ResourceTableBuilder& ResourceTableBuilder::AddValue(const StringPiece& name, const ResourceId& id, std::unique_ptr<Value> value) { ResourceName res_name = ParseNameOrDie(name); - CHECK(table_->AddResourceAllowMangled(res_name, id, config, {}, std::move(value), - GetDiagnostics())); + CHECK(table_->AddResourceWithIdMangled(res_name, id, config, {}, std::move(value), + GetDiagnostics())); return *this; } ResourceTableBuilder& ResourceTableBuilder::SetSymbolState(const StringPiece& name, - const ResourceId& id, SymbolState state, + const ResourceId& id, + Visibility::Level level, bool allow_new) { ResourceName res_name = ParseNameOrDie(name); - Symbol symbol; - symbol.state = state; - symbol.allow_new = allow_new; - CHECK(table_->SetSymbolStateAllowMangled(res_name, id, symbol, GetDiagnostics())); + Visibility visibility; + visibility.level = level; + CHECK(table_->SetVisibilityWithIdMangled(res_name, visibility, id, GetDiagnostics())); + CHECK(table_->SetAllowNewMangled(res_name, AllowNew{}, GetDiagnostics())); return *this; } diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index 2f83b78ffb2a..b4569085c180 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -68,7 +68,7 @@ class ResourceTableBuilder { ResourceTableBuilder& AddValue(const android::StringPiece& name, const ConfigDescription& config, const ResourceId& id, std::unique_ptr<Value> value); ResourceTableBuilder& SetSymbolState(const android::StringPiece& name, const ResourceId& id, - SymbolState state, bool allow_new = false); + Visibility::Level level, bool allow_new = false); StringPool* string_pool(); std::unique_ptr<ResourceTable> Build(); |