diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp index f41e57cc66d603f41fadf314a3580e55e1c40253..add0d8d48108d6d185e0b27fe9f08c3b3f7adba7 100644 --- a/cmds/idmap2/idmap2/Lookup.cpp +++ b/cmds/idmap2/idmap2/Lookup.cpp @@ -174,7 +174,7 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { return Error("failed to parse config"); } - std::vector<std::unique_ptr<const ApkAssets>> apk_assets; + std::vector<AssetManager2::ApkAssetsPtr> apk_assets; std::string target_path; std::string target_package_name; for (size_t i = 0; i < idmap_paths.size(); i++) { @@ -217,24 +217,21 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { apk_assets.push_back(std::move(overlay_apk)); } - // AssetManager2::SetApkAssets requires raw ApkAssets pointers, not unique_ptrs - std::vector<const ApkAssets*> raw_pointer_apk_assets; - std::transform(apk_assets.cbegin(), apk_assets.cend(), std::back_inserter(raw_pointer_apk_assets), - [](const auto& p) -> const ApkAssets* { return p.get(); }); - AssetManager2 am; - am.SetApkAssets(raw_pointer_apk_assets); - am.SetConfiguration(config); - - const Result<ResourceId> resid = ParseResReference(am, resid_str, target_package_name); - if (!resid) { - return Error(resid.GetError(), "failed to parse resource ID"); - } + { + // Make sure |apk_assets| vector outlives the asset manager as it doesn't own the assets. + AssetManager2 am(apk_assets, config); + + const Result<ResourceId> resid = ParseResReference(am, resid_str, target_package_name); + if (!resid) { + return Error(resid.GetError(), "failed to parse resource ID"); + } - const Result<std::string> value = GetValue(&am, *resid); - if (!value) { - return Error(value.GetError(), "resource 0x%08x not found", *resid); + const Result<std::string> value = GetValue(&am, *resid); + if (!value) { + return Error(value.GetError(), "resource 0x%08x not found", *resid); + } + std::cout << *value << std::endl; } - std::cout << *value << std::endl; return Unit{}; } diff --git a/cmds/idmap2/libidmap2/ResourceContainer.cpp b/cmds/idmap2/libidmap2/ResourceContainer.cpp index 0e3590486c6f6ace78db89da19fe8a385c25f331..7869fbdb8cea874f1135c4ee28783c8a212ff061 100644 --- a/cmds/idmap2/libidmap2/ResourceContainer.cpp +++ b/cmds/idmap2/libidmap2/ResourceContainer.cpp @@ -262,7 +262,7 @@ OverlayData CreateResourceMappingLegacy(const AssetManager2* overlay_am, } struct ResState { - std::unique_ptr<ApkAssets> apk_assets; + AssetManager2::ApkAssetsPtr apk_assets; const LoadedArsc* arsc; const LoadedPackage* package; std::unique_ptr<AssetManager2> am; @@ -284,7 +284,7 @@ struct ResState { } state.am = std::make_unique<AssetManager2>(); - if (!state.am->SetApkAssets({state.apk_assets.get()})) { + if (!state.am->SetApkAssets({state.apk_assets})) { return Error("failed to create asset manager"); } diff --git a/cmds/idmap2/tests/ResourceUtilsTests.cpp b/cmds/idmap2/tests/ResourceUtilsTests.cpp index 69142086765c07b75a3814c06e7b8f1514c8b09b..011040ba0ebf6d7f18d48525a8476bc8232c144c 100644 --- a/cmds/idmap2/tests/ResourceUtilsTests.cpp +++ b/cmds/idmap2/tests/ResourceUtilsTests.cpp @@ -38,7 +38,7 @@ class ResourceUtilsTests : public Idmap2Tests { apk_assets_ = ApkAssets::Load(GetTargetApkPath()); ASSERT_THAT(apk_assets_, NotNull()); - am_.SetApkAssets({apk_assets_.get()}); + am_.SetApkAssets({apk_assets_}); } const AssetManager2& GetAssetManager() { @@ -47,7 +47,7 @@ class ResourceUtilsTests : public Idmap2Tests { private: AssetManager2 am_; - std::unique_ptr<const ApkAssets> apk_assets_; + AssetManager2::ApkAssetsPtr apk_assets_; }; TEST_F(ResourceUtilsTests, ResToTypeEntryName) { diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp index e9ada235b3888b4a497ab1e87a2718863be8f96c..87f9652f85441ceb441905dfef32d2f7266b3b79 100644 --- a/core/jni/android_content_res_ApkAssets.cpp +++ b/core/jni/android_content_res_ApkAssets.cpp @@ -74,17 +74,37 @@ enum : format_type_t { FORMAT_DIRECTORY = 3, }; -Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr) { - return *reinterpret_cast<Guarded<std::unique_ptr<const ApkAssets>>*>(ptr); +Guarded<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr) { + return *reinterpret_cast<Guarded<AssetManager2::ApkAssetsPtr>*>(ptr); } -static jlong CreateGuardedApkAssets(std::unique_ptr<const ApkAssets> assets) { - auto guarded_assets = new Guarded<std::unique_ptr<const ApkAssets>>(std::move(assets)); - return reinterpret_cast<jlong>(guarded_assets); +static jlong CreateGuardedApkAssets(AssetManager2::ApkAssetsPtr assets) { + auto guarded_assets = new Guarded<AssetManager2::ApkAssetsPtr>(std::move(assets)); + return reinterpret_cast<jlong>(guarded_assets); } -static void DeleteGuardedApkAssets(Guarded<std::unique_ptr<const ApkAssets>>& apk_assets) { - delete &apk_assets; +static void DeleteGuardedApkAssets(Guarded<AssetManager2::ApkAssetsPtr>& apk_assets) { + apk_assets.safeDelete([&apk_assets](AssetManager2::ApkAssetsPtr* assets) { + if (!assets) { + ALOGW("ApkAssets: Double delete of native assets object %p, ignored", &apk_assets); + } else if (!*assets) { + ALOGW("ApkAssets: Empty native assets pointer in native assets object %p", &apk_assets); + } else { + // |RefBase| increments |StrongCount| for each |sp<>| instance, and |WeakCount| for + // both |sp<>| and |wp<>| instances. This means the actual |wp<>| instance count + // is |WeakCount - StrongCount|. + const auto useCount = (*assets)->getStrongCount(); + const auto weakCount = (*assets)->getWeakRefs()->getWeakCount() - useCount; + if (useCount > 1) { + ALOGW("ApkAssets: Deleting an object '%s' with %d > 1 strong and %d weak references", + (*assets)->GetDebugName().c_str(), int(useCount), int(weakCount)); + } else if (weakCount > 0) { + ALOGW("ApkAssets: Deleting an ApkAssets object '%s' with %d weak references", + (*assets)->GetDebugName().c_str(), int(weakCount)); + } + } + }); + delete &apk_assets; } class LoaderAssetsProvider : public AssetsProvider { @@ -209,7 +229,7 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma ATRACE_NAME(base::StringPrintf("LoadApkAssets(%s)", path.c_str()).c_str()); auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider); - std::unique_ptr<ApkAssets> apk_assets; + AssetManager2::ApkAssetsPtr apk_assets; switch (format) { case FORMAT_APK: { auto assets = MultiAssetsProvider::Create(std::move(loader_assets), @@ -269,7 +289,7 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t } auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider); - std::unique_ptr<const ApkAssets> apk_assets; + AssetManager2::ApkAssetsPtr apk_assets; switch (format) { case FORMAT_APK: { auto assets = @@ -336,7 +356,7 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ } auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider); - std::unique_ptr<const ApkAssets> apk_assets; + AssetManager2::ApkAssetsPtr apk_assets; switch (format) { case FORMAT_APK: { auto assets = @@ -374,11 +394,17 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ static jlong NativeLoadEmpty(JNIEnv* env, jclass /*clazz*/, jint flags, jobject assets_provider) { auto apk_assets = ApkAssets::Load(LoaderAssetsProvider::Create(env, assets_provider), flags); + if (apk_assets == nullptr) { + const std::string error_msg = base::StringPrintf("Failed to load empty assets with provider %p", + (void*)assets_provider); + jniThrowException(env, "java/io/IOException", error_msg.c_str()); + return 0; + } return CreateGuardedApkAssets(std::move(apk_assets)); } static void NativeDestroy(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr) { - DeleteGuardedApkAssets(ApkAssetsFromLong(ptr)); + DeleteGuardedApkAssets(ApkAssetsFromLong(ptr)); } static jstring NativeGetAssetPath(JNIEnv* env, jclass /*clazz*/, jlong ptr) { diff --git a/core/jni/android_content_res_ApkAssets.h b/core/jni/android_content_res_ApkAssets.h index 7e525dc75ef0133ff10966210a44078b40d5b5b2..8159a53caaa53d2bd043e35425f3480fc9a9b71d 100644 --- a/core/jni/android_content_res_ApkAssets.h +++ b/core/jni/android_content_res_ApkAssets.h @@ -18,13 +18,13 @@ #define ANDROID_CONTENT_RES_APKASSETS_H #include "androidfw/ApkAssets.h" +#include "androidfw/AssetManager2.h" #include "androidfw/MutexGuard.h" - #include "jni.h" namespace android { -Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr); +Guarded<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr); } // namespace android diff --git a/core/jni/android_util_AssetManager.cpp b/core/jni/android_util_AssetManager.cpp index a2205eb7dfdb76602d2c1866104fc896a8fc5894..3d1d1433ba1cdc397e93a3e182fe2cd79b335555 100644 --- a/core/jni/android_util_AssetManager.cpp +++ b/core/jni/android_util_AssetManager.cpp @@ -296,7 +296,7 @@ static void NativeSetApkAssets(JNIEnv* env, jclass /*clazz*/, jlong ptr, ATRACE_NAME("AssetManager::SetApkAssets"); const jsize apk_assets_len = env->GetArrayLength(apk_assets_array); - std::vector<const ApkAssets*> apk_assets; + std::vector<AssetManager2::ApkAssetsPtr> apk_assets; apk_assets.reserve(apk_assets_len); for (jsize i = 0; i < apk_assets_len; i++) { jobject obj = env->GetObjectArrayElement(apk_assets_array, i); @@ -310,9 +310,14 @@ static void NativeSetApkAssets(JNIEnv* env, jclass /*clazz*/, jlong ptr, if (env->ExceptionCheck()) { return; } - + if (!apk_assets_native_ptr) { + ALOGW("Got a closed ApkAssets instance at index %d for AssetManager %p", i, (void*)ptr); + std::string msg = StringPrintf("ApkAssets at index %d is closed, native pointer is null", i); + jniThrowException(env, "java/lang/IllegalArgumentException", msg.c_str()); + return; + } auto scoped_assets = ScopedLock(ApkAssetsFromLong(apk_assets_native_ptr)); - apk_assets.push_back(scoped_assets->get()); + apk_assets.emplace_back(*scoped_assets); } ScopedLock<AssetManager2> assetmanager(AssetManagerFromLong(ptr)); @@ -720,31 +725,36 @@ static jobjectArray NativeGetResourceStringArray(JNIEnv* env, jclass /*clazz*/, } if (attr_value.type == Res_value::TYPE_STRING) { - const ApkAssets* apk_assets = assetmanager->GetApkAssets()[attr_value.cookie]; - const ResStringPool* pool = apk_assets->GetLoadedArsc()->GetStringPool(); + auto apk_assets_weak = assetmanager->GetApkAssets()[attr_value.cookie]; + if (auto apk_assets = apk_assets_weak.promote()) { + const ResStringPool* pool = apk_assets->GetLoadedArsc()->GetStringPool(); + + jstring java_string; + if (auto str_utf8 = pool->string8At(attr_value.data); str_utf8.has_value()) { + java_string = env->NewStringUTF(str_utf8->data()); + } else { + auto str_utf16 = pool->stringAt(attr_value.data); + if (!str_utf16.has_value()) { + return nullptr; + } + java_string = env->NewString(reinterpret_cast<const jchar*>(str_utf16->data()), + str_utf16->size()); + } - jstring java_string; - if (auto str_utf8 = pool->string8At(attr_value.data); str_utf8.has_value()) { - java_string = env->NewStringUTF(str_utf8->data()); - } else { - auto str_utf16 = pool->stringAt(attr_value.data); - if (!str_utf16.has_value()) { + // Check for errors creating the strings (if malformed or no memory). + if (env->ExceptionCheck()) { return nullptr; } - java_string = env->NewString(reinterpret_cast<const jchar*>(str_utf16->data()), - str_utf16->size()); - } - - // Check for errors creating the strings (if malformed or no memory). - if (env->ExceptionCheck()) { - return nullptr; - } - env->SetObjectArrayElement(array, i, java_string); + env->SetObjectArrayElement(array, i, java_string); - // If we have a large amount of string in our array, we might overflow the - // local reference table of the VM. - env->DeleteLocalRef(java_string); + // If we have a large amount of string in our array, we might overflow the + // local reference table of the VM. + env->DeleteLocalRef(java_string); + } else { + ALOGW("NativeGetResourceStringArray: an expired assets object #%d / %d", i, + attr_value.cookie); + } } } return array; diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp index 15aaae25f75440105d859cb1f6691525670fc5fd..f0c639574a9ff8e99043fcd73bfd0174a82f5fdc 100644 --- a/libs/androidfw/ApkAssets.cpp +++ b/libs/androidfw/ApkAssets.cpp @@ -27,39 +27,34 @@ using base::unique_fd; constexpr const char* kResourcesArsc = "resources.arsc"; -ApkAssets::ApkAssets(std::unique_ptr<Asset> resources_asset, +ApkAssets::ApkAssets(PrivateConstructorUtil, std::unique_ptr<Asset> resources_asset, std::unique_ptr<LoadedArsc> loaded_arsc, - std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, - std::unique_ptr<LoadedIdmap> loaded_idmap) + std::unique_ptr<AssetsProvider> assets, package_property_t property_flags, + std::unique_ptr<Asset> idmap_asset, std::unique_ptr<LoadedIdmap> loaded_idmap) : resources_asset_(std::move(resources_asset)), loaded_arsc_(std::move(loaded_arsc)), assets_provider_(std::move(assets)), property_flags_(property_flags), idmap_asset_(std::move(idmap_asset)), - loaded_idmap_(std::move(loaded_idmap)) {} + loaded_idmap_(std::move(loaded_idmap)) { +} -std::unique_ptr<ApkAssets> ApkAssets::Load(const std::string& path, package_property_t flags) { +ApkAssetsPtr ApkAssets::Load(const std::string& path, package_property_t flags) { return Load(ZipAssetsProvider::Create(path, flags), flags); } -std::unique_ptr<ApkAssets> ApkAssets::LoadFromFd(base::unique_fd fd, - const std::string& debug_name, - package_property_t flags, - off64_t offset, - off64_t len) { +ApkAssetsPtr ApkAssets::LoadFromFd(base::unique_fd fd, const std::string& debug_name, + package_property_t flags, off64_t offset, off64_t len) { return Load(ZipAssetsProvider::Create(std::move(fd), debug_name, offset, len), flags); } -std::unique_ptr<ApkAssets> ApkAssets::Load(std::unique_ptr<AssetsProvider> assets, - package_property_t flags) { +ApkAssetsPtr ApkAssets::Load(std::unique_ptr<AssetsProvider> assets, package_property_t flags) { return LoadImpl(std::move(assets), flags, nullptr /* idmap_asset */, nullptr /* loaded_idmap */); } -std::unique_ptr<ApkAssets> ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t flags) { +ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset, + std::unique_ptr<AssetsProvider> assets, + package_property_t flags) { if (resources_asset == nullptr) { return {}; } @@ -67,8 +62,7 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadTable(std::unique_ptr<Asset> resources nullptr /* loaded_idmap */); } -std::unique_ptr<ApkAssets> ApkAssets::LoadOverlay(const std::string& idmap_path, - package_property_t flags) { +ApkAssetsPtr ApkAssets::LoadOverlay(const std::string& idmap_path, package_property_t flags) { CHECK((flags & PROPERTY_LOADER) == 0U) << "Cannot load RROs through loaders"; auto idmap_asset = AssetsProvider::CreateAssetFromFile(idmap_path); if (idmap_asset == nullptr) { @@ -103,10 +97,10 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadOverlay(const std::string& idmap_path, std::move(loaded_idmap)); } -std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, - std::unique_ptr<LoadedIdmap> loaded_idmap) { +ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> assets, + package_property_t property_flags, + std::unique_ptr<Asset> idmap_asset, + std::unique_ptr<LoadedIdmap> loaded_idmap) { if (assets == nullptr) { return {}; } @@ -125,11 +119,11 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> a std::move(idmap_asset), std::move(loaded_idmap)); } -std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, - std::unique_ptr<LoadedIdmap> loaded_idmap) { +ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_asset, + std::unique_ptr<AssetsProvider> assets, + package_property_t property_flags, + std::unique_ptr<Asset> idmap_asset, + std::unique_ptr<LoadedIdmap> loaded_idmap) { if (assets == nullptr ) { return {}; } @@ -155,10 +149,9 @@ std::unique_ptr<ApkAssets> ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_ return {}; } - return std::unique_ptr<ApkAssets>(new ApkAssets(std::move(resources_asset), - std::move(loaded_arsc), std::move(assets), - property_flags, std::move(idmap_asset), - std::move(loaded_idmap))); + return ApkAssetsPtr::make(PrivateConstructorUtil{}, std::move(resources_asset), + std::move(loaded_arsc), std::move(assets), property_flags, + std::move(idmap_asset), std::move(loaded_idmap)); } std::optional<std::string_view> ApkAssets::GetPath() const { @@ -174,4 +167,5 @@ bool ApkAssets::IsUpToDate() const { return IsLoader() || ((!loaded_idmap_ || loaded_idmap_->IsUpToDate()) && assets_provider_->IsUpToDate()); } + } // namespace android diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 68f5e4a88c7ef39331d9f6cf928314137fd8ead7..3a7c91ef5b08ba088c77dbf77aeff71a29f6e90d 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -91,13 +91,14 @@ struct FindEntryResult { StringPoolRef entry_string_ref; }; -AssetManager2::AssetManager2() { - memset(&configuration_, 0, sizeof(configuration_)); +AssetManager2::AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration) + : configuration_(configuration) { + // Don't invalidate caches here as there's nothing cached yet. + SetApkAssets(apk_assets, false); } -bool AssetManager2::SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool invalidate_caches) { - apk_assets_ = std::move(apk_assets); - BuildDynamicRefTable(); +bool AssetManager2::SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches) { + BuildDynamicRefTable(apk_assets); RebuildFilterList(); if (invalidate_caches) { InvalidateCaches(static_cast<uint32_t>(-1)); @@ -105,7 +106,13 @@ bool AssetManager2::SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool return true; } -void AssetManager2::BuildDynamicRefTable() { +bool AssetManager2::SetApkAssets(std::initializer_list<ApkAssetsPtr> apk_assets, + bool invalidate_caches) { + return SetApkAssets(ApkAssetsList(apk_assets.begin(), apk_assets.size()), invalidate_caches); +} + +void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { + apk_assets_.assign(apk_assets.begin(), apk_assets.end()); package_groups_.clear(); package_ids_.fill(0xff); @@ -116,16 +123,19 @@ void AssetManager2::BuildDynamicRefTable() { // Overlay resources are not directly referenced by an application so their resource ids // can change throughout the application's lifetime. Assign overlay package ids last. - std::vector<const ApkAssets*> sorted_apk_assets(apk_assets_); - std::stable_partition(sorted_apk_assets.begin(), sorted_apk_assets.end(), [](const ApkAssets* a) { - return !a->IsOverlay(); - }); + std::vector<const ApkAssets*> sorted_apk_assets; + sorted_apk_assets.reserve(apk_assets_.size()); + for (auto& asset : apk_assets) { + sorted_apk_assets.push_back(asset.get()); + } + std::stable_partition(sorted_apk_assets.begin(), sorted_apk_assets.end(), + [](auto a) { return !a->IsOverlay(); }); // The assets cookie must map to the position of the apk assets in the unsorted apk assets list. std::unordered_map<const ApkAssets*, ApkAssetsCookie> apk_assets_cookies; - apk_assets_cookies.reserve(apk_assets_.size()); - for (size_t i = 0, n = apk_assets_.size(); i < n; i++) { - apk_assets_cookies[apk_assets_[i]] = static_cast<ApkAssetsCookie>(i); + apk_assets_cookies.reserve(apk_assets.size()); + for (size_t i = 0, n = apk_assets.size(); i < n; i++) { + apk_assets_cookies[apk_assets[i].get()] = static_cast<ApkAssetsCookie>(i); } // 0x01 is reserved for the android package. @@ -242,7 +252,8 @@ void AssetManager2::DumpToLog() const { std::string list; for (const auto& apk_assets : apk_assets_) { - base::StringAppendF(&list, "%s,", apk_assets->GetDebugName().c_str()); + auto assets = apk_assets.promote(); + base::StringAppendF(&list, "%s,", assets ? assets->GetDebugName().c_str() : "nullptr"); } LOG(INFO) << "ApkAssets: " << list; @@ -279,7 +290,8 @@ const ResStringPool* AssetManager2::GetStringPoolForCookie(ApkAssetsCookie cooki if (cookie < 0 || static_cast<size_t>(cookie) >= apk_assets_.size()) { return nullptr; } - return apk_assets_[cookie]->GetLoadedArsc()->GetStringPool(); + auto assets = apk_assets_[cookie].promote(); + return assets ? assets->GetLoadedArsc()->GetStringPool() : nullptr; } const DynamicRefTable* AssetManager2::GetDynamicRefTableForPackage(uint32_t package_id) const { @@ -331,7 +343,11 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, std::string* out) const { uint8_t package_id = 0U; for (const auto& apk_assets : apk_assets_) { - const LoadedArsc* loaded_arsc = apk_assets->GetLoadedArsc(); + auto assets = apk_assets.promote(); + if (!assets) { + continue; + } + const LoadedArsc* loaded_arsc = assets->GetLoadedArsc(); if (loaded_arsc == nullptr) { continue; } @@ -384,8 +400,10 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, } bool AssetManager2::ContainsAllocatedTable() const { - return std::find_if(apk_assets_.begin(), apk_assets_.end(), - std::mem_fn(&ApkAssets::IsTableAllocated)) != apk_assets_.end(); + return std::find_if(apk_assets_.begin(), apk_assets_.end(), [](auto&& assets_weak) { + auto assets = assets_weak.promote(); + return assets && assets->IsTableAllocated(); + }) != apk_assets_.end(); } void AssetManager2::SetConfiguration(const ResTable_config& configuration) { @@ -398,8 +416,8 @@ void AssetManager2::SetConfiguration(const ResTable_config& configuration) { } } -std::set<const ApkAssets*> AssetManager2::GetNonSystemOverlays() const { - std::set<const ApkAssets*> non_system_overlays; +std::set<AssetManager2::ApkAssetsPtr> AssetManager2::GetNonSystemOverlays() const { + std::set<ApkAssetsPtr> non_system_overlays; for (const PackageGroup& package_group : package_groups_) { bool found_system_package = false; for (const ConfiguredPackage& package : package_group.packages_) { @@ -411,7 +429,9 @@ std::set<const ApkAssets*> AssetManager2::GetNonSystemOverlays() const { if (!found_system_package) { for (const ConfiguredOverlay& overlay : package_group.overlays_) { - non_system_overlays.insert(apk_assets_[overlay.cookie]); + if (auto asset = apk_assets_[overlay.cookie].promote()) { + non_system_overlays.insert(std::move(asset)); + } } } } @@ -423,21 +443,24 @@ base::expected<std::set<ResTable_config>, IOError> AssetManager2::GetResourceCon bool exclude_system, bool exclude_mipmap) const { ATRACE_NAME("AssetManager::GetResourceConfigurations"); const auto non_system_overlays = - (exclude_system) ? GetNonSystemOverlays() : std::set<const ApkAssets*>(); + exclude_system ? GetNonSystemOverlays() : std::set<ApkAssetsPtr>(); std::set<ResTable_config> configurations; for (const PackageGroup& package_group : package_groups_) { for (size_t i = 0; i < package_group.packages_.size(); i++) { const ConfiguredPackage& package = package_group.packages_[i]; - if (exclude_system && package.loaded_package_->IsSystem()) { - continue; - } - - auto apk_assets = apk_assets_[package_group.cookies_[i]]; - if (exclude_system && apk_assets->IsOverlay() && - non_system_overlays.find(apk_assets) == non_system_overlays.end()) { - // Exclude overlays that target system resources. - continue; + if (exclude_system) { + if (package.loaded_package_->IsSystem()) { + continue; + } + if (!non_system_overlays.empty()) { + // Exclude overlays that target only system resources. + auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + if (apk_assets && apk_assets->IsOverlay() && + non_system_overlays.find(apk_assets) == non_system_overlays.end()) { + continue; + } + } } auto result = package.loaded_package_->CollectConfigurations(exclude_mipmap, &configurations); @@ -454,20 +477,23 @@ std::set<std::string> AssetManager2::GetResourceLocales(bool exclude_system, ATRACE_NAME("AssetManager::GetResourceLocales"); std::set<std::string> locales; const auto non_system_overlays = - (exclude_system) ? GetNonSystemOverlays() : std::set<const ApkAssets*>(); + exclude_system ? GetNonSystemOverlays() : std::set<ApkAssetsPtr>(); for (const PackageGroup& package_group : package_groups_) { for (size_t i = 0; i < package_group.packages_.size(); i++) { const ConfiguredPackage& package = package_group.packages_[i]; - if (exclude_system && package.loaded_package_->IsSystem()) { - continue; - } - - auto apk_assets = apk_assets_[package_group.cookies_[i]]; - if (exclude_system && apk_assets->IsOverlay() && - non_system_overlays.find(apk_assets) == non_system_overlays.end()) { - // Exclude overlays that target system resources. - continue; + if (exclude_system) { + if (package.loaded_package_->IsSystem()) { + continue; + } + if (!non_system_overlays.empty()) { + // Exclude overlays that target only system resources. + auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + if (apk_assets && apk_assets->IsOverlay() && + non_system_overlays.find(apk_assets) == non_system_overlays.end()) { + continue; + } + } } package.loaded_package_->CollectLocales(merge_equivalent_languages, &locales); @@ -492,13 +518,12 @@ std::unique_ptr<AssetDir> AssetManager2::OpenDir(const std::string& dirname) con ATRACE_NAME("AssetManager::OpenDir"); std::string full_path = "assets/" + dirname; - std::unique_ptr<SortedVector<AssetDir::FileInfo>> files = - util::make_unique<SortedVector<AssetDir::FileInfo>>(); + auto files = util::make_unique<SortedVector<AssetDir::FileInfo>>(); // Start from the back. for (auto iter = apk_assets_.rbegin(); iter != apk_assets_.rend(); ++iter) { - const ApkAssets* apk_assets = *iter; - if (apk_assets->IsOverlay()) { + auto apk_assets = iter->promote(); + if (!apk_assets || apk_assets->IsOverlay()) { continue; } @@ -527,14 +552,15 @@ std::unique_ptr<Asset> AssetManager2::OpenNonAsset(const std::string& filename, Asset::AccessMode mode, ApkAssetsCookie* out_cookie) const { for (int32_t i = apk_assets_.size() - 1; i >= 0; i--) { + const auto assets = apk_assets_[i].promote(); // Prevent RRO from modifying assets and other entries accessed by file // path. Explicitly asking for a path in a given package (denoted by a // cookie) is still OK. - if (apk_assets_[i]->IsOverlay()) { + if (!assets || assets->IsOverlay()) { continue; } - std::unique_ptr<Asset> asset = apk_assets_[i]->GetAssetsProvider()->Open(filename, mode); + std::unique_ptr<Asset> asset = assets->GetAssetsProvider()->Open(filename, mode); if (asset) { if (out_cookie != nullptr) { *out_cookie = i; @@ -555,7 +581,8 @@ std::unique_ptr<Asset> AssetManager2::OpenNonAsset(const std::string& filename, if (cookie < 0 || static_cast<size_t>(cookie) >= apk_assets_.size()) { return {}; } - return apk_assets_[cookie]->GetAssetsProvider()->Open(filename, mode); + auto assets = apk_assets_[cookie].promote(); + return assets ? assets->GetAssetsProvider()->Open(filename, mode) : nullptr; } base::expected<FindEntryResult, NullOrIOError> AssetManager2::FindEntry( @@ -603,90 +630,97 @@ base::expected<FindEntryResult, NullOrIOError> AssetManager2::FindEntry( } bool overlaid = false; - if (!stop_at_first_match && !ignore_configuration && !apk_assets_[result->cookie]->IsLoader()) { - for (const auto& id_map : package_group.overlays_) { - auto overlay_entry = id_map.overlay_res_maps_.Lookup(resid); - if (!overlay_entry) { - // No id map entry exists for this target resource. - continue; - } - if (overlay_entry.IsInlineValue()) { - // The target resource is overlaid by an inline value not represented by a resource. - ConfigDescription best_frro_config; - Res_value best_frro_value; - bool frro_found = false; - for( const auto& [config, value] : overlay_entry.GetInlineValue()) { - if ((!frro_found || config.isBetterThan(best_frro_config, desired_config)) - && config.match(*desired_config)) { - frro_found = true; - best_frro_config = config; - best_frro_value = value; - } - } - if (!frro_found) { + if (!stop_at_first_match && !ignore_configuration) { + auto assets = apk_assets_[result->cookie].promote(); + if (!assets) { + ALOGE("Found expired ApkAssets #%d for resource ID 0x%08x.", result->cookie, resid); + return base::unexpected(std::nullopt); + } + if (!assets->IsLoader()) { + for (const auto& id_map : package_group.overlays_) { + auto overlay_entry = id_map.overlay_res_maps_.Lookup(resid); + if (!overlay_entry) { + // No id map entry exists for this target resource. continue; } - result->entry = best_frro_value; - result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); - result->cookie = id_map.cookie; - - if (UNLIKELY(logging_enabled)) { - last_resolution_.steps.push_back( - Resolution::Step{Resolution::Step::Type::OVERLAID_INLINE, String8(), result->cookie}); - if (auto path = apk_assets_[result->cookie]->GetPath()) { - const std::string overlay_path = path->data(); - if (IsFabricatedOverlay(overlay_path)) { - // FRRO don't have package name so we use the creating package here. - String8 frro_name = String8("FRRO"); - // Get the first part of it since the expected one should be like - // {overlayPackageName}-{overlayName}-{4 alphanumeric chars}.frro - // under /data/resource-cache/. - const std::string name = overlay_path.substr(overlay_path.rfind('/') + 1); - const size_t end = name.find('-'); - if (frro_name.size() != overlay_path.size() && end != std::string::npos) { - frro_name.append(base::StringPrintf(" created by %s", - name.substr(0 /* pos */, - end).c_str()).c_str()); + if (overlay_entry.IsInlineValue()) { + // The target resource is overlaid by an inline value not represented by a resource. + ConfigDescription best_frro_config; + Res_value best_frro_value; + bool frro_found = false; + for( const auto& [config, value] : overlay_entry.GetInlineValue()) { + if ((!frro_found || config.isBetterThan(best_frro_config, desired_config)) + && config.match(*desired_config)) { + frro_found = true; + best_frro_config = config; + best_frro_value = value; + } + } + if (!frro_found) { + continue; + } + result->entry = best_frro_value; + result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); + result->cookie = id_map.cookie; + + if (UNLIKELY(logging_enabled)) { + last_resolution_.steps.push_back( + Resolution::Step{Resolution::Step::Type::OVERLAID_INLINE, String8(), result->cookie}); + if (auto path = assets->GetPath()) { + const std::string overlay_path = path->data(); + if (IsFabricatedOverlay(overlay_path)) { + // FRRO don't have package name so we use the creating package here. + String8 frro_name = String8("FRRO"); + // Get the first part of it since the expected one should be like + // {overlayPackageName}-{overlayName}-{4 alphanumeric chars}.frro + // under /data/resource-cache/. + const std::string name = overlay_path.substr(overlay_path.rfind('/') + 1); + const size_t end = name.find('-'); + if (frro_name.size() != overlay_path.size() && end != std::string::npos) { + frro_name.append(base::StringPrintf(" created by %s", + name.substr(0 /* pos */, + end).c_str()).c_str()); + } + last_resolution_.best_package_name = frro_name; + } else { + last_resolution_.best_package_name = result->package_name->c_str(); } - last_resolution_.best_package_name = frro_name; - } else { - last_resolution_.best_package_name = result->package_name->c_str(); } + overlaid = true; } - overlaid = true; + continue; } - continue; - } - auto overlay_result = FindEntry(overlay_entry.GetResourceId(), density_override, - false /* stop_at_first_match */, - false /* ignore_configuration */); - if (UNLIKELY(IsIOError(overlay_result))) { - return base::unexpected(overlay_result.error()); - } - if (!overlay_result.has_value()) { - continue; - } + auto overlay_result = FindEntry(overlay_entry.GetResourceId(), density_override, + false /* stop_at_first_match */, + false /* ignore_configuration */); + if (UNLIKELY(IsIOError(overlay_result))) { + return base::unexpected(overlay_result.error()); + } + if (!overlay_result.has_value()) { + continue; + } - if (!overlay_result->config.isBetterThan(result->config, desired_config) - && overlay_result->config.compare(result->config) != 0) { - // The configuration of the entry for the overlay must be equal to or better than the target - // configuration to be chosen as the better value. - continue; - } + if (!overlay_result->config.isBetterThan(result->config, desired_config) + && overlay_result->config.compare(result->config) != 0) { + // The configuration of the entry for the overlay must be equal to or better than the target + // configuration to be chosen as the better value. + continue; + } - result->cookie = overlay_result->cookie; - result->entry = overlay_result->entry; - result->config = overlay_result->config; - result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); + result->cookie = overlay_result->cookie; + result->entry = overlay_result->entry; + result->config = overlay_result->config; + result->dynamic_ref_table = id_map.overlay_res_maps_.GetOverlayDynamicRefTable(); - if (UNLIKELY(logging_enabled)) { - last_resolution_.steps.push_back( - Resolution::Step{Resolution::Step::Type::OVERLAID, overlay_result->config.toString(), - overlay_result->cookie}); - last_resolution_.best_package_name = - overlay_result->package_name->c_str(); - overlaid = true; + if (UNLIKELY(logging_enabled)) { + last_resolution_.steps.push_back( + Resolution::Step{Resolution::Step::Type::OVERLAID, overlay_result->config.toString(), + overlay_result->cookie}); + last_resolution_.best_package_name = + overlay_result->package_name->c_str(); + overlaid = true; + } } } } @@ -868,7 +902,9 @@ std::string AssetManager2::GetLastResourceResolution() const { } const uint32_t resid = last_resolution_.resid; - const auto package = apk_assets_[cookie]->GetLoadedArsc()->GetPackageById(get_package_id(resid)); + auto assets = apk_assets_[cookie].promote(); + const auto package = + assets ? assets->GetLoadedArsc()->GetPackageById(get_package_id(resid)) : nullptr; std::string resource_name_string; if (package != nullptr) { @@ -898,8 +934,9 @@ std::string AssetManager2::GetLastResourceResolution() const { if (prefix == kStepStrings.end()) { continue; } - - log_stream << "\n\t" << prefix->second << ": " << apk_assets_[step.cookie]->GetDebugName(); + auto assets = apk_assets_[step.cookie].promote(); + log_stream << "\n\t" << prefix->second << ": " + << (assets ? assets->GetDebugName() : "<null>") << " #" << step.cookie; if (!step.config_name.isEmpty()) { log_stream << " - " << step.config_name; } @@ -1563,12 +1600,20 @@ base::expected<std::monostate, IOError> Theme::SetTo(const Theme& source) { // Determine which ApkAssets are loaded in both theme AssetManagers. const auto& src_assets = source.asset_manager_->GetApkAssets(); - for (size_t i = 0; i < src_assets.size(); i++) { - const ApkAssets* src_asset = src_assets[i]; + const auto& dest_assets = asset_manager_->GetApkAssets(); + std::vector<AssetManager2::ApkAssetsPtr> promoted_src_assets; + promoted_src_assets.reserve(src_assets.size()); + for (const auto& src_asset : src_assets) { + promoted_src_assets.emplace_back(src_asset.promote()); + } - const auto& dest_assets = asset_manager_->GetApkAssets(); - for (size_t j = 0; j < dest_assets.size(); j++) { - const ApkAssets* dest_asset = dest_assets[j]; + for (size_t j = 0; j < dest_assets.size(); j++) { + auto dest_asset = dest_assets[j].promote(); + if (!dest_asset) { + continue; + } + for (size_t i = 0; i < promoted_src_assets.size(); i++) { + const auto& src_asset = promoted_src_assets[i]; if (src_asset != dest_asset) { // ResourcesManager caches and reuses ApkAssets when the same apk must be present in // multiple AssetManagers. Two ApkAssets point to the same version of the same resources diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index c0fdfe25da2193b62d979eede693961f1363ce19..fbfae5e2bcbe34f29f50b8d2f931fefaaf71121a 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -323,7 +323,7 @@ LoadedPackage::GetEntryFromOffset(incfs::verified_map_ptr<ResTable_type> type_ch } base::expected<std::monostate, IOError> LoadedPackage::CollectConfigurations( - bool exclude_mipmap, std::set<ResTable_config>* out_configs) const {\ + bool exclude_mipmap, std::set<ResTable_config>* out_configs) const { for (const auto& type_spec : type_specs_) { if (exclude_mipmap) { const int type_idx = type_spec.first - 1; diff --git a/libs/androidfw/include/androidfw/ApkAssets.h b/libs/androidfw/include/androidfw/ApkAssets.h index 6f88f41976cdb2b6e678c35798e16f4e3d70ec04..1fa67528c78b67e06f1d18e4d94efdc600e36b32 100644 --- a/libs/androidfw/include/androidfw/ApkAssets.h +++ b/libs/androidfw/include/androidfw/ApkAssets.h @@ -17,12 +17,13 @@ #ifndef APKASSETS_H_ #define APKASSETS_H_ +#include <utils/RefBase.h> + #include <memory> #include <string> #include "android-base/macros.h" #include "android-base/unique_fd.h" - #include "androidfw/Asset.h" #include "androidfw/AssetsProvider.h" #include "androidfw/Idmap.h" @@ -31,34 +32,33 @@ namespace android { +class ApkAssets; + +using ApkAssetsPtr = sp<ApkAssets>; + // Holds an APK. -class ApkAssets { +class ApkAssets : public RefBase { public: // Creates an ApkAssets from a path on device. - static std::unique_ptr<ApkAssets> Load(const std::string& path, - package_property_t flags = 0U); + static ApkAssetsPtr Load(const std::string& path, package_property_t flags = 0U); // Creates an ApkAssets from an open file descriptor. - static std::unique_ptr<ApkAssets> LoadFromFd(base::unique_fd fd, - const std::string& debug_name, - package_property_t flags = 0U, - off64_t offset = 0, - off64_t len = AssetsProvider::kUnknownLength); + static ApkAssetsPtr LoadFromFd(base::unique_fd fd, const std::string& debug_name, + package_property_t flags = 0U, off64_t offset = 0, + off64_t len = AssetsProvider::kUnknownLength); // Creates an ApkAssets from an AssetProvider. // The ApkAssets will take care of destroying the AssetsProvider when it is destroyed. - static std::unique_ptr<ApkAssets> Load(std::unique_ptr<AssetsProvider> assets, - package_property_t flags = 0U); + static ApkAssetsPtr Load(std::unique_ptr<AssetsProvider> assets, package_property_t flags = 0U); // Creates an ApkAssets from the given asset file representing a resources.arsc. - static std::unique_ptr<ApkAssets> LoadTable(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t flags = 0U); + static ApkAssetsPtr LoadTable(std::unique_ptr<Asset> resources_asset, + std::unique_ptr<AssetsProvider> assets, + package_property_t flags = 0U); // Creates an ApkAssets from an IDMAP, which contains the original APK path, and the overlay // data. - static std::unique_ptr<ApkAssets> LoadOverlay(const std::string& idmap_path, - package_property_t flags = 0U); + static ApkAssetsPtr LoadOverlay(const std::string& idmap_path, package_property_t flags = 0U); // Path to the contents of the ApkAssets on disk. The path could represent an APk, a directory, // or some other file type. @@ -95,22 +95,27 @@ class ApkAssets { bool IsUpToDate() const; private: - static std::unique_ptr<ApkAssets> LoadImpl(std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, - std::unique_ptr<LoadedIdmap> loaded_idmap); - - static std::unique_ptr<ApkAssets> LoadImpl(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, - std::unique_ptr<LoadedIdmap> loaded_idmap); - - ApkAssets(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<LoadedArsc> loaded_arsc, - std::unique_ptr<AssetsProvider> assets, - package_property_t property_flags, - std::unique_ptr<Asset> idmap_asset, + static ApkAssetsPtr LoadImpl(std::unique_ptr<AssetsProvider> assets, + package_property_t property_flags, + std::unique_ptr<Asset> idmap_asset, + std::unique_ptr<LoadedIdmap> loaded_idmap); + + static ApkAssetsPtr LoadImpl(std::unique_ptr<Asset> resources_asset, + std::unique_ptr<AssetsProvider> assets, + package_property_t property_flags, + std::unique_ptr<Asset> idmap_asset, + std::unique_ptr<LoadedIdmap> loaded_idmap); + + // Allows us to make it possible to call make_shared from inside the class but still keeps the + // ctor 'private' for all means and purposes. + struct PrivateConstructorUtil { + explicit PrivateConstructorUtil() = default; + }; + + public: + ApkAssets(PrivateConstructorUtil, std::unique_ptr<Asset> resources_asset, + std::unique_ptr<LoadedArsc> loaded_arsc, std::unique_ptr<AssetsProvider> assets, + package_property_t property_flags, std::unique_ptr<Asset> idmap_asset, std::unique_ptr<LoadedIdmap> loaded_idmap); std::unique_ptr<Asset> resources_asset_; diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h index f10cb9bf480a1cc8dc0ab4f651b412818b7fcfa6..e276cd211ee7e8baf6e357b47b2a38a5dd22a1e9 100644 --- a/libs/androidfw/include/androidfw/AssetManager2.h +++ b/libs/androidfw/include/androidfw/AssetManager2.h @@ -17,14 +17,16 @@ #ifndef ANDROIDFW_ASSETMANAGER2_H_ #define ANDROIDFW_ASSETMANAGER2_H_ -#include "android-base/function_ref.h" -#include "android-base/macros.h" +#include <utils/RefBase.h> #include <array> #include <limits> #include <set> +#include <span> #include <unordered_map> +#include "android-base/function_ref.h" +#include "android-base/macros.h" #include "androidfw/ApkAssets.h" #include "androidfw/Asset.h" #include "androidfw/AssetManager.h" @@ -94,18 +96,25 @@ class AssetManager2 { size_t entry_len = 0u; }; - AssetManager2(); + using ApkAssetsPtr = sp<const ApkAssets>; + using ApkAssetsWPtr = wp<const ApkAssets>; + using ApkAssetsList = std::span<const ApkAssetsPtr>; + + AssetManager2() = default; explicit AssetManager2(AssetManager2&& other) = default; + AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration); + // Sets/resets the underlying ApkAssets for this AssetManager. The ApkAssets // are not owned by the AssetManager, and must have a longer lifetime. // // Only pass invalidate_caches=false when it is known that the structure // change in ApkAssets is due to a safe addition of resources with completely // new resource IDs. - bool SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool invalidate_caches = true); + bool SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches = true); + bool SetApkAssets(std::initializer_list<ApkAssetsPtr> apk_assets, bool invalidate_caches = true); - inline const std::vector<const ApkAssets*>& GetApkAssets() const { + inline const std::vector<ApkAssetsWPtr>& GetApkAssets() const { return apk_assets_; } @@ -399,7 +408,7 @@ class AssetManager2 { // Assigns package IDs to all shared library ApkAssets. // Should be called whenever the ApkAssets are changed. - void BuildDynamicRefTable(); + void BuildDynamicRefTable(ApkAssetsList assets); // Purge all resources that are cached and vary by the configuration axis denoted by the // bitmask `diff`. @@ -410,7 +419,7 @@ class AssetManager2 { void RebuildFilterList(); // Retrieves the APK paths of overlays that overlay non-system packages. - std::set<const ApkAssets*> GetNonSystemOverlays() const; + std::set<ApkAssetsPtr> GetNonSystemOverlays() const; // AssetManager2::GetBag(resid) wraps this function to track which resource ids have already // been seen while traversing bag parents. @@ -419,7 +428,7 @@ class AssetManager2 { // The ordered list of ApkAssets to search. These are not owned by the AssetManager, and must // have a longer lifetime. - std::vector<const ApkAssets*> apk_assets_; + std::vector<ApkAssetsWPtr> apk_assets_; // DynamicRefTables for shared library package resolution. // These are ordered according to apk_assets_. The mappings may change depending on what is @@ -433,7 +442,7 @@ class AssetManager2 { // The current configuration set for this AssetManager. When this changes, cached resources // may need to be purged. - ResTable_config configuration_; + ResTable_config configuration_ = {}; // Cached set of bags. These are cached because they can inherit keys from parent bags, // which involves some calculation. diff --git a/libs/androidfw/include/androidfw/MutexGuard.h b/libs/androidfw/include/androidfw/MutexGuard.h index 6fc6d64e2f6ee80500d05954ba8f40e0626be2a7..b6093dbb6d3da1a92c99d29748171d2478ce0163 100644 --- a/libs/androidfw/include/androidfw/MutexGuard.h +++ b/libs/androidfw/include/androidfw/MutexGuard.h @@ -14,12 +14,12 @@ * limitations under the License. */ -#ifndef ANDROIDFW_MUTEXGUARD_H -#define ANDROIDFW_MUTEXGUARD_H +#pragma once #include <mutex> #include <optional> #include <type_traits> +#include <utility> #include "android-base/macros.h" @@ -45,20 +45,25 @@ class ScopedLock; // template <typename T> class Guarded { - static_assert(!std::is_pointer<T>::value, "T must not be a raw pointer"); + static_assert(!std::is_pointer_v<T>, "T must not be a raw pointer"); public: - Guarded() : guarded_(std::in_place, T()) { + Guarded() : guarded_(std::in_place) { } explicit Guarded(const T& guarded) : guarded_(std::in_place, guarded) { } - explicit Guarded(T&& guarded) : guarded_(std::in_place, std::forward<T>(guarded)) { + explicit Guarded(T&& guarded) : guarded_(std::in_place, std::move(guarded)) { } - ~Guarded() { - std::lock_guard<std::mutex> scoped_lock(lock_); + // Unfortunately, some legacy designs make even class deletion race-prone, where some other + // thread may have not finished working with the same object. For those cases one may destroy the + // object under a lock (but please fix your code, at least eventually!). + template <class Func> + void safeDelete(Func f) { + std::lock_guard scoped_lock(lock_); + f(guarded_ ? &guarded_.value() : nullptr); guarded_.reset(); } @@ -96,5 +101,3 @@ class ScopedLock { }; } // namespace android - -#endif // ANDROIDFW_MUTEXGUARD_H diff --git a/libs/androidfw/tests/ApkAssets_test.cpp b/libs/androidfw/tests/ApkAssets_test.cpp index 19db25ce88116a73bf1c3378e8bc7a2ea431aad3..70326b71da282a9363ac010720e59b20c519ff73 100644 --- a/libs/androidfw/tests/ApkAssets_test.cpp +++ b/libs/androidfw/tests/ApkAssets_test.cpp @@ -35,8 +35,7 @@ using ::testing::StrEq; namespace android { TEST(ApkAssetsTest, LoadApk) { - std::unique_ptr<const ApkAssets> loaded_apk = - ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -50,7 +49,7 @@ TEST(ApkAssetsTest, LoadApkFromFd) { unique_fd fd(::open(path.c_str(), O_RDONLY | O_BINARY)); ASSERT_THAT(fd.get(), Ge(0)); - std::unique_ptr<const ApkAssets> loaded_apk = ApkAssets::LoadFromFd(std::move(fd), path); + auto loaded_apk = ApkAssets::LoadFromFd(std::move(fd), path); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -60,8 +59,7 @@ TEST(ApkAssetsTest, LoadApkFromFd) { } TEST(ApkAssetsTest, LoadApkAsSharedLibrary) { - std::unique_ptr<const ApkAssets> loaded_apk = - ApkAssets::Load(GetTestDataPath() + "/appaslib/appaslib.apk"); + auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/appaslib/appaslib.apk"); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -79,8 +77,7 @@ TEST(ApkAssetsTest, LoadApkAsSharedLibrary) { } TEST(ApkAssetsTest, CreateAndDestroyAssetKeepsApkAssetsOpen) { - std::unique_ptr<const ApkAssets> loaded_apk = - ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); ASSERT_THAT(loaded_apk, NotNull()); { ASSERT_THAT(loaded_apk->GetAssetsProvider()->Open("res/layout/main.xml", @@ -91,8 +88,7 @@ TEST(ApkAssetsTest, CreateAndDestroyAssetKeepsApkAssetsOpen) { } TEST(ApkAssetsTest, OpenUncompressedAssetFd) { - std::unique_ptr<const ApkAssets> loaded_apk = - ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); ASSERT_THAT(loaded_apk, NotNull()); auto asset = loaded_apk->GetAssetsProvider()->Open("assets/uncompressed.txt", diff --git a/libs/androidfw/tests/AssetManager2_bench.cpp b/libs/androidfw/tests/AssetManager2_bench.cpp index c7ae618991b9fbd65ea6d56021a153596b1e3900..6fae72a6d10ead5bad7f432182adcef79cd9233d 100644 --- a/libs/androidfw/tests/AssetManager2_bench.cpp +++ b/libs/androidfw/tests/AssetManager2_bench.cpp @@ -38,9 +38,9 @@ constexpr const static char* kFrameworkPath = "/system/framework/framework-res.a static void BM_AssetManagerLoadAssets(benchmark::State& state) { std::string path = GetTestDataPath() + "/basic/basic.apk"; while (state.KeepRunning()) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path); + auto apk = ApkAssets::Load(path); AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); } } BENCHMARK(BM_AssetManagerLoadAssets); @@ -61,9 +61,9 @@ BENCHMARK(BM_AssetManagerLoadAssetsOld); static void BM_AssetManagerLoadFrameworkAssets(benchmark::State& state) { std::string path = kFrameworkPath; while (state.KeepRunning()) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path); + auto apk = ApkAssets::Load(path); AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); } } BENCHMARK(BM_AssetManagerLoadFrameworkAssets); @@ -129,14 +129,14 @@ static void BM_AssetManagerGetResourceFrameworkLocaleOld(benchmark::State& state BENCHMARK(BM_AssetManagerGetResourceFrameworkLocaleOld); static void BM_AssetManagerGetBag(benchmark::State& state) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); + auto apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); while (state.KeepRunning()) { auto bag = assets.GetBag(app::R::style::StyleTwo); @@ -181,14 +181,14 @@ static void BM_AssetManagerGetBagOld(benchmark::State& state) { BENCHMARK(BM_AssetManagerGetBagOld); static void BM_AssetManagerGetResourceLocales(benchmark::State& state) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); + auto apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); while (state.KeepRunning()) { std::set<std::string> locales = @@ -217,14 +217,14 @@ static void BM_AssetManagerGetResourceLocalesOld(benchmark::State& state) { BENCHMARK(BM_AssetManagerGetResourceLocalesOld); static void BM_AssetManagerSetConfigurationFramework(benchmark::State& state) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); + auto apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); ResTable_config config; memset(&config, 0, sizeof(config)); diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp index 4394740e44ba45688c1c9ed1449a127a214c0670..5a5bafdf829a7d55bac2c608e3cd653b6c65320a 100644 --- a/libs/androidfw/tests/AssetManager2_test.cpp +++ b/libs/androidfw/tests/AssetManager2_test.cpp @@ -91,19 +91,19 @@ class AssetManager2Test : public ::testing::Test { } protected: - std::unique_ptr<const ApkAssets> basic_assets_; - std::unique_ptr<const ApkAssets> basic_de_fr_assets_; - std::unique_ptr<const ApkAssets> basic_xhdpi_assets_; - std::unique_ptr<const ApkAssets> basic_xxhdpi_assets_; - std::unique_ptr<const ApkAssets> style_assets_; - std::unique_ptr<const ApkAssets> lib_one_assets_; - std::unique_ptr<const ApkAssets> lib_two_assets_; - std::unique_ptr<const ApkAssets> libclient_assets_; - std::unique_ptr<const ApkAssets> appaslib_assets_; - std::unique_ptr<const ApkAssets> system_assets_; - std::unique_ptr<const ApkAssets> app_assets_; - std::unique_ptr<const ApkAssets> overlay_assets_; - std::unique_ptr<const ApkAssets> overlayable_assets_; + AssetManager2::ApkAssetsPtr basic_assets_; + AssetManager2::ApkAssetsPtr basic_de_fr_assets_; + AssetManager2::ApkAssetsPtr basic_xhdpi_assets_; + AssetManager2::ApkAssetsPtr basic_xxhdpi_assets_; + AssetManager2::ApkAssetsPtr style_assets_; + AssetManager2::ApkAssetsPtr lib_one_assets_; + AssetManager2::ApkAssetsPtr lib_two_assets_; + AssetManager2::ApkAssetsPtr libclient_assets_; + AssetManager2::ApkAssetsPtr appaslib_assets_; + AssetManager2::ApkAssetsPtr system_assets_; + AssetManager2::ApkAssetsPtr app_assets_; + AssetManager2::ApkAssetsPtr overlay_assets_; + AssetManager2::ApkAssetsPtr overlayable_assets_; }; TEST_F(AssetManager2Test, FindsResourceFromSingleApkAssets) { @@ -114,7 +114,7 @@ TEST_F(AssetManager2Test, FindsResourceFromSingleApkAssets) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -138,7 +138,7 @@ TEST_F(AssetManager2Test, FindsResourceFromMultipleApkAssets) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get(), basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_, basic_de_fr_assets_}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -159,8 +159,7 @@ TEST_F(AssetManager2Test, FindsResourceFromSharedLibrary) { // libclient is built with lib_one and then lib_two in order. // Reverse the order to test that proper package ID re-assignment is happening. - assetmanager.SetApkAssets( - {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); + assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); auto value = assetmanager.GetResource(libclient::R::string::foo_one); ASSERT_TRUE(value.has_value()); @@ -195,7 +194,7 @@ TEST_F(AssetManager2Test, FindsResourceFromSharedLibrary) { TEST_F(AssetManager2Test, FindsResourceFromAppLoadedAsSharedLibrary) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({appaslib_assets_.get()}); + assetmanager.SetApkAssets({appaslib_assets_}); // The appaslib package will have been assigned the package ID 0x02. auto value = assetmanager.GetResource(fix_package_id(appaslib::R::integer::number1, 0x02)); @@ -206,27 +205,26 @@ TEST_F(AssetManager2Test, FindsResourceFromAppLoadedAsSharedLibrary) { TEST_F(AssetManager2Test, AssignsOverlayPackageIdLast) { AssetManager2 assetmanager; - assetmanager.SetApkAssets( - {overlayable_assets_.get(), overlay_assets_.get(), lib_one_assets_.get()}); + assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); auto apk_assets = assetmanager.GetApkAssets(); ASSERT_EQ(3, apk_assets.size()); - ASSERT_EQ(overlayable_assets_.get(), apk_assets[0]); - ASSERT_EQ(overlay_assets_.get(), apk_assets[1]); - ASSERT_EQ(lib_one_assets_.get(), apk_assets[2]); + ASSERT_EQ(overlayable_assets_, apk_assets[0].promote()); + ASSERT_EQ(overlay_assets_, apk_assets[1].promote()); + ASSERT_EQ(lib_one_assets_, apk_assets[2].promote()); - auto get_first_package_id = [&assetmanager](const ApkAssets* apkAssets) -> uint8_t { + auto get_first_package_id = [&assetmanager](auto apkAssets) -> uint8_t { return assetmanager.GetAssignedPackageId(apkAssets->GetLoadedArsc()->GetPackages()[0].get()); }; - ASSERT_EQ(0x7f, get_first_package_id(overlayable_assets_.get())); - ASSERT_EQ(0x03, get_first_package_id(overlay_assets_.get())); - ASSERT_EQ(0x02, get_first_package_id(lib_one_assets_.get())); + ASSERT_EQ(0x7f, get_first_package_id(overlayable_assets_)); + ASSERT_EQ(0x03, get_first_package_id(overlay_assets_)); + ASSERT_EQ(0x02, get_first_package_id(lib_one_assets_)); } TEST_F(AssetManager2Test, GetSharedLibraryResourceName) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({lib_one_assets_.get()}); + assetmanager.SetApkAssets({lib_one_assets_}); auto name = assetmanager.GetResourceName(lib_one::R::string::foo); ASSERT_TRUE(name.has_value()); @@ -235,7 +233,7 @@ TEST_F(AssetManager2Test, GetSharedLibraryResourceName) { TEST_F(AssetManager2Test, GetResourceNameNonMatchingConfig) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({basic_de_fr_assets_}); auto value = assetmanager.GetResourceName(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -244,7 +242,7 @@ TEST_F(AssetManager2Test, GetResourceNameNonMatchingConfig) { TEST_F(AssetManager2Test, GetResourceTypeSpecFlags) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({basic_de_fr_assets_}); auto value = assetmanager.GetResourceTypeSpecFlags(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -253,7 +251,7 @@ TEST_F(AssetManager2Test, GetResourceTypeSpecFlags) { TEST_F(AssetManager2Test, FindsBagResourceFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto bag = assetmanager.GetBag(basic::R::array::integerArray1); ASSERT_TRUE(bag.has_value()); @@ -280,8 +278,7 @@ TEST_F(AssetManager2Test, FindsBagResourceFromSharedLibrary) { // libclient is built with lib_one and then lib_two in order. // Reverse the order to test that proper package ID re-assignment is happening. - assetmanager.SetApkAssets( - {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); + assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); auto bag = assetmanager.GetBag(fix_package_id(lib_one::R::style::Theme, 0x03)); ASSERT_TRUE(bag.has_value()); @@ -300,8 +297,7 @@ TEST_F(AssetManager2Test, FindsBagResourceFromMultipleSharedLibraries) { // libclient is built with lib_one and then lib_two in order. // Reverse the order to test that proper package ID re-assignment is happening. - assetmanager.SetApkAssets( - {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); + assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); auto bag = assetmanager.GetBag(libclient::R::style::ThemeMultiLib); ASSERT_TRUE(bag.has_value()); @@ -321,8 +317,7 @@ TEST_F(AssetManager2Test, FindsStyleResourceWithParentFromSharedLibrary) { // libclient is built with lib_one and then lib_two in order. // Reverse the order to test that proper package ID re-assignment is happening. - assetmanager.SetApkAssets( - {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); + assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); auto bag = assetmanager.GetBag(libclient::R::style::Theme); ASSERT_TRUE(bag.has_value()); @@ -337,7 +332,7 @@ TEST_F(AssetManager2Test, FindsStyleResourceWithParentFromSharedLibrary) { TEST_F(AssetManager2Test, MergesStylesWithParentFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); auto bag_one = assetmanager.GetBag(app::R::style::StyleOne); ASSERT_TRUE(bag_one.has_value()); @@ -401,7 +396,7 @@ TEST_F(AssetManager2Test, MergesStylesWithParentFromSingleApkAssets) { TEST_F(AssetManager2Test, MergeStylesCircularDependency) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); // GetBag should stop traversing the parents of styles when a circular // dependency is detected @@ -412,7 +407,7 @@ TEST_F(AssetManager2Test, MergeStylesCircularDependency) { TEST_F(AssetManager2Test, ResolveReferenceToResource) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto value = assetmanager.GetResource(basic::R::integer::ref1); ASSERT_TRUE(value.has_value()); @@ -428,7 +423,7 @@ TEST_F(AssetManager2Test, ResolveReferenceToResource) { TEST_F(AssetManager2Test, ResolveReferenceToBag) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto value = assetmanager.GetResource(basic::R::integer::number2, true /*may_be_bag*/); ASSERT_TRUE(value.has_value()); @@ -444,7 +439,7 @@ TEST_F(AssetManager2Test, ResolveReferenceToBag) { TEST_F(AssetManager2Test, ResolveDeepIdReference) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); // Set up the resource ids auto high_ref = assetmanager.GetResourceId("@id/high_ref", "values", "com.android.basic"); @@ -470,8 +465,7 @@ TEST_F(AssetManager2Test, ResolveDeepIdReference) { TEST_F(AssetManager2Test, DensityOverride) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get(), basic_xhdpi_assets_.get(), - basic_xxhdpi_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_, basic_xhdpi_assets_, basic_xxhdpi_assets_}); assetmanager.SetConfiguration({ .density = ResTable_config::DENSITY_XHIGH, .sdkVersion = 21, @@ -493,7 +487,7 @@ TEST_F(AssetManager2Test, DensityOverride) { TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); // Create some kind of value that is NOT a reference. AssetManager2::SelectedValue value{}; @@ -509,7 +503,7 @@ TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved) TEST_F(AssetManager2Test, ResolveReferenceMissingResourceDoNotCacheFlags) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); { AssetManager2::SelectedValue value{}; value.data = basic::R::string::test1; @@ -540,7 +534,7 @@ TEST_F(AssetManager2Test, ResolveReferenceMissingResourceDoNotCacheFlags) { TEST_F(AssetManager2Test, ResolveReferenceMissingResource) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); const uint32_t kMissingResId = 0x8001ffff; AssetManager2::SelectedValue value{}; @@ -558,7 +552,7 @@ TEST_F(AssetManager2Test, ResolveReferenceMissingResource) { TEST_F(AssetManager2Test, ResolveReferenceMissingResourceLib) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({libclient_assets_.get()}); + assetmanager.SetApkAssets({libclient_assets_}); AssetManager2::SelectedValue value{}; value.type = Res_value::TYPE_REFERENCE; @@ -580,7 +574,7 @@ static bool IsConfigurationPresent(const std::set<ResTable_config>& configuratio TEST_F(AssetManager2Test, GetResourceConfigurations) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get(), basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({system_assets_, basic_de_fr_assets_}); auto configurations = assetmanager.GetResourceConfigurations(); ASSERT_TRUE(configurations.has_value()); @@ -625,7 +619,7 @@ TEST_F(AssetManager2Test, GetResourceConfigurations) { TEST_F(AssetManager2Test, GetResourceLocales) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get(), basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({system_assets_, basic_de_fr_assets_}); std::set<std::string> locales = assetmanager.GetResourceLocales(); @@ -644,7 +638,7 @@ TEST_F(AssetManager2Test, GetResourceLocales) { TEST_F(AssetManager2Test, GetResourceId) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto resid = assetmanager.GetResourceId("com.android.basic:layout/main", "", ""); ASSERT_TRUE(resid.has_value()); @@ -661,7 +655,7 @@ TEST_F(AssetManager2Test, GetResourceId) { TEST_F(AssetManager2Test, OpensFileFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get()}); + assetmanager.SetApkAssets({system_assets_}); std::unique_ptr<Asset> asset = assetmanager.Open("file.txt", Asset::ACCESS_BUFFER); ASSERT_THAT(asset, NotNull()); @@ -673,7 +667,7 @@ TEST_F(AssetManager2Test, OpensFileFromSingleApkAssets) { TEST_F(AssetManager2Test, OpensFileFromMultipleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get(), app_assets_.get()}); + assetmanager.SetApkAssets({system_assets_, app_assets_}); std::unique_ptr<Asset> asset = assetmanager.Open("file.txt", Asset::ACCESS_BUFFER); ASSERT_THAT(asset, NotNull()); @@ -685,7 +679,7 @@ TEST_F(AssetManager2Test, OpensFileFromMultipleApkAssets) { TEST_F(AssetManager2Test, OpenDir) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get()}); + assetmanager.SetApkAssets({system_assets_}); std::unique_ptr<AssetDir> asset_dir = assetmanager.OpenDir(""); ASSERT_THAT(asset_dir, NotNull()); @@ -707,7 +701,7 @@ TEST_F(AssetManager2Test, OpenDir) { TEST_F(AssetManager2Test, OpenDirFromManyApks) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_.get(), app_assets_.get()}); + assetmanager.SetApkAssets({system_assets_, app_assets_}); std::unique_ptr<AssetDir> asset_dir = assetmanager.OpenDir(""); ASSERT_THAT(asset_dir, NotNull()); @@ -728,7 +722,7 @@ TEST_F(AssetManager2Test, GetLastPathWithoutEnablingReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); assetmanager.SetResourceResolutionLoggingEnabled(false); auto value = assetmanager.GetResource(basic::R::string::test1); @@ -743,7 +737,7 @@ TEST_F(AssetManager2Test, GetLastPathWithoutResolutionReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto result = assetmanager.GetLastResourceResolution(); EXPECT_EQ("", result); @@ -758,17 +752,18 @@ TEST_F(AssetManager2Test, GetLastPathWithSingleApkAssets) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); auto result = assetmanager.GetLastResourceResolution(); - EXPECT_EQ("Resolution for 0x7f030000 com.android.basic:string/test1\n" - "\tFor config - de\n" - "\tFound initial: basic/basic.apk\n" - "Best matching is from default configuration of com.android.basic", - result); + EXPECT_EQ( + "Resolution for 0x7f030000 com.android.basic:string/test1\n" + "\tFor config - de\n" + "\tFound initial: basic/basic.apk #0\n" + "Best matching is from default configuration of com.android.basic", + result); } TEST_F(AssetManager2Test, GetLastPathWithMultipleApkAssets) { @@ -780,18 +775,19 @@ TEST_F(AssetManager2Test, GetLastPathWithMultipleApkAssets) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get(), basic_de_fr_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_, basic_de_fr_assets_}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); auto result = assetmanager.GetLastResourceResolution(); - EXPECT_EQ("Resolution for 0x7f030000 com.android.basic:string/test1\n" - "\tFor config - de\n" - "\tFound initial: basic/basic.apk\n" - "\tFound better: basic/basic_de_fr.apk - de\n" - "Best matching is from de configuration of com.android.basic", - result); + EXPECT_EQ( + "Resolution for 0x7f030000 com.android.basic:string/test1\n" + "\tFor config - de\n" + "\tFound initial: basic/basic.apk #0\n" + "\tFound better: basic/basic_de_fr.apk #1 - de\n" + "Best matching is from de configuration of com.android.basic", + result); } TEST_F(AssetManager2Test, GetLastPathAfterDisablingReturnsEmpty) { @@ -801,7 +797,7 @@ TEST_F(AssetManager2Test, GetLastPathAfterDisablingReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_.get()}); + assetmanager.SetApkAssets({basic_assets_}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -822,7 +818,7 @@ TEST_F(AssetManager2Test, GetOverlayablesToString) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({overlayable_assets_.get()}); + assetmanager.SetApkAssets({overlayable_assets_}); const auto map = assetmanager.GetOverlayableMapForPackage(0x7f); ASSERT_NE(nullptr, map); diff --git a/libs/androidfw/tests/AttributeResolution_bench.cpp b/libs/androidfw/tests/AttributeResolution_bench.cpp index 1c89c61c8f7847c709ba13cb3d08306a288cbf61..384f4a78b36d186ea144c36a1b476cdae379280d 100644 --- a/libs/androidfw/tests/AttributeResolution_bench.cpp +++ b/libs/androidfw/tests/AttributeResolution_bench.cpp @@ -36,15 +36,14 @@ constexpr const static char* kFrameworkPath = "/system/framework/framework-res.a constexpr const static uint32_t Theme_Material_Light = 0x01030237u; static void BM_ApplyStyle(benchmark::State& state) { - std::unique_ptr<const ApkAssets> styles_apk = - ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); + auto styles_apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); if (styles_apk == nullptr) { state.SkipWithError("failed to load assets"); return; } AssetManager2 assetmanager; - assetmanager.SetApkAssets({styles_apk.get()}); + assetmanager.SetApkAssets({styles_apk}); std::unique_ptr<Asset> asset = assetmanager.OpenNonAsset("res/layout/layout.xml", Asset::ACCESS_BUFFER); @@ -80,21 +79,20 @@ static void BM_ApplyStyle(benchmark::State& state) { BENCHMARK(BM_ApplyStyle); static void BM_ApplyStyleFramework(benchmark::State& state) { - std::unique_ptr<const ApkAssets> framework_apk = ApkAssets::Load(kFrameworkPath); + auto framework_apk = ApkAssets::Load(kFrameworkPath); if (framework_apk == nullptr) { state.SkipWithError("failed to load framework assets"); return; } - std::unique_ptr<const ApkAssets> basic_apk = - ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + auto basic_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); if (basic_apk == nullptr) { state.SkipWithError("failed to load assets"); return; } AssetManager2 assetmanager; - assetmanager.SetApkAssets({framework_apk.get(), basic_apk.get()}); + assetmanager.SetApkAssets({framework_apk, basic_apk}); ResTable_config device_config; memset(&device_config, 0, sizeof(device_config)); diff --git a/libs/androidfw/tests/AttributeResolution_test.cpp b/libs/androidfw/tests/AttributeResolution_test.cpp index bb9129ad01c83aa22d942ce21ca6f292a689f936..329830fa47b2502ee48d565a0cef100e5edab100 100644 --- a/libs/androidfw/tests/AttributeResolution_test.cpp +++ b/libs/androidfw/tests/AttributeResolution_test.cpp @@ -36,11 +36,11 @@ class AttributeResolutionTest : public ::testing::Test { virtual void SetUp() override { styles_assets_ = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); ASSERT_NE(nullptr, styles_assets_); - assetmanager_.SetApkAssets({styles_assets_.get()}); + assetmanager_.SetApkAssets({styles_assets_}); } protected: - std::unique_ptr<const ApkAssets> styles_assets_; + AssetManager2::ApkAssetsPtr styles_assets_; AssetManager2 assetmanager_; }; @@ -69,7 +69,7 @@ TEST(AttributeResolutionLibraryTest, ApplyStyleWithDefaultStyleResId) { AssetManager2 assetmanager; auto apk_assets = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk", PROPERTY_DYNAMIC); ASSERT_NE(nullptr, apk_assets); - assetmanager.SetApkAssets({apk_assets.get()}); + assetmanager.SetApkAssets({apk_assets}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); diff --git a/libs/androidfw/tests/BenchmarkHelpers.cpp b/libs/androidfw/tests/BenchmarkHelpers.cpp index 0fa0573bcbb807d8caea97a8fe311ef47a556f50..b97dd96f893422a42690f9d8fc6a65df0eca501b 100644 --- a/libs/androidfw/tests/BenchmarkHelpers.cpp +++ b/libs/androidfw/tests/BenchmarkHelpers.cpp @@ -53,20 +53,18 @@ void GetResourceBenchmarkOld(const std::vector<std::string>& paths, const ResTab void GetResourceBenchmark(const std::vector<std::string>& paths, const ResTable_config* config, uint32_t resid, benchmark::State& state) { - std::vector<std::unique_ptr<const ApkAssets>> apk_assets; - std::vector<const ApkAssets*> apk_assets_ptrs; + std::vector<AssetManager2::ApkAssetsPtr> apk_assets; for (const std::string& path : paths) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path); + auto apk = ApkAssets::Load(path); if (apk == nullptr) { state.SkipWithError(base::StringPrintf("Failed to load assets %s", path.c_str()).c_str()); return; } - apk_assets_ptrs.push_back(apk.get()); apk_assets.push_back(std::move(apk)); } AssetManager2 assetmanager; - assetmanager.SetApkAssets(apk_assets_ptrs); + assetmanager.SetApkAssets(apk_assets); if (config != nullptr) { assetmanager.SetConfiguration(*config); } diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp index b43491548e2b3e99184aa1e88af0df3244b2ca7b..568e041ebe5bdaead095f699d64c00229cca089d 100644 --- a/libs/androidfw/tests/Idmap_test.cpp +++ b/libs/androidfw/tests/Idmap_test.cpp @@ -59,15 +59,16 @@ class IdmapTest : public ::testing::Test { protected: std::string original_path; - std::unique_ptr<const ApkAssets> system_assets_; - std::unique_ptr<const ApkAssets> overlay_assets_; - std::unique_ptr<const ApkAssets> overlayable_assets_; + AssetManager2::ApkAssetsPtr system_assets_; + AssetManager2::ApkAssetsPtr overlay_assets_; + AssetManager2::ApkAssetsPtr overlayable_assets_; }; std::string GetStringFromApkAssets(const AssetManager2& asset_manager, const AssetManager2::SelectedValue& value) { auto assets = asset_manager.GetApkAssets(); - const ResStringPool* string_pool = assets[value.cookie]->GetLoadedArsc()->GetStringPool(); + const ResStringPool* string_pool = + assets[value.cookie].promote()->GetLoadedArsc()->GetStringPool(); return GetStringFromPool(string_pool, value.data); } @@ -75,8 +76,7 @@ std::string GetStringFromApkAssets(const AssetManager2& asset_manager, TEST_F(IdmapTest, OverlayOverridesResourceValue) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable5); ASSERT_TRUE(value.has_value()); @@ -87,8 +87,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValue) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingDifferentPackage) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable10); ASSERT_TRUE(value.has_value()); @@ -99,8 +98,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingDifferentPackage) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInternalResource) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable8); ASSERT_TRUE(value.has_value()); @@ -111,8 +109,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInternalResource) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineInteger) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::integer::config_integer); ASSERT_TRUE(value.has_value()); @@ -123,8 +120,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineInteger) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineString) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable11); ASSERT_TRUE(value.has_value()); @@ -135,8 +131,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineString) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingOverlayingResource) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable9); ASSERT_TRUE(value.has_value()); @@ -147,8 +142,7 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingOverlayingResource) { TEST_F(IdmapTest, OverlayOverridesXmlParser) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::layout::hello_view); ASSERT_TRUE(value.has_value()); @@ -186,8 +180,7 @@ TEST_F(IdmapTest, OverlayOverridesXmlParser) { TEST_F(IdmapTest, OverlaidResourceHasSameName) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); auto name = asset_manager.GetResourceName(overlayable::R::string::overlayable9); ASSERT_TRUE(name.has_value()); @@ -203,8 +196,7 @@ TEST_F(IdmapTest, OverlayLoaderInterop) { auto loader_assets = ApkAssets::LoadTable(std::move(asset), EmptyAssetsProvider::Create(), PROPERTY_LOADER); AssetManager2 asset_manager; - asset_manager.SetApkAssets({overlayable_assets_.get(), loader_assets.get(), - overlay_assets_.get()}); + asset_manager.SetApkAssets({overlayable_assets_, loader_assets, overlay_assets_}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable11); ASSERT_TRUE(value.has_value()); diff --git a/libs/androidfw/tests/Theme_bench.cpp b/libs/androidfw/tests/Theme_bench.cpp index f3d60bbe4f1569fe6ee55f59d0d2d195bb8cebe6..dfbb5a76dec67e9bfde6d518664a129ec10c04a2 100644 --- a/libs/androidfw/tests/Theme_bench.cpp +++ b/libs/androidfw/tests/Theme_bench.cpp @@ -28,14 +28,14 @@ constexpr const static uint32_t kStyleId = 0x01030237u; // android:style/Theme. constexpr const static uint32_t kAttrId = 0x01010030u; // android:attr/colorForeground static void BM_ThemeApplyStyleFramework(benchmark::State& state) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); + auto apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); while (state.KeepRunning()) { auto theme = assets.NewTheme(); @@ -62,10 +62,10 @@ static void BM_ThemeApplyStyleFrameworkOld(benchmark::State& state) { BENCHMARK(BM_ThemeApplyStyleFrameworkOld); static void BM_ThemeGetAttribute(benchmark::State& state) { - std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); + auto apk = ApkAssets::Load(kFrameworkPath); AssetManager2 assets; - assets.SetApkAssets({apk.get()}); + assets.SetApkAssets({apk}); auto theme = assets.NewTheme(); theme->ApplyStyle(kStyleId, false /* force */); diff --git a/libs/androidfw/tests/Theme_test.cpp b/libs/androidfw/tests/Theme_test.cpp index 77114f273d3da908776e23c3853143980e21377c..e08a6a7f277dc92c430fe5777ec378126fdaf072 100644 --- a/libs/androidfw/tests/Theme_test.cpp +++ b/libs/androidfw/tests/Theme_test.cpp @@ -53,16 +53,16 @@ class ThemeTest : public ::testing::Test { } protected: - std::unique_ptr<const ApkAssets> system_assets_; - std::unique_ptr<const ApkAssets> style_assets_; - std::unique_ptr<const ApkAssets> libclient_assets_; - std::unique_ptr<const ApkAssets> lib_one_assets_; - std::unique_ptr<const ApkAssets> lib_two_assets_; + AssetManager2::ApkAssetsPtr system_assets_; + AssetManager2::ApkAssetsPtr style_assets_; + AssetManager2::ApkAssetsPtr libclient_assets_; + AssetManager2::ApkAssetsPtr lib_one_assets_; + AssetManager2::ApkAssetsPtr lib_two_assets_; }; TEST_F(ThemeTest, EmptyTheme) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); EXPECT_EQ(0u, theme->GetChangingConfigurations()); @@ -72,7 +72,7 @@ TEST_F(ThemeTest, EmptyTheme) { TEST_F(ThemeTest, SingleThemeNoParent) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleOne).has_value()); @@ -92,7 +92,7 @@ TEST_F(ThemeTest, SingleThemeNoParent) { TEST_F(ThemeTest, SingleThemeWithParent) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo).has_value()); @@ -121,7 +121,7 @@ TEST_F(ThemeTest, SingleThemeWithParent) { TEST_F(ThemeTest, TryToUseBadResourceId) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo).has_value()); @@ -130,7 +130,7 @@ TEST_F(ThemeTest, TryToUseBadResourceId) { TEST_F(ThemeTest, MultipleThemesOverlaidNotForce) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo).has_value()); @@ -160,7 +160,7 @@ TEST_F(ThemeTest, MultipleThemesOverlaidNotForce) { TEST_F(ThemeTest, MultipleThemesOverlaidForced) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo).has_value()); @@ -190,8 +190,7 @@ TEST_F(ThemeTest, MultipleThemesOverlaidForced) { TEST_F(ThemeTest, ResolveDynamicAttributesAndReferencesToSharedLibrary) { AssetManager2 assetmanager; - assetmanager.SetApkAssets( - {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); + assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(libclient::R::style::Theme, false /*force*/).has_value()); @@ -216,7 +215,7 @@ TEST_F(ThemeTest, ResolveDynamicAttributesAndReferencesToSharedLibrary) { TEST_F(ThemeTest, CopyThemeSameAssetManager) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_.get()}); + assetmanager.SetApkAssets({style_assets_}); std::unique_ptr<Theme> theme_one = assetmanager.NewTheme(); ASSERT_TRUE(theme_one->ApplyStyle(app::R::style::StyleOne).has_value()); @@ -253,10 +252,10 @@ TEST_F(ThemeTest, CopyThemeSameAssetManager) { TEST_F(ThemeTest, ThemeRebase) { AssetManager2 am; - am.SetApkAssets({style_assets_.get()}); + am.SetApkAssets({style_assets_}); AssetManager2 am_night; - am_night.SetApkAssets({style_assets_.get()}); + am_night.SetApkAssets({style_assets_}); ResTable_config night{}; night.uiMode = ResTable_config::UI_MODE_NIGHT_YES; @@ -327,12 +326,11 @@ TEST_F(ThemeTest, ThemeRebase) { TEST_F(ThemeTest, OnlyCopySameAssetsThemeWhenAssetManagersDiffer) { AssetManager2 assetmanager_dst; - assetmanager_dst.SetApkAssets({system_assets_.get(), lib_one_assets_.get(), style_assets_.get(), - libclient_assets_.get()}); + assetmanager_dst.SetApkAssets( + {system_assets_, lib_one_assets_, style_assets_, libclient_assets_}); AssetManager2 assetmanager_src; - assetmanager_src.SetApkAssets({system_assets_.get(), lib_two_assets_.get(), lib_one_assets_.get(), - style_assets_.get()}); + assetmanager_src.SetApkAssets({system_assets_, lib_two_assets_, lib_one_assets_, style_assets_}); auto theme_dst = assetmanager_dst.NewTheme(); ASSERT_TRUE(theme_dst->ApplyStyle(app::R::style::StyleOne).has_value()); @@ -376,10 +374,10 @@ TEST_F(ThemeTest, OnlyCopySameAssetsThemeWhenAssetManagersDiffer) { TEST_F(ThemeTest, CopyNonReferencesWhenPackagesDiffer) { AssetManager2 assetmanager_dst; - assetmanager_dst.SetApkAssets({system_assets_.get()}); + assetmanager_dst.SetApkAssets({system_assets_}); AssetManager2 assetmanager_src; - assetmanager_src.SetApkAssets({system_assets_.get(), style_assets_.get()}); + assetmanager_src.SetApkAssets({system_assets_, style_assets_}); auto theme_dst = assetmanager_dst.NewTheme(); auto theme_src = assetmanager_src.NewTheme(); diff --git a/rs/jni/Android.bp b/rs/jni/Android.bp index 8a6897c055c5dfdfb1358ea63dc555d105b88d1d..f732c216c25061a79c36419677b6d604d8bfccbc 100644 --- a/rs/jni/Android.bp +++ b/rs/jni/Android.bp @@ -22,6 +22,7 @@ package { cc_library_shared { name: "librs_jni", + cpp_std: "gnu++2b", srcs: ["android_renderscript_RenderScript.cpp"], shared_libs: [ diff --git a/startop/view_compiler/Android.bp b/startop/view_compiler/Android.bp index 90239212f66515ec03e0c25c1b0492edb58e6d1f..e17209088750dc8bbfba5ff0657739bcf964ba5b 100644 --- a/startop/view_compiler/Android.bp +++ b/startop/view_compiler/Android.bp @@ -40,7 +40,7 @@ cc_defaults { "libziparchive", "libz", ], - cppflags: ["-std=c++17"], + cpp_std: "gnu++2b", target: { android: { shared_libs: [ @@ -80,7 +80,7 @@ cc_binary { "libgflags", "libviewcompiler", ], - host_supported: true + host_supported: true, } cc_test_host { diff --git a/startop/view_compiler/apk_layout_compiler.cc b/startop/view_compiler/apk_layout_compiler.cc index 1d3b648175cce97158522bfb8097a6727e7b22ca..5f5652c2acac80a9e7e78540621848f096065eae 100644 --- a/startop/view_compiler/apk_layout_compiler.cc +++ b/startop/view_compiler/apk_layout_compiler.cc @@ -80,10 +80,10 @@ bool CanCompileLayout(ResXMLParser* parser) { } namespace { -void CompileApkAssetsLayouts(const std::unique_ptr<android::ApkAssets>& assets, - CompilationTarget target, std::ostream& target_out) { +void CompileApkAssetsLayouts(const android::ApkAssetsPtr& assets, CompilationTarget target, + std::ostream& target_out) { android::AssetManager2 resources; - resources.SetApkAssets({assets.get()}); + resources.SetApkAssets({assets}); std::string package_name; diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp index 28fcc1a4800ec1350aa8febe4f03a7eb72f1bdec..725a1b86f6169dc9d66ed679a715f6a9a47bee55 100644 --- a/tools/aapt2/cmd/Link_test.cpp +++ b/tools/aapt2/cmd/Link_test.cpp @@ -575,7 +575,7 @@ TEST_F(LinkTest, StagedAndroidApi) { android::AssetManager2 am; auto android_asset = android::ApkAssets::Load(android_apk); ASSERT_THAT(android_asset, NotNull()); - ASSERT_TRUE(am.SetApkAssets({android_asset.get()})); + ASSERT_TRUE(am.SetApkAssets({android_asset})); auto result = am.GetResourceId("android:attr/finalized_res"); ASSERT_TRUE(result.has_value()); @@ -631,7 +631,7 @@ TEST_F(LinkTest, FinalizedAndroidApi) { auto app_against_non_final = android::ApkAssets::Load(app_apk); ASSERT_THAT(android_asset, NotNull()); ASSERT_THAT(app_against_non_final, NotNull()); - ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_non_final.get()})); + ASSERT_TRUE(am.SetApkAssets({android_asset, app_against_non_final})); auto result = am.GetResourceId("android:attr/finalized_res"); ASSERT_TRUE(result.has_value()); @@ -667,7 +667,7 @@ TEST_F(LinkTest, FinalizedAndroidApi) { auto app_against_final = android::ApkAssets::Load(app_apk_respin); ASSERT_THAT(app_against_final, NotNull()); - ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_final.get()})); + ASSERT_TRUE(am.SetApkAssets({android_asset, app_against_final})); { auto style = am.GetBag(0x7f020000); diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index bca62da447b0b27627c46c0ea75469e1b3c77465..b75458a7b8b67afa5e6e0b42b8c1132ecca26282 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -220,15 +220,9 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName( bool AssetManagerSymbolSource::AddAssetPath(StringPiece path) { TRACE_CALL(); - if (std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path.data())) { + if (auto apk = ApkAssets::Load(path.data())) { apk_assets_.push_back(std::move(apk)); - - std::vector<const ApkAssets*> apk_assets; - for (const std::unique_ptr<const ApkAssets>& apk_asset : apk_assets_) { - apk_assets.push_back(apk_asset.get()); - } - - asset_manager_.SetApkAssets(apk_assets); + asset_manager_.SetApkAssets(apk_assets_); return true; } return false; @@ -251,7 +245,7 @@ bool AssetManagerSymbolSource::IsPackageDynamic(uint32_t packageId, return true; } - for (const std::unique_ptr<const ApkAssets>& assets : apk_assets_) { + for (auto&& assets : apk_assets_) { for (const std::unique_ptr<const android::LoadedPackage>& loaded_package : assets->GetLoadedArsc()->GetPackages()) { if (package_name == loaded_package->GetPackageName() && loaded_package->IsDynamic()) { diff --git a/tools/aapt2/process/SymbolTable.h b/tools/aapt2/process/SymbolTable.h index b09ff702ca5889fd0cf7506e69e5b57ce99e4cc6..36eb0bab604668b1c561bc0a7d78f97569a1a756 100644 --- a/tools/aapt2/process/SymbolTable.h +++ b/tools/aapt2/process/SymbolTable.h @@ -207,8 +207,8 @@ class AssetManagerSymbolSource : public ISymbolSource { } private: + std::vector<android::AssetManager2::ApkAssetsPtr> apk_assets_; android::AssetManager2 asset_manager_; - std::vector<std::unique_ptr<const android::ApkAssets>> apk_assets_; DISALLOW_COPY_AND_ASSIGN(AssetManagerSymbolSource); };