Use reference counted pointers for ApkAssets
The primary reason for memory corruption is freed ApkAssets
Java expected them to only be freed in the finalizers, but
there are explicit close() calls now, destroying objects that
are still in use in some AssetManager2 objects
This CL makes sure those AssetManagers don't assume ApkAssets
always exist, but instead tries to lock them in memory for any
access
It also adds logging in case of deleting an assets object with
any weak pointers still existing. Those will get into the
bugreports attached to related bugs to help with investigation.
Benchmarks don't regress, and the device appears to be working.
Given that the crashes used to be pretty rare, let's wait for
any new reports or lack of those.
+ add a missing .clang-format file to the jni directory
+ enabled C++23 in the project that uses AssetManager headers
Bug: 197260547
Bug: 276922628
Test: unit tests + boot + benchmarks
Change-Id: I495fd9e012fe370a1f725dbb0265b4ee1be8d805
diff --git a/libs/androidfw/tests/ApkAssets_test.cpp b/libs/androidfw/tests/ApkAssets_test.cpp
index 19db25c..70326b7 100644
--- a/libs/androidfw/tests/ApkAssets_test.cpp
+++ b/libs/androidfw/tests/ApkAssets_test.cpp
@@ -35,8 +35,7 @@
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 @@
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, 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, 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, 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 c7ae618..6fae72a 100644
--- a/libs/androidfw/tests/AssetManager2_bench.cpp
+++ b/libs/androidfw/tests/AssetManager2_bench.cpp
@@ -38,9 +38,9 @@
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 @@
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 @@
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 @@
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 @@
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 4394740..5a5bafdf 100644
--- a/libs/androidfw/tests/AssetManager2_test.cpp
+++ b/libs/androidfw/tests/AssetManager2_test.cpp
@@ -91,19 +91,19 @@
}
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 @@
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 @@
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 @@
// 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, 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, 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, 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, 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, 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 @@
// 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 @@
// 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 @@
// 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, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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 @@
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, 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, 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, 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, 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, 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, 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 1c89c61..384f4a7 100644
--- a/libs/androidfw/tests/AttributeResolution_bench.cpp
+++ b/libs/androidfw/tests/AttributeResolution_bench.cpp
@@ -36,15 +36,14 @@
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 @@
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 bb9129a..329830f 100644
--- a/libs/androidfw/tests/AttributeResolution_test.cpp
+++ b/libs/androidfw/tests/AttributeResolution_test.cpp
@@ -36,11 +36,11 @@
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 @@
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 0fa0573..b97dd96 100644
--- a/libs/androidfw/tests/BenchmarkHelpers.cpp
+++ b/libs/androidfw/tests/BenchmarkHelpers.cpp
@@ -53,20 +53,18 @@
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 b434915..568e041 100644
--- a/libs/androidfw/tests/Idmap_test.cpp
+++ b/libs/androidfw/tests/Idmap_test.cpp
@@ -59,15 +59,16 @@
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 @@
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, 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, 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, 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, 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, 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, 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, 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 @@
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 f3d60bb..dfbb5a7 100644
--- a/libs/androidfw/tests/Theme_bench.cpp
+++ b/libs/androidfw/tests/Theme_bench.cpp
@@ -28,14 +28,14 @@
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 @@
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 77114f2..e08a6a7 100644
--- a/libs/androidfw/tests/Theme_test.cpp
+++ b/libs/androidfw/tests/Theme_test.cpp
@@ -53,16 +53,16 @@
}
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, 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, 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, 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, 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, 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, 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, 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, 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, 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, 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();