diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h index cffe8d5d803f2..493f238b033da 100644 --- a/tools/aapt2/Resource.h +++ b/tools/aapt2/Resource.h @@ -118,6 +118,9 @@ struct ResourceNameRef { bool is_valid() const; }; +constexpr const uint8_t kAppPackageId = 0x7fu; +constexpr const uint8_t kFrameworkPackageId = 0x01u; + /** * A binary identifier representing a resource. Internally it * is a 32bit integer split as follows: diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 2d8e5a2190261..ca6738b16911a 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -533,6 +533,7 @@ uint32_t AndroidTypeToAttributeTypeMask(uint16_t type) { case android::Res_value::TYPE_REFERENCE: case android::Res_value::TYPE_ATTRIBUTE: case android::Res_value::TYPE_DYNAMIC_REFERENCE: + case android::Res_value::TYPE_DYNAMIC_ATTRIBUTE: return android::ResTable_map::TYPE_REFERENCE; case android::Res_value::TYPE_STRING: diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index f75ed7ad978a6..2868b2acee2d5 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -88,10 +88,24 @@ bool Reference::Equals(const Value* value) const { } bool Reference::Flatten(android::Res_value* out_value) const { - out_value->dataType = (reference_type == Reference::Type::kResource) - ? android::Res_value::TYPE_REFERENCE - : android::Res_value::TYPE_ATTRIBUTE; - out_value->data = util::HostToDevice32(id ? id.value().id : 0); + const ResourceId resid = id.value_or_default(ResourceId(0)); + const bool dynamic = + (resid.package_id() != kFrameworkPackageId && resid.package_id() != kAppPackageId); + + if (reference_type == Reference::Type::kResource) { + if (dynamic) { + out_value->dataType = android::Res_value::TYPE_DYNAMIC_REFERENCE; + } else { + out_value->dataType = android::Res_value::TYPE_REFERENCE; + } + } else { + if (dynamic) { + out_value->dataType = android::Res_value::TYPE_DYNAMIC_ATTRIBUTE; + } else { + out_value->dataType = android::Res_value::TYPE_ATTRIBUTE; + } + } + out_value->data = util::HostToDevice32(resid.id); return true; } diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 9c0f13c0241d4..68bdb959ca079 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -533,9 +533,14 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate, std::unique_ptr rewrite_method; // Generate an onResourcesLoaded() callback if requested. - if (options_.generate_rewrite_callback) { + if (options_.rewrite_callback_options) { rewrite_method = util::make_unique("public static void onResourcesLoaded(int p)"); + for (const std::string& package_to_callback : + options_.rewrite_callback_options.value().packages_to_callback) { + rewrite_method->AppendStatement( + StringPrintf("%s.R.onResourcesLoaded(p);", package_to_callback.data())); + } } for (const auto& package : table_->packages) { diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h index 178f558ac2ec9..45104301e7f51 100644 --- a/tools/aapt2/java/JavaClassGenerator.h +++ b/tools/aapt2/java/JavaClassGenerator.h @@ -33,13 +33,19 @@ class AnnotationProcessor; class ClassDefinition; class MethodDefinition; +// Options for generating onResourcesLoaded callback in R.java. +struct OnResourcesLoadedCallbackOptions { + // Other R classes to delegate the same callback to (with the same package ID). + std::vector packages_to_callback; +}; + struct JavaClassGeneratorOptions { // Specifies whether to use the 'final' modifier on resource entries. Default is true. bool use_final = true; - // Whether to generate code to rewrite the package ID of resources. - // Implies use_final == true. Default is false. - bool generate_rewrite_callback = false; + // If set, generates code to rewrite the package ID of resources. + // Implies use_final == true. Default is unset. + Maybe rewrite_callback_options; enum class SymbolTypes { kAll, diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index bcb2d4f42eda5..271279ff5e92a 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -381,7 +381,9 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) JavaClassGeneratorOptions options; options.use_final = false; - options.generate_rewrite_callback = true; + options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{ + {"com.foo", "com.boo"}, + }; JavaClassGenerator generator(context.get(), table.get(), options); std::stringstream out; @@ -389,7 +391,9 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary) std::string actual = out.str(); - EXPECT_NE(std::string::npos, actual.find("onResourcesLoaded")); + EXPECT_NE(std::string::npos, actual.find("void onResourcesLoaded")); + EXPECT_NE(std::string::npos, actual.find("com.foo.R.onResourcesLoaded")); + EXPECT_NE(std::string::npos, actual.find("com.boo.R.onResourcesLoaded")); } } // namespace aapt diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp index 49052169b6e66..dd8e14b40fa81 100644 --- a/tools/aapt2/link/Link.cpp +++ b/tools/aapt2/link/Link.cpp @@ -1097,18 +1097,15 @@ class LinkCommand { return false; } - std::unique_ptr table = - LoadTablePbFromCollection(collection.get()); + std::unique_ptr table = LoadTablePbFromCollection(collection.get()); if (!table) { - context_->GetDiagnostics()->Error(DiagMessage(input) - << "invalid static library"); + context_->GetDiagnostics()->Error(DiagMessage(input) << "invalid static library"); return false; } ResourceTablePackage* pkg = table->FindPackageById(0x7f); if (!pkg) { - context_->GetDiagnostics()->Error(DiagMessage(input) - << "static library has no package"); + context_->GetDiagnostics()->Error(DiagMessage(input) << "static library has no package"); return false; } @@ -1125,11 +1122,9 @@ class LinkCommand { pkg->name = ""; if (override) { - result = table_merger_->MergeOverlay(Source(input), table.get(), - collection.get()); + result = table_merger_->MergeOverlay(Source(input), table.get(), collection.get()); } else { - result = - table_merger_->Merge(Source(input), table.get(), collection.get()); + result = table_merger_->Merge(Source(input), table.get(), collection.get()); } } else { @@ -1782,17 +1777,21 @@ class LinkCommand { } if (options_.generate_java_class_path) { - JavaClassGeneratorOptions options; - options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; - options.javadoc_annotations = options_.javadoc_annotations; + // The set of packages whose R class to call in the main classes + // onResourcesLoaded callback. + std::vector packages_to_callback; + + JavaClassGeneratorOptions template_options; + template_options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + template_options.javadoc_annotations = options_.javadoc_annotations; if (options_.package_type == PackageType::kStaticLib || options_.generate_non_final_ids) { - options.use_final = false; + template_options.use_final = false; } if (options_.package_type == PackageType::kSharedLib) { - options.use_final = false; - options.generate_rewrite_callback = true; + template_options.use_final = false; + template_options.rewrite_callback_options = OnResourcesLoadedCallbackOptions{}; } const StringPiece actual_package = context_->GetCompilationPackage(); @@ -1802,36 +1801,51 @@ class LinkCommand { output_package = options_.custom_java_package.value(); } + // Generate the private symbols if required. if (options_.private_symbols) { + packages_to_callback.push_back(options_.private_symbols.value()); + // If we defined a private symbols package, we only emit Public symbols // to the original package, and private and public symbols to the // private package. - - options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic; - if (!WriteJavaFile(&final_table_, context_->GetCompilationPackage(), - output_package, options)) { - return 1; - } - + JavaClassGeneratorOptions options = template_options; options.types = JavaClassGeneratorOptions::SymbolTypes::kPublicPrivate; - output_package = options_.private_symbols.value(); - } - - if (!WriteJavaFile(&final_table_, actual_package, output_package, - options)) { - return 1; - } - - for (const std::string& extra_package : options_.extra_java_packages) { - if (!WriteJavaFile(&final_table_, actual_package, extra_package, + if (!WriteJavaFile(&final_table_, actual_package, options_.private_symbols.value(), options)) { return 1; } } + + // Generate all the symbols for all extra packages. + for (const std::string& extra_package : options_.extra_java_packages) { + packages_to_callback.push_back(extra_package); + + JavaClassGeneratorOptions options = template_options; + options.types = JavaClassGeneratorOptions::SymbolTypes::kAll; + if (!WriteJavaFile(&final_table_, actual_package, extra_package, options)) { + return 1; + } + } + + // Generate the main public R class. + JavaClassGeneratorOptions options = template_options; + + // Only generate public symbols if we have a private package. + if (options_.private_symbols) { + options.types = JavaClassGeneratorOptions::SymbolTypes::kPublic; + } + + if (options.rewrite_callback_options) { + options.rewrite_callback_options.value().packages_to_callback = + std::move(packages_to_callback); + } + + if (!WriteJavaFile(&final_table_, actual_package, output_package, options)) { + return 1; + } } - if (!WriteProguardFile(options_.generate_proguard_rules_path, - proguard_keep_set)) { + if (!WriteProguardFile(options_.generate_proguard_rules_path, proguard_keep_set)) { return 1; } diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp index d5c0dc4087ba7..313fe453dff52 100644 --- a/tools/aapt2/link/ManifestFixer.cpp +++ b/tools/aapt2/link/ManifestFixer.cpp @@ -79,6 +79,17 @@ static bool RequiredNameIsJavaPackage(xml::Element* el, SourcePathDiagnostics* d return false; } +static xml::XmlNodeAction::ActionFuncWithDiag RequiredAndroidAttribute(const std::string& attr) { + return [=](xml::Element* el, SourcePathDiagnostics* diag) -> bool { + if (el->FindAttribute(xml::kSchemaAndroid, attr) == nullptr) { + diag->Error(DiagMessage(el->line_number) + << "<" << el->name << "> is missing required attribute 'android:" << attr << "'"); + return false; + } + return true; + }; +} + static bool VerifyManifest(xml::Element* el, SourcePathDiagnostics* diag) { xml::Attribute* attr = el->FindAttribute({}, "package"); if (!attr) { @@ -272,6 +283,16 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor, application_action["uses-library"].Action(RequiredNameIsJavaPackage); application_action["library"].Action(RequiredNameIsJavaPackage); + + xml::XmlNodeAction& static_library_action = application_action["static-library"]; + static_library_action.Action(RequiredNameIsJavaPackage); + static_library_action.Action(RequiredAndroidAttribute("version")); + + xml::XmlNodeAction& uses_static_library_action = application_action["uses-static-library"]; + uses_static_library_action.Action(RequiredNameIsJavaPackage); + uses_static_library_action.Action(RequiredAndroidAttribute("version")); + uses_static_library_action.Action(RequiredAndroidAttribute("certDigest")); + application_action["meta-data"] = meta_data_action; application_action["activity"] = component_action; application_action["activity-alias"] = component_action; diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index 4a4282637b3b6..0331313563c89 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -122,8 +122,7 @@ class ReferenceLinkerVisitor : public ValueVisitor { DiagMessage msg(entry.key.GetSource()); // Call the matches method again, this time with a DiagMessage so we - // fill - // in the actual error message. + // fill in the actual error message. symbol->attribute->Matches(entry.value.get(), &msg); context_->GetDiagnostics()->Error(msg); error_ = true; diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 7e7b9fb869489..9311091252631 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -39,14 +39,12 @@ TableMerger::TableMerger(IAaptContext* context, ResourceTable* out_table, bool TableMerger::Merge(const Source& src, ResourceTable* table, io::IFileCollection* collection) { - return MergeImpl(src, table, collection, false /* overlay */, - true /* allow new */); + return MergeImpl(src, table, collection, false /* overlay */, true /* allow new */); } bool TableMerger::MergeOverlay(const Source& src, ResourceTable* table, io::IFileCollection* collection) { - return MergeImpl(src, table, collection, true /* overlay */, - options_.auto_add_overlay); + return MergeImpl(src, table, collection, true /* overlay */, options_.auto_add_overlay); } /** @@ -55,25 +53,13 @@ bool TableMerger::MergeOverlay(const Source& src, ResourceTable* table, bool TableMerger::MergeImpl(const Source& src, ResourceTable* table, io::IFileCollection* collection, bool overlay, bool allow_new) { - const uint8_t desired_package_id = context_->GetPackageId(); - bool error = false; for (auto& package : table->packages) { - // Warn of packages with an unrelated ID. - const Maybe& id = package->id; - if (id && id.value() != 0x0 && id.value() != desired_package_id) { - context_->GetDiagnostics()->Warn(DiagMessage(src) << "ignoring package " - << package->name); - continue; - } - // Only merge an empty package or the package we're building. // Other packages may exist, which likely contain attribute definitions. // This is because at compile time it is unknown if the attributes are - // simply - // uses of the attribute or definitions. - if (package->name.empty() || - context_->GetCompilationPackage() == package->name) { + // simply uses of the attribute or definitions. + if (package->name.empty() || context_->GetCompilationPackage() == package->name) { FileMergeCallback callback; if (collection) { callback = [&](const ResourceNameRef& name, @@ -83,8 +69,7 @@ bool TableMerger::MergeImpl(const Source& src, ResourceTable* table, io::IFile* f = collection->FindFile(*old_file->path); if (!f) { context_->GetDiagnostics()->Error(DiagMessage(src) - << "file '" << *old_file->path - << "' not found"); + << "file '" << *old_file->path << "' not found"); return false; } diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 5d75e76af0351..bcafbcabee6c7 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -293,6 +293,11 @@ static Maybe GetResourceName(const android::ResTable& table, std::unique_ptr AssetManagerSymbolSource::FindById( ResourceId id) { + if (!id.is_valid()) { + // Exit early and avoid the error logs from AssetManager. + return {}; + } + const android::ResTable& table = assets_.getResources(false); Maybe maybe_name = GetResourceName(table, id); if (!maybe_name) { diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp index 29a921c497c96..9158bddfb1932 100644 --- a/tools/aapt2/unflatten/BinaryResourceParser.cpp +++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp @@ -458,10 +458,14 @@ std::unique_ptr BinaryResourceParser::ParseValue( } if (value->dataType == Res_value::TYPE_REFERENCE || - value->dataType == Res_value::TYPE_ATTRIBUTE) { - const Reference::Type type = (value->dataType == Res_value::TYPE_REFERENCE) - ? Reference::Type::kResource - : Reference::Type::kAttribute; + value->dataType == Res_value::TYPE_ATTRIBUTE || + value->dataType == Res_value::TYPE_DYNAMIC_REFERENCE || + value->dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE) { + Reference::Type type = Reference::Type::kResource; + if (value->dataType == Res_value::TYPE_ATTRIBUTE || + value->dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE) { + type = Reference::Type::kAttribute; + } if (data == 0) { // A reference of 0, must be the magic @null reference.