From cf6e79f809034386b04ae551db815ab087f76c0a Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi <zyy@google.com> Date: Wed, 26 Apr 2023 16:20:09 +0000 Subject: [PATCH] Revert "Use reference counted pointers for ApkAssets" This reverts commit c357f719ed3734aee05418808699a48dd150a1a5. Reason for revert: b/279154343 - performance regression Change-Id: If2e212d8fc5b9ed8638032a33f450440cbaeceb0 --- cmds/idmap2/idmap2/Lookup.cpp | 31 ++-- cmds/idmap2/libidmap2/ResourceContainer.cpp | 4 +- cmds/idmap2/tests/ResourceUtilsTests.cpp | 4 +- core/jni/android_content_res_ApkAssets.cpp | 48 ++---- core/jni/android_content_res_ApkAssets.h | 4 +- core/jni/android_util_AssetManager.cpp | 50 +++--- libs/androidfw/ApkAssets.cpp | 58 +++---- libs/androidfw/AssetManager2.cpp | 147 +++++++----------- libs/androidfw/include/androidfw/ApkAssets.h | 69 ++++---- .../include/androidfw/AssetManager2.h | 27 ++-- libs/androidfw/include/androidfw/MutexGuard.h | 21 ++- libs/androidfw/tests/ApkAssets_test.cpp | 14 +- libs/androidfw/tests/AssetManager2_bench.cpp | 20 +-- libs/androidfw/tests/AssetManager2_test.cpp | 140 +++++++++-------- .../tests/AttributeResolution_bench.cpp | 12 +- .../tests/AttributeResolution_test.cpp | 6 +- libs/androidfw/tests/BenchmarkHelpers.cpp | 8 +- libs/androidfw/tests/Idmap_test.cpp | 36 +++-- libs/androidfw/tests/Theme_bench.cpp | 8 +- libs/androidfw/tests/Theme_test.cpp | 42 ++--- rs/jni/Android.bp | 1 - startop/view_compiler/Android.bp | 4 +- startop/view_compiler/apk_layout_compiler.cc | 6 +- tools/aapt2/cmd/Link_test.cpp | 6 +- tools/aapt2/process/SymbolTable.cpp | 12 +- tools/aapt2/process/SymbolTable.h | 2 +- 26 files changed, 362 insertions(+), 418 deletions(-) diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp index add0d8d48108..f41e57cc66d6 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<AssetManager2::ApkAssetsPtr> apk_assets; + std::vector<std::unique_ptr<const ApkAssets>> apk_assets; std::string target_path; std::string target_package_name; for (size_t i = 0; i < idmap_paths.size(); i++) { @@ -217,21 +217,24 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { apk_assets.push_back(std::move(overlay_apk)); } - { - // 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"); - } + // 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"); + } - 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; + 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; return Unit{}; } diff --git a/cmds/idmap2/libidmap2/ResourceContainer.cpp b/cmds/idmap2/libidmap2/ResourceContainer.cpp index 7869fbdb8cea..0e3590486c6f 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 { - AssetManager2::ApkAssetsPtr apk_assets; + std::unique_ptr<ApkAssets> 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})) { + if (!state.am->SetApkAssets({state.apk_assets.get()})) { return Error("failed to create asset manager"); } diff --git a/cmds/idmap2/tests/ResourceUtilsTests.cpp b/cmds/idmap2/tests/ResourceUtilsTests.cpp index 011040ba0ebf..69142086765c 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_}); + am_.SetApkAssets({apk_assets_.get()}); } const AssetManager2& GetAssetManager() { @@ -47,7 +47,7 @@ class ResourceUtilsTests : public Idmap2Tests { private: AssetManager2 am_; - AssetManager2::ApkAssetsPtr apk_assets_; + std::unique_ptr<const ApkAssets> 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 6fc0519f70a1..e9ada235b388 100644 --- a/core/jni/android_content_res_ApkAssets.cpp +++ b/core/jni/android_content_res_ApkAssets.cpp @@ -74,37 +74,17 @@ enum : format_type_t { FORMAT_DIRECTORY = 3, }; -Guarded<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr) { - return *reinterpret_cast<Guarded<AssetManager2::ApkAssetsPtr>*>(ptr); +Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr) { + return *reinterpret_cast<Guarded<std::unique_ptr<const ApkAssets>>*>(ptr); } -static jlong CreateGuardedApkAssets(AssetManager2::ApkAssetsPtr assets) { - auto guarded_assets = new Guarded<AssetManager2::ApkAssetsPtr>(std::move(assets)); - return reinterpret_cast<jlong>(guarded_assets); +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 void DeleteGuardedApkAssets(Guarded<AssetManager2::ApkAssetsPtr>& apk_assets) { - apk_assets.safeDelete([&apk_assets](AssetManager2::ApkAssetsPtr* assets) { - if (!assets) { - ALOGE("ApkAssets: Double delete of native assets object %p, ignored", &apk_assets); - } else if (!*assets) { - ALOGE("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) { - ALOGE("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) { - ALOGE("ApkAssets: Deleting an ApkAssets object '%s' with %d weak references", - (*assets)->GetDebugName().c_str(), int(weakCount)); - } - } - }); - delete &apk_assets; +static void DeleteGuardedApkAssets(Guarded<std::unique_ptr<const ApkAssets>>& apk_assets) { + delete &apk_assets; } class LoaderAssetsProvider : public AssetsProvider { @@ -229,7 +209,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); - AssetManager2::ApkAssetsPtr apk_assets; + std::unique_ptr<ApkAssets> apk_assets; switch (format) { case FORMAT_APK: { auto assets = MultiAssetsProvider::Create(std::move(loader_assets), @@ -289,7 +269,7 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t } auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider); - AssetManager2::ApkAssetsPtr apk_assets; + std::unique_ptr<const ApkAssets> apk_assets; switch (format) { case FORMAT_APK: { auto assets = @@ -356,7 +336,7 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ } auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider); - AssetManager2::ApkAssetsPtr apk_assets; + std::unique_ptr<const ApkAssets> apk_assets; switch (format) { case FORMAT_APK: { auto assets = @@ -394,17 +374,11 @@ 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 8159a53caaa5..7e525dc75ef0 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<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr); +Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr); } // namespace android diff --git a/core/jni/android_util_AssetManager.cpp b/core/jni/android_util_AssetManager.cpp index e46a0e0062a5..a2205eb7dfdb 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<AssetManager2::ApkAssetsPtr> apk_assets; + std::vector<const ApkAssets*> 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,14 +310,9 @@ static void NativeSetApkAssets(JNIEnv* env, jclass /*clazz*/, jlong ptr, if (env->ExceptionCheck()) { return; } - if (!apk_assets_native_ptr) { - ALOGE("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.emplace_back(*scoped_assets); + apk_assets.push_back(scoped_assets->get()); } ScopedLock<AssetManager2> assetmanager(AssetManagerFromLong(ptr)); @@ -725,36 +720,31 @@ static jobjectArray NativeGetResourceStringArray(JNIEnv* env, jclass /*clazz*/, } if (attr_value.type == Res_value::TYPE_STRING) { - auto apk_assets_weak = assetmanager->GetApkAssets()[attr_value.cookie]; - if (auto apk_assets = apk_assets_weak.promote()) { - const ResStringPool* pool = apk_assets->GetLoadedArsc()->GetStringPool(); + const ApkAssets* apk_assets = assetmanager->GetApkAssets()[attr_value.cookie]; + const ResStringPool* pool = apk_assets->GetLoadedArsc()->GetStringPool(); - jstring java_string; - if (auto str_utf8 = pool->string8At(attr_value.data); str_utf8.has_value()) { + jstring java_string; + if (auto str_utf8 = pool->string8At(attr_value.data); str_utf8.has_value()) { java_string = env->NewStringUTF(str_utf8->data()); - } else { + } else { auto str_utf16 = pool->stringAt(attr_value.data); if (!str_utf16.has_value()) { - return nullptr; + return nullptr; } - java_string = - env->NewString(reinterpret_cast<const jchar*>(str_utf16->data()), str_utf16->size()); - } + 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; - } + // 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); - } else { - ALOGW("NativeGetResourceStringArray: an expired assets object #%d / %d", i, - attr_value.cookie); - } + // 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); } } return array; diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp index f0c639574a9f..15aaae25f754 100644 --- a/libs/androidfw/ApkAssets.cpp +++ b/libs/androidfw/ApkAssets.cpp @@ -27,34 +27,39 @@ using base::unique_fd; constexpr const char* kResourcesArsc = "resources.arsc"; -ApkAssets::ApkAssets(PrivateConstructorUtil, std::unique_ptr<Asset> resources_asset, +ApkAssets::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, 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)) {} -ApkAssetsPtr ApkAssets::Load(const std::string& path, package_property_t flags) { +std::unique_ptr<ApkAssets> ApkAssets::Load(const std::string& path, package_property_t flags) { return Load(ZipAssetsProvider::Create(path, flags), flags); } -ApkAssetsPtr ApkAssets::LoadFromFd(base::unique_fd fd, const std::string& debug_name, - package_property_t flags, off64_t offset, off64_t len) { +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) { return Load(ZipAssetsProvider::Create(std::move(fd), debug_name, offset, len), flags); } -ApkAssetsPtr ApkAssets::Load(std::unique_ptr<AssetsProvider> assets, package_property_t flags) { +std::unique_ptr<ApkAssets> ApkAssets::Load(std::unique_ptr<AssetsProvider> assets, + package_property_t flags) { return LoadImpl(std::move(assets), flags, nullptr /* idmap_asset */, nullptr /* loaded_idmap */); } -ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t flags) { +std::unique_ptr<ApkAssets> ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset, + std::unique_ptr<AssetsProvider> assets, + package_property_t flags) { if (resources_asset == nullptr) { return {}; } @@ -62,7 +67,8 @@ ApkAssetsPtr ApkAssets::LoadTable(std::unique_ptr<Asset> resources_asset, nullptr /* loaded_idmap */); } -ApkAssetsPtr ApkAssets::LoadOverlay(const std::string& idmap_path, package_property_t flags) { +std::unique_ptr<ApkAssets> 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) { @@ -97,10 +103,10 @@ ApkAssetsPtr ApkAssets::LoadOverlay(const std::string& idmap_path, package_prope std::move(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) { +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) { if (assets == nullptr) { return {}; } @@ -119,11 +125,11 @@ ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<AssetsProvider> assets, std::move(idmap_asset), std::move(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) { +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) { if (assets == nullptr ) { return {}; } @@ -149,9 +155,10 @@ ApkAssetsPtr ApkAssets::LoadImpl(std::unique_ptr<Asset> resources_asset, return {}; } - 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)); + 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))); } std::optional<std::string_view> ApkAssets::GetPath() const { @@ -167,5 +174,4 @@ 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 d604df3565d2..bc46cf5a98ce 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -91,14 +91,13 @@ struct FindEntryResult { StringPoolRef entry_string_ref; }; -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); +AssetManager2::AssetManager2() { + memset(&configuration_, 0, sizeof(configuration_)); } -bool AssetManager2::SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches) { - BuildDynamicRefTable(apk_assets); +bool AssetManager2::SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool invalidate_caches) { + apk_assets_ = std::move(apk_assets); + BuildDynamicRefTable(); RebuildFilterList(); if (invalidate_caches) { InvalidateCaches(static_cast<uint32_t>(-1)); @@ -106,13 +105,7 @@ bool AssetManager2::SetApkAssets(ApkAssetsList apk_assets, bool invalidate_cache return true; } -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()); +void AssetManager2::BuildDynamicRefTable() { package_groups_.clear(); package_ids_.fill(0xff); @@ -123,19 +116,16 @@ void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { // 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; - 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(); }); + 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(); + }); // 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].get()] = 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]] = static_cast<ApkAssetsCookie>(i); } // 0x01 is reserved for the android package. @@ -252,8 +242,7 @@ void AssetManager2::DumpToLog() const { std::string list; for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); - base::StringAppendF(&list, "%s,", assets ? assets->GetDebugName().c_str() : "nullptr"); + base::StringAppendF(&list, "%s,", apk_assets->GetDebugName().c_str()); } LOG(INFO) << "ApkAssets: " << list; @@ -290,8 +279,7 @@ const ResStringPool* AssetManager2::GetStringPoolForCookie(ApkAssetsCookie cooki if (cookie < 0 || static_cast<size_t>(cookie) >= apk_assets_.size()) { return nullptr; } - auto assets = apk_assets_[cookie].promote(); - return assets ? assets->GetLoadedArsc()->GetStringPool() : nullptr; + return apk_assets_[cookie]->GetLoadedArsc()->GetStringPool(); } const DynamicRefTable* AssetManager2::GetDynamicRefTableForPackage(uint32_t package_id) const { @@ -343,11 +331,7 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, std::string* out) const { uint8_t package_id = 0U; for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); - if (!assets) { - continue; - } - const LoadedArsc* loaded_arsc = assets->GetLoadedArsc(); + const LoadedArsc* loaded_arsc = apk_assets->GetLoadedArsc(); if (loaded_arsc == nullptr) { continue; } @@ -400,10 +384,8 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, } bool AssetManager2::ContainsAllocatedTable() const { - 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(); + return std::find_if(apk_assets_.begin(), apk_assets_.end(), + std::mem_fn(&ApkAssets::IsTableAllocated)) != apk_assets_.end(); } void AssetManager2::SetConfiguration(const ResTable_config& configuration) { @@ -416,8 +398,8 @@ void AssetManager2::SetConfiguration(const ResTable_config& configuration) { } } -std::set<AssetManager2::ApkAssetsPtr> AssetManager2::GetNonSystemOverlays() const { - std::set<ApkAssetsPtr> non_system_overlays; +std::set<const ApkAssets*> AssetManager2::GetNonSystemOverlays() const { + std::set<const ApkAssets*> non_system_overlays; for (const PackageGroup& package_group : package_groups_) { bool found_system_package = false; for (const ConfiguredPackage& package : package_group.packages_) { @@ -429,9 +411,7 @@ std::set<AssetManager2::ApkAssetsPtr> AssetManager2::GetNonSystemOverlays() cons if (!found_system_package) { for (const ConfiguredOverlay& overlay : package_group.overlays_) { - if (auto asset = apk_assets_[overlay.cookie].promote()) { - non_system_overlays.insert(std::move(asset)); - } + non_system_overlays.insert(apk_assets_[overlay.cookie]); } } } @@ -443,24 +423,21 @@ 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<ApkAssetsPtr>(); + (exclude_system) ? GetNonSystemOverlays() : std::set<const ApkAssets*>(); 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) { - 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; - } - } + 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; } auto result = package.loaded_package_->CollectConfigurations(exclude_mipmap, &configurations); @@ -477,23 +454,20 @@ 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<ApkAssetsPtr>(); + (exclude_system) ? GetNonSystemOverlays() : std::set<const ApkAssets*>(); 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) { - 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; - } - } + 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; } package.loaded_package_->CollectLocales(merge_equivalent_languages, &locales); @@ -518,12 +492,13 @@ std::unique_ptr<AssetDir> AssetManager2::OpenDir(const std::string& dirname) con ATRACE_NAME("AssetManager::OpenDir"); std::string full_path = "assets/" + dirname; - auto files = util::make_unique<SortedVector<AssetDir::FileInfo>>(); + std::unique_ptr<SortedVector<AssetDir::FileInfo>> files = + util::make_unique<SortedVector<AssetDir::FileInfo>>(); // Start from the back. for (auto iter = apk_assets_.rbegin(); iter != apk_assets_.rend(); ++iter) { - auto apk_assets = iter->promote(); - if (!apk_assets || apk_assets->IsOverlay()) { + const ApkAssets* apk_assets = *iter; + if (apk_assets->IsOverlay()) { continue; } @@ -552,15 +527,14 @@ 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 (!assets || assets->IsOverlay()) { + if (apk_assets_[i]->IsOverlay()) { continue; } - std::unique_ptr<Asset> asset = assets->GetAssetsProvider()->Open(filename, mode); + std::unique_ptr<Asset> asset = apk_assets_[i]->GetAssetsProvider()->Open(filename, mode); if (asset) { if (out_cookie != nullptr) { *out_cookie = i; @@ -581,8 +555,7 @@ std::unique_ptr<Asset> AssetManager2::OpenNonAsset(const std::string& filename, if (cookie < 0 || static_cast<size_t>(cookie) >= apk_assets_.size()) { return {}; } - auto assets = apk_assets_[cookie].promote(); - return assets ? assets->GetAssetsProvider()->Open(filename, mode) : nullptr; + return apk_assets_[cookie]->GetAssetsProvider()->Open(filename, mode); } base::expected<FindEntryResult, NullOrIOError> AssetManager2::FindEntry( @@ -630,12 +603,7 @@ base::expected<FindEntryResult, NullOrIOError> AssetManager2::FindEntry( } bool overlaid = false; - 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 (!stop_at_first_match && !ignore_configuration && !assets->IsLoader()) { + 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) { @@ -665,7 +633,7 @@ base::expected<FindEntryResult, NullOrIOError> AssetManager2::FindEntry( if (UNLIKELY(logging_enabled)) { last_resolution_.steps.push_back( Resolution::Step{Resolution::Step::Type::OVERLAID_INLINE, result->cookie, String8()}); - if (auto path = assets->GetPath()) { + 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. @@ -891,9 +859,7 @@ std::string AssetManager2::GetLastResourceResolution() const { } const uint32_t resid = last_resolution_.resid; - auto assets = apk_assets_[cookie].promote(); - const auto package = - assets ? assets->GetLoadedArsc()->GetPackageById(get_package_id(resid)) : nullptr; + const auto package = apk_assets_[cookie]->GetLoadedArsc()->GetPackageById(get_package_id(resid)); std::string resource_name_string; if (package != nullptr) { @@ -924,9 +890,7 @@ std::string AssetManager2::GetLastResourceResolution() const { continue; } const auto prefix = kStepStrings[int(step.type) - int(Resolution::Step::Type::INITIAL)]; - auto assets = apk_assets_[step.cookie].promote(); - log_stream << "\n\t" << prefix << ": " << (assets ? assets->GetDebugName() : "<null>") - << " #" << step.cookie; + log_stream << "\n\t" << prefix << ": " << apk_assets_[step.cookie]->GetDebugName(); if (!step.config_name.isEmpty()) { log_stream << " - " << step.config_name; } @@ -1591,14 +1555,11 @@ 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++) { - auto src_asset = src_assets[i].promote(); - if (!src_asset) { - continue; - } + const ApkAssets* src_asset = src_assets[i]; const auto& dest_assets = asset_manager_->GetApkAssets(); for (size_t j = 0; j < dest_assets.size(); j++) { - auto dest_asset = dest_assets[j].promote(); + const ApkAssets* dest_asset = dest_assets[j]; 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/include/androidfw/ApkAssets.h b/libs/androidfw/include/androidfw/ApkAssets.h index 1fa67528c78b..6f88f41976cd 100644 --- a/libs/androidfw/include/androidfw/ApkAssets.h +++ b/libs/androidfw/include/androidfw/ApkAssets.h @@ -17,13 +17,12 @@ #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" @@ -32,33 +31,34 @@ namespace android { -class ApkAssets; - -using ApkAssetsPtr = sp<ApkAssets>; - // Holds an APK. -class ApkAssets : public RefBase { +class ApkAssets { public: // Creates an ApkAssets from a path on device. - static ApkAssetsPtr Load(const std::string& path, package_property_t flags = 0U); + static std::unique_ptr<ApkAssets> Load(const std::string& path, + package_property_t flags = 0U); // Creates an ApkAssets from an open file descriptor. - 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); + 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); // Creates an ApkAssets from an AssetProvider. // The ApkAssets will take care of destroying the AssetsProvider when it is destroyed. - static ApkAssetsPtr Load(std::unique_ptr<AssetsProvider> assets, package_property_t flags = 0U); + static std::unique_ptr<ApkAssets> Load(std::unique_ptr<AssetsProvider> assets, + package_property_t flags = 0U); // Creates an ApkAssets from the given asset file representing a resources.arsc. - static ApkAssetsPtr LoadTable(std::unique_ptr<Asset> resources_asset, - std::unique_ptr<AssetsProvider> assets, - package_property_t flags = 0U); + static std::unique_ptr<ApkAssets> 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 ApkAssetsPtr LoadOverlay(const std::string& idmap_path, package_property_t flags = 0U); + static std::unique_ptr<ApkAssets> 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,27 +95,22 @@ class ApkAssets : public RefBase { bool IsUpToDate() const; private: - 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, + 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, 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 1f979956873f..c1162404a3bd 100644 --- a/libs/androidfw/include/androidfw/AssetManager2.h +++ b/libs/androidfw/include/androidfw/AssetManager2.h @@ -17,16 +17,14 @@ #ifndef ANDROIDFW_ASSETMANAGER2_H_ #define ANDROIDFW_ASSETMANAGER2_H_ -#include <utils/RefBase.h> +#include "android-base/function_ref.h" +#include "android-base/macros.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" @@ -96,25 +94,18 @@ class AssetManager2 { size_t entry_len = 0u; }; - using ApkAssetsPtr = sp<const ApkAssets>; - using ApkAssetsWPtr = wp<const ApkAssets>; - using ApkAssetsList = std::span<const ApkAssetsPtr>; - - AssetManager2() = default; + AssetManager2(); 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(ApkAssetsList apk_assets, bool invalidate_caches = true); - bool SetApkAssets(std::initializer_list<ApkAssetsPtr> apk_assets, bool invalidate_caches = true); + bool SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool invalidate_caches = true); - inline const std::vector<ApkAssetsWPtr>& GetApkAssets() const { + inline const std::vector<const ApkAssets*>& GetApkAssets() const { return apk_assets_; } @@ -408,7 +399,7 @@ class AssetManager2 { // Assigns package IDs to all shared library ApkAssets. // Should be called whenever the ApkAssets are changed. - void BuildDynamicRefTable(ApkAssetsList assets); + void BuildDynamicRefTable(); // Purge all resources that are cached and vary by the configuration axis denoted by the // bitmask `diff`. @@ -419,7 +410,7 @@ class AssetManager2 { void RebuildFilterList(); // Retrieves the APK paths of overlays that overlay non-system packages. - std::set<ApkAssetsPtr> GetNonSystemOverlays() const; + std::set<const ApkAssets*> GetNonSystemOverlays() const; // AssetManager2::GetBag(resid) wraps this function to track which resource ids have already // been seen while traversing bag parents. @@ -428,7 +419,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<ApkAssetsWPtr> apk_assets_; + std::vector<const ApkAssets*> apk_assets_; // DynamicRefTables for shared library package resolution. // These are ordered according to apk_assets_. The mappings may change depending on what is @@ -442,7 +433,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 b6093dbb6d3d..6fc6d64e2f6e 100644 --- a/libs/androidfw/include/androidfw/MutexGuard.h +++ b/libs/androidfw/include/androidfw/MutexGuard.h @@ -14,12 +14,12 @@ * limitations under the License. */ -#pragma once +#ifndef ANDROIDFW_MUTEXGUARD_H +#define ANDROIDFW_MUTEXGUARD_H #include <mutex> #include <optional> #include <type_traits> -#include <utility> #include "android-base/macros.h" @@ -45,25 +45,20 @@ class ScopedLock; // template <typename T> class Guarded { - static_assert(!std::is_pointer_v<T>, "T must not be a raw pointer"); + static_assert(!std::is_pointer<T>::value, "T must not be a raw pointer"); public: - Guarded() : guarded_(std::in_place) { + Guarded() : guarded_(std::in_place, T()) { } explicit Guarded(const T& guarded) : guarded_(std::in_place, guarded) { } - explicit Guarded(T&& guarded) : guarded_(std::in_place, std::move(guarded)) { + explicit Guarded(T&& guarded) : guarded_(std::in_place, std::forward<T>(guarded)) { } - // 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() { + std::lock_guard<std::mutex> scoped_lock(lock_); guarded_.reset(); } @@ -101,3 +96,5 @@ 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 70326b71da28..19db25ce8811 100644 --- a/libs/androidfw/tests/ApkAssets_test.cpp +++ b/libs/androidfw/tests/ApkAssets_test.cpp @@ -35,7 +35,8 @@ using ::testing::StrEq; namespace android { TEST(ApkAssetsTest, LoadApk) { - auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + std::unique_ptr<const ApkAssets> loaded_apk = + ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -49,7 +50,7 @@ TEST(ApkAssetsTest, LoadApkFromFd) { unique_fd fd(::open(path.c_str(), O_RDONLY | O_BINARY)); ASSERT_THAT(fd.get(), Ge(0)); - auto loaded_apk = ApkAssets::LoadFromFd(std::move(fd), path); + std::unique_ptr<const ApkAssets> loaded_apk = ApkAssets::LoadFromFd(std::move(fd), path); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -59,7 +60,8 @@ TEST(ApkAssetsTest, LoadApkFromFd) { } TEST(ApkAssetsTest, LoadApkAsSharedLibrary) { - auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/appaslib/appaslib.apk"); + std::unique_ptr<const ApkAssets> loaded_apk = + ApkAssets::Load(GetTestDataPath() + "/appaslib/appaslib.apk"); ASSERT_THAT(loaded_apk, NotNull()); const LoadedArsc* loaded_arsc = loaded_apk->GetLoadedArsc(); @@ -77,7 +79,8 @@ TEST(ApkAssetsTest, LoadApkAsSharedLibrary) { } TEST(ApkAssetsTest, CreateAndDestroyAssetKeepsApkAssetsOpen) { - auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + std::unique_ptr<const ApkAssets> loaded_apk = + ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); ASSERT_THAT(loaded_apk, NotNull()); { ASSERT_THAT(loaded_apk->GetAssetsProvider()->Open("res/layout/main.xml", @@ -88,7 +91,8 @@ TEST(ApkAssetsTest, CreateAndDestroyAssetKeepsApkAssetsOpen) { } TEST(ApkAssetsTest, OpenUncompressedAssetFd) { - auto loaded_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + std::unique_ptr<const ApkAssets> 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 6fae72a6d10e..c7ae618991b9 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()) { - auto apk = ApkAssets::Load(path); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path); AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); } } BENCHMARK(BM_AssetManagerLoadAssets); @@ -61,9 +61,9 @@ BENCHMARK(BM_AssetManagerLoadAssetsOld); static void BM_AssetManagerLoadFrameworkAssets(benchmark::State& state) { std::string path = kFrameworkPath; while (state.KeepRunning()) { - auto apk = ApkAssets::Load(path); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path); AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); } } BENCHMARK(BM_AssetManagerLoadFrameworkAssets); @@ -129,14 +129,14 @@ static void BM_AssetManagerGetResourceFrameworkLocaleOld(benchmark::State& state BENCHMARK(BM_AssetManagerGetResourceFrameworkLocaleOld); static void BM_AssetManagerGetBag(benchmark::State& state) { - auto apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); 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) { - auto apk = ApkAssets::Load(kFrameworkPath); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); 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) { - auto apk = ApkAssets::Load(kFrameworkPath); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); 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 5a5bafdf829a..4394740e44ba 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: - 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_; + 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_; }; TEST_F(AssetManager2Test, FindsResourceFromSingleApkAssets) { @@ -114,7 +114,7 @@ TEST_F(AssetManager2Test, FindsResourceFromSingleApkAssets) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); 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_, basic_de_fr_assets_}); + assetmanager.SetApkAssets({basic_assets_.get(), basic_de_fr_assets_.get()}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -159,7 +159,8 @@ 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_, lib_one_assets_, libclient_assets_}); + assetmanager.SetApkAssets( + {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); auto value = assetmanager.GetResource(libclient::R::string::foo_one); ASSERT_TRUE(value.has_value()); @@ -194,7 +195,7 @@ TEST_F(AssetManager2Test, FindsResourceFromSharedLibrary) { TEST_F(AssetManager2Test, FindsResourceFromAppLoadedAsSharedLibrary) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({appaslib_assets_}); + assetmanager.SetApkAssets({appaslib_assets_.get()}); // The appaslib package will have been assigned the package ID 0x02. auto value = assetmanager.GetResource(fix_package_id(appaslib::R::integer::number1, 0x02)); @@ -205,26 +206,27 @@ TEST_F(AssetManager2Test, FindsResourceFromAppLoadedAsSharedLibrary) { TEST_F(AssetManager2Test, AssignsOverlayPackageIdLast) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); + assetmanager.SetApkAssets( + {overlayable_assets_.get(), overlay_assets_.get(), lib_one_assets_.get()}); auto apk_assets = assetmanager.GetApkAssets(); ASSERT_EQ(3, apk_assets.size()); - 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()); + 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]); - auto get_first_package_id = [&assetmanager](auto apkAssets) -> uint8_t { + auto get_first_package_id = [&assetmanager](const ApkAssets* apkAssets) -> uint8_t { return assetmanager.GetAssignedPackageId(apkAssets->GetLoadedArsc()->GetPackages()[0].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_)); + 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())); } TEST_F(AssetManager2Test, GetSharedLibraryResourceName) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({lib_one_assets_}); + assetmanager.SetApkAssets({lib_one_assets_.get()}); auto name = assetmanager.GetResourceName(lib_one::R::string::foo); ASSERT_TRUE(name.has_value()); @@ -233,7 +235,7 @@ TEST_F(AssetManager2Test, GetSharedLibraryResourceName) { TEST_F(AssetManager2Test, GetResourceNameNonMatchingConfig) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_de_fr_assets_}); + assetmanager.SetApkAssets({basic_de_fr_assets_.get()}); auto value = assetmanager.GetResourceName(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -242,7 +244,7 @@ TEST_F(AssetManager2Test, GetResourceNameNonMatchingConfig) { TEST_F(AssetManager2Test, GetResourceTypeSpecFlags) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_de_fr_assets_}); + assetmanager.SetApkAssets({basic_de_fr_assets_.get()}); auto value = assetmanager.GetResourceTypeSpecFlags(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -251,7 +253,7 @@ TEST_F(AssetManager2Test, GetResourceTypeSpecFlags) { TEST_F(AssetManager2Test, FindsBagResourceFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto bag = assetmanager.GetBag(basic::R::array::integerArray1); ASSERT_TRUE(bag.has_value()); @@ -278,7 +280,8 @@ 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_, lib_one_assets_, libclient_assets_}); + assetmanager.SetApkAssets( + {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); auto bag = assetmanager.GetBag(fix_package_id(lib_one::R::style::Theme, 0x03)); ASSERT_TRUE(bag.has_value()); @@ -297,7 +300,8 @@ 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_, lib_one_assets_, libclient_assets_}); + assetmanager.SetApkAssets( + {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); auto bag = assetmanager.GetBag(libclient::R::style::ThemeMultiLib); ASSERT_TRUE(bag.has_value()); @@ -317,7 +321,8 @@ 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_, lib_one_assets_, libclient_assets_}); + assetmanager.SetApkAssets( + {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); auto bag = assetmanager.GetBag(libclient::R::style::Theme); ASSERT_TRUE(bag.has_value()); @@ -332,7 +337,7 @@ TEST_F(AssetManager2Test, FindsStyleResourceWithParentFromSharedLibrary) { TEST_F(AssetManager2Test, MergesStylesWithParentFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_}); + assetmanager.SetApkAssets({style_assets_.get()}); auto bag_one = assetmanager.GetBag(app::R::style::StyleOne); ASSERT_TRUE(bag_one.has_value()); @@ -396,7 +401,7 @@ TEST_F(AssetManager2Test, MergesStylesWithParentFromSingleApkAssets) { TEST_F(AssetManager2Test, MergeStylesCircularDependency) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_}); + assetmanager.SetApkAssets({style_assets_.get()}); // GetBag should stop traversing the parents of styles when a circular // dependency is detected @@ -407,7 +412,7 @@ TEST_F(AssetManager2Test, MergeStylesCircularDependency) { TEST_F(AssetManager2Test, ResolveReferenceToResource) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto value = assetmanager.GetResource(basic::R::integer::ref1); ASSERT_TRUE(value.has_value()); @@ -423,7 +428,7 @@ TEST_F(AssetManager2Test, ResolveReferenceToResource) { TEST_F(AssetManager2Test, ResolveReferenceToBag) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto value = assetmanager.GetResource(basic::R::integer::number2, true /*may_be_bag*/); ASSERT_TRUE(value.has_value()); @@ -439,7 +444,7 @@ TEST_F(AssetManager2Test, ResolveReferenceToBag) { TEST_F(AssetManager2Test, ResolveDeepIdReference) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); // Set up the resource ids auto high_ref = assetmanager.GetResourceId("@id/high_ref", "values", "com.android.basic"); @@ -465,7 +470,8 @@ TEST_F(AssetManager2Test, ResolveDeepIdReference) { TEST_F(AssetManager2Test, DensityOverride) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_, basic_xhdpi_assets_, basic_xxhdpi_assets_}); + assetmanager.SetApkAssets({basic_assets_.get(), basic_xhdpi_assets_.get(), + basic_xxhdpi_assets_.get()}); assetmanager.SetConfiguration({ .density = ResTable_config::DENSITY_XHIGH, .sdkVersion = 21, @@ -487,7 +493,7 @@ TEST_F(AssetManager2Test, DensityOverride) { TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); // Create some kind of value that is NOT a reference. AssetManager2::SelectedValue value{}; @@ -503,7 +509,7 @@ TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved) TEST_F(AssetManager2Test, ResolveReferenceMissingResourceDoNotCacheFlags) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); { AssetManager2::SelectedValue value{}; value.data = basic::R::string::test1; @@ -534,7 +540,7 @@ TEST_F(AssetManager2Test, ResolveReferenceMissingResourceDoNotCacheFlags) { TEST_F(AssetManager2Test, ResolveReferenceMissingResource) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); const uint32_t kMissingResId = 0x8001ffff; AssetManager2::SelectedValue value{}; @@ -552,7 +558,7 @@ TEST_F(AssetManager2Test, ResolveReferenceMissingResource) { TEST_F(AssetManager2Test, ResolveReferenceMissingResourceLib) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({libclient_assets_}); + assetmanager.SetApkAssets({libclient_assets_.get()}); AssetManager2::SelectedValue value{}; value.type = Res_value::TYPE_REFERENCE; @@ -574,7 +580,7 @@ static bool IsConfigurationPresent(const std::set<ResTable_config>& configuratio TEST_F(AssetManager2Test, GetResourceConfigurations) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_, basic_de_fr_assets_}); + assetmanager.SetApkAssets({system_assets_.get(), basic_de_fr_assets_.get()}); auto configurations = assetmanager.GetResourceConfigurations(); ASSERT_TRUE(configurations.has_value()); @@ -619,7 +625,7 @@ TEST_F(AssetManager2Test, GetResourceConfigurations) { TEST_F(AssetManager2Test, GetResourceLocales) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_, basic_de_fr_assets_}); + assetmanager.SetApkAssets({system_assets_.get(), basic_de_fr_assets_.get()}); std::set<std::string> locales = assetmanager.GetResourceLocales(); @@ -638,7 +644,7 @@ TEST_F(AssetManager2Test, GetResourceLocales) { TEST_F(AssetManager2Test, GetResourceId) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto resid = assetmanager.GetResourceId("com.android.basic:layout/main", "", ""); ASSERT_TRUE(resid.has_value()); @@ -655,7 +661,7 @@ TEST_F(AssetManager2Test, GetResourceId) { TEST_F(AssetManager2Test, OpensFileFromSingleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_}); + assetmanager.SetApkAssets({system_assets_.get()}); std::unique_ptr<Asset> asset = assetmanager.Open("file.txt", Asset::ACCESS_BUFFER); ASSERT_THAT(asset, NotNull()); @@ -667,7 +673,7 @@ TEST_F(AssetManager2Test, OpensFileFromSingleApkAssets) { TEST_F(AssetManager2Test, OpensFileFromMultipleApkAssets) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_, app_assets_}); + assetmanager.SetApkAssets({system_assets_.get(), app_assets_.get()}); std::unique_ptr<Asset> asset = assetmanager.Open("file.txt", Asset::ACCESS_BUFFER); ASSERT_THAT(asset, NotNull()); @@ -679,7 +685,7 @@ TEST_F(AssetManager2Test, OpensFileFromMultipleApkAssets) { TEST_F(AssetManager2Test, OpenDir) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_}); + assetmanager.SetApkAssets({system_assets_.get()}); std::unique_ptr<AssetDir> asset_dir = assetmanager.OpenDir(""); ASSERT_THAT(asset_dir, NotNull()); @@ -701,7 +707,7 @@ TEST_F(AssetManager2Test, OpenDir) { TEST_F(AssetManager2Test, OpenDirFromManyApks) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({system_assets_, app_assets_}); + assetmanager.SetApkAssets({system_assets_.get(), app_assets_.get()}); std::unique_ptr<AssetDir> asset_dir = assetmanager.OpenDir(""); ASSERT_THAT(asset_dir, NotNull()); @@ -722,7 +728,7 @@ TEST_F(AssetManager2Test, GetLastPathWithoutEnablingReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); assetmanager.SetResourceResolutionLoggingEnabled(false); auto value = assetmanager.GetResource(basic::R::string::test1); @@ -737,7 +743,7 @@ TEST_F(AssetManager2Test, GetLastPathWithoutResolutionReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto result = assetmanager.GetLastResourceResolution(); EXPECT_EQ("", result); @@ -752,18 +758,17 @@ TEST_F(AssetManager2Test, GetLastPathWithSingleApkAssets) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); 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 #0\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\n" + "Best matching is from default configuration of com.android.basic", + result); } TEST_F(AssetManager2Test, GetLastPathWithMultipleApkAssets) { @@ -775,19 +780,18 @@ TEST_F(AssetManager2Test, GetLastPathWithMultipleApkAssets) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_, basic_de_fr_assets_}); + assetmanager.SetApkAssets({basic_assets_.get(), basic_de_fr_assets_.get()}); 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 #0\n" - "\tFound better: basic/basic_de_fr.apk #1 - 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\n" + "\tFound better: basic/basic_de_fr.apk - de\n" + "Best matching is from de configuration of com.android.basic", + result); } TEST_F(AssetManager2Test, GetLastPathAfterDisablingReturnsEmpty) { @@ -797,7 +801,7 @@ TEST_F(AssetManager2Test, GetLastPathAfterDisablingReturnsEmpty) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({basic_assets_}); + assetmanager.SetApkAssets({basic_assets_.get()}); auto value = assetmanager.GetResource(basic::R::string::test1); ASSERT_TRUE(value.has_value()); @@ -818,7 +822,7 @@ TEST_F(AssetManager2Test, GetOverlayablesToString) { AssetManager2 assetmanager; assetmanager.SetResourceResolutionLoggingEnabled(true); assetmanager.SetConfiguration(desired_config); - assetmanager.SetApkAssets({overlayable_assets_}); + assetmanager.SetApkAssets({overlayable_assets_.get()}); 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 384f4a78b36d..1c89c61c8f78 100644 --- a/libs/androidfw/tests/AttributeResolution_bench.cpp +++ b/libs/androidfw/tests/AttributeResolution_bench.cpp @@ -36,14 +36,15 @@ 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) { - auto styles_apk = ApkAssets::Load(GetTestDataPath() + "/styles/styles.apk"); + std::unique_ptr<const ApkAssets> 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}); + assetmanager.SetApkAssets({styles_apk.get()}); std::unique_ptr<Asset> asset = assetmanager.OpenNonAsset("res/layout/layout.xml", Asset::ACCESS_BUFFER); @@ -79,20 +80,21 @@ static void BM_ApplyStyle(benchmark::State& state) { BENCHMARK(BM_ApplyStyle); static void BM_ApplyStyleFramework(benchmark::State& state) { - auto framework_apk = ApkAssets::Load(kFrameworkPath); + std::unique_ptr<const ApkAssets> framework_apk = ApkAssets::Load(kFrameworkPath); if (framework_apk == nullptr) { state.SkipWithError("failed to load framework assets"); return; } - auto basic_apk = ApkAssets::Load(GetTestDataPath() + "/basic/basic.apk"); + std::unique_ptr<const ApkAssets> 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, basic_apk}); + assetmanager.SetApkAssets({framework_apk.get(), basic_apk.get()}); 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 329830fa47b2..bb9129ad01c8 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_}); + assetmanager_.SetApkAssets({styles_assets_.get()}); } protected: - AssetManager2::ApkAssetsPtr styles_assets_; + std::unique_ptr<const ApkAssets> 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}); + assetmanager.SetApkAssets({apk_assets.get()}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); diff --git a/libs/androidfw/tests/BenchmarkHelpers.cpp b/libs/androidfw/tests/BenchmarkHelpers.cpp index b97dd96f8934..0fa0573bcbb8 100644 --- a/libs/androidfw/tests/BenchmarkHelpers.cpp +++ b/libs/androidfw/tests/BenchmarkHelpers.cpp @@ -53,18 +53,20 @@ 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<AssetManager2::ApkAssetsPtr> apk_assets; + std::vector<std::unique_ptr<const ApkAssets>> apk_assets; + std::vector<const ApkAssets*> apk_assets_ptrs; for (const std::string& path : paths) { - auto apk = ApkAssets::Load(path); + std::unique_ptr<const ApkAssets> 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); + assetmanager.SetApkAssets(apk_assets_ptrs); if (config != nullptr) { assetmanager.SetConfiguration(*config); } diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp index 568e041ebe5b..b43491548e2b 100644 --- a/libs/androidfw/tests/Idmap_test.cpp +++ b/libs/androidfw/tests/Idmap_test.cpp @@ -59,16 +59,15 @@ class IdmapTest : public ::testing::Test { protected: std::string original_path; - AssetManager2::ApkAssetsPtr system_assets_; - AssetManager2::ApkAssetsPtr overlay_assets_; - AssetManager2::ApkAssetsPtr overlayable_assets_; + std::unique_ptr<const ApkAssets> system_assets_; + std::unique_ptr<const ApkAssets> overlay_assets_; + std::unique_ptr<const ApkAssets> 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].promote()->GetLoadedArsc()->GetStringPool(); + const ResStringPool* string_pool = assets[value.cookie]->GetLoadedArsc()->GetStringPool(); return GetStringFromPool(string_pool, value.data); } @@ -76,7 +75,8 @@ std::string GetStringFromApkAssets(const AssetManager2& asset_manager, TEST_F(IdmapTest, OverlayOverridesResourceValue) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable5); ASSERT_TRUE(value.has_value()); @@ -87,7 +87,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValue) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingDifferentPackage) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable10); ASSERT_TRUE(value.has_value()); @@ -98,7 +99,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingDifferentPackage) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInternalResource) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable8); ASSERT_TRUE(value.has_value()); @@ -109,7 +111,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInternalResource) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineInteger) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::integer::config_integer); ASSERT_TRUE(value.has_value()); @@ -120,7 +123,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineInteger) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineString) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable11); ASSERT_TRUE(value.has_value()); @@ -131,7 +135,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingInlineString) { TEST_F(IdmapTest, OverlayOverridesResourceValueUsingOverlayingResource) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::string::overlayable9); ASSERT_TRUE(value.has_value()); @@ -142,7 +147,8 @@ TEST_F(IdmapTest, OverlayOverridesResourceValueUsingOverlayingResource) { TEST_F(IdmapTest, OverlayOverridesXmlParser) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto value = asset_manager.GetResource(overlayable::R::layout::hello_view); ASSERT_TRUE(value.has_value()); @@ -180,7 +186,8 @@ TEST_F(IdmapTest, OverlayOverridesXmlParser) { TEST_F(IdmapTest, OverlaidResourceHasSameName) { AssetManager2 asset_manager; - asset_manager.SetApkAssets({system_assets_, overlayable_assets_, overlay_assets_}); + asset_manager.SetApkAssets({system_assets_.get(), overlayable_assets_.get(), + overlay_assets_.get()}); auto name = asset_manager.GetResourceName(overlayable::R::string::overlayable9); ASSERT_TRUE(name.has_value()); @@ -196,7 +203,8 @@ TEST_F(IdmapTest, OverlayLoaderInterop) { auto loader_assets = ApkAssets::LoadTable(std::move(asset), EmptyAssetsProvider::Create(), PROPERTY_LOADER); AssetManager2 asset_manager; - asset_manager.SetApkAssets({overlayable_assets_, loader_assets, overlay_assets_}); + asset_manager.SetApkAssets({overlayable_assets_.get(), loader_assets.get(), + overlay_assets_.get()}); 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 dfbb5a76dec6..f3d60bbe4f15 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) { - auto apk = ApkAssets::Load(kFrameworkPath); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); if (apk == nullptr) { state.SkipWithError("Failed to load assets"); return; } AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); 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) { - auto apk = ApkAssets::Load(kFrameworkPath); + std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(kFrameworkPath); AssetManager2 assets; - assets.SetApkAssets({apk}); + assets.SetApkAssets({apk.get()}); 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 e08a6a7f277d..77114f273d3d 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: - AssetManager2::ApkAssetsPtr system_assets_; - AssetManager2::ApkAssetsPtr style_assets_; - AssetManager2::ApkAssetsPtr libclient_assets_; - AssetManager2::ApkAssetsPtr lib_one_assets_; - AssetManager2::ApkAssetsPtr lib_two_assets_; + 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_; }; TEST_F(ThemeTest, EmptyTheme) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_}); + assetmanager.SetApkAssets({style_assets_.get()}); 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_}); + assetmanager.SetApkAssets({style_assets_.get()}); 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_}); + assetmanager.SetApkAssets({style_assets_.get()}); 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_}); + assetmanager.SetApkAssets({style_assets_.get()}); 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_}); + assetmanager.SetApkAssets({style_assets_.get()}); 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_}); + assetmanager.SetApkAssets({style_assets_.get()}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo).has_value()); @@ -190,7 +190,8 @@ TEST_F(ThemeTest, MultipleThemesOverlaidForced) { TEST_F(ThemeTest, ResolveDynamicAttributesAndReferencesToSharedLibrary) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({lib_two_assets_, lib_one_assets_, libclient_assets_}); + assetmanager.SetApkAssets( + {lib_two_assets_.get(), lib_one_assets_.get(), libclient_assets_.get()}); std::unique_ptr<Theme> theme = assetmanager.NewTheme(); ASSERT_TRUE(theme->ApplyStyle(libclient::R::style::Theme, false /*force*/).has_value()); @@ -215,7 +216,7 @@ TEST_F(ThemeTest, ResolveDynamicAttributesAndReferencesToSharedLibrary) { TEST_F(ThemeTest, CopyThemeSameAssetManager) { AssetManager2 assetmanager; - assetmanager.SetApkAssets({style_assets_}); + assetmanager.SetApkAssets({style_assets_.get()}); std::unique_ptr<Theme> theme_one = assetmanager.NewTheme(); ASSERT_TRUE(theme_one->ApplyStyle(app::R::style::StyleOne).has_value()); @@ -252,10 +253,10 @@ TEST_F(ThemeTest, CopyThemeSameAssetManager) { TEST_F(ThemeTest, ThemeRebase) { AssetManager2 am; - am.SetApkAssets({style_assets_}); + am.SetApkAssets({style_assets_.get()}); AssetManager2 am_night; - am_night.SetApkAssets({style_assets_}); + am_night.SetApkAssets({style_assets_.get()}); ResTable_config night{}; night.uiMode = ResTable_config::UI_MODE_NIGHT_YES; @@ -326,11 +327,12 @@ TEST_F(ThemeTest, ThemeRebase) { TEST_F(ThemeTest, OnlyCopySameAssetsThemeWhenAssetManagersDiffer) { AssetManager2 assetmanager_dst; - assetmanager_dst.SetApkAssets( - {system_assets_, lib_one_assets_, style_assets_, libclient_assets_}); + assetmanager_dst.SetApkAssets({system_assets_.get(), lib_one_assets_.get(), style_assets_.get(), + libclient_assets_.get()}); AssetManager2 assetmanager_src; - assetmanager_src.SetApkAssets({system_assets_, lib_two_assets_, lib_one_assets_, style_assets_}); + assetmanager_src.SetApkAssets({system_assets_.get(), lib_two_assets_.get(), lib_one_assets_.get(), + style_assets_.get()}); auto theme_dst = assetmanager_dst.NewTheme(); ASSERT_TRUE(theme_dst->ApplyStyle(app::R::style::StyleOne).has_value()); @@ -374,10 +376,10 @@ TEST_F(ThemeTest, OnlyCopySameAssetsThemeWhenAssetManagersDiffer) { TEST_F(ThemeTest, CopyNonReferencesWhenPackagesDiffer) { AssetManager2 assetmanager_dst; - assetmanager_dst.SetApkAssets({system_assets_}); + assetmanager_dst.SetApkAssets({system_assets_.get()}); AssetManager2 assetmanager_src; - assetmanager_src.SetApkAssets({system_assets_, style_assets_}); + assetmanager_src.SetApkAssets({system_assets_.get(), style_assets_.get()}); 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 f732c216c250..8a6897c055c5 100644 --- a/rs/jni/Android.bp +++ b/rs/jni/Android.bp @@ -22,7 +22,6 @@ 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 e17209088750..90239212f665 100644 --- a/startop/view_compiler/Android.bp +++ b/startop/view_compiler/Android.bp @@ -40,7 +40,7 @@ cc_defaults { "libziparchive", "libz", ], - cpp_std: "gnu++2b", + cppflags: ["-std=c++17"], 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 5f5652c2acac..1d3b648175cc 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 android::ApkAssetsPtr& assets, CompilationTarget target, - std::ostream& target_out) { +void CompileApkAssetsLayouts(const std::unique_ptr<android::ApkAssets>& assets, + CompilationTarget target, std::ostream& target_out) { android::AssetManager2 resources; - resources.SetApkAssets({assets}); + resources.SetApkAssets({assets.get()}); std::string package_name; diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp index 7096f5cc54e3..e629bafbdd25 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})); + ASSERT_TRUE(am.SetApkAssets({android_asset.get()})); 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, app_against_non_final})); + ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_non_final.get()})); 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, app_against_final})); + ASSERT_TRUE(am.SetApkAssets({android_asset.get(), app_against_final.get()})); { auto style = am.GetBag(0x7f020000); diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index b75458a7b8b6..bca62da447b0 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -220,9 +220,15 @@ std::unique_ptr<SymbolTable::Symbol> ResourceTableSymbolSource::FindByName( bool AssetManagerSymbolSource::AddAssetPath(StringPiece path) { TRACE_CALL(); - if (auto apk = ApkAssets::Load(path.data())) { + if (std::unique_ptr<const ApkAssets> apk = ApkAssets::Load(path.data())) { apk_assets_.push_back(std::move(apk)); - asset_manager_.SetApkAssets(apk_assets_); + + 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); return true; } return false; @@ -245,7 +251,7 @@ bool AssetManagerSymbolSource::IsPackageDynamic(uint32_t packageId, return true; } - for (auto&& assets : apk_assets_) { + for (const std::unique_ptr<const ApkAssets>& 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 36eb0bab6046..b09ff702ca58 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); }; -- GitLab