diff options
author | Adam Lesinski <adamlesinski@google.com> | 2017-12-12 16:48:07 -0800 |
---|---|---|
committer | Adam Lesinski <adamlesinski@google.com> | 2017-12-18 14:16:02 -0800 |
commit | 71be70507de9cb619b644e55eda1cc181e3f7e90 (patch) | |
tree | 1ad3c588be3dd06b39b1ba5c3229f80ca08d62bd /tools | |
parent | 6bb6fad16d93a5859d47dcf962337c2719e585dd (diff) |
AAPT2: Propagate SPEC_OVERLAYABLE flag to final APK
Resources can be marked as overlayable, which means they can
be overlaid by runtime resource overlays.
This change propagates this state to the final resource table that
is installed on device.
Future work:
- Have the idmap tool respect the overlayable state and ignore
entries that overlay anything else.
Bug: 64980941
Test: make aapt2_tests
Change-Id: Id45b1e141a281be2ee32a4ac3096fcf1114d523b
Diffstat (limited to 'tools')
30 files changed, 811 insertions, 462 deletions
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(); |