From 93bca97e7ce1f3a7938d6ce5af5f6f325ab66f89 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Fri, 8 Mar 2019 17:26:28 -0800 Subject: [PATCH] Allow non-references to be copied between AssetManagers Hard-coded values in styles can be copied between AssetManagers even if the source package is not present in the destination AssetManager. Only references and strings should be prevented from being copied over because they would be invalid in the destination AssetManager. Bug:126400561 Test: manual Change-Id: I970a3e961763b2c003c15b950d864a9a0b615022 --- libs/androidfw/AssetManager2.cpp | 68 +++++++++------- libs/androidfw/tests/Theme_test.cpp | 73 +++++++++++++----- libs/androidfw/tests/data/styles/R.h | 1 + libs/androidfw/tests/data/styles/build | 4 +- .../tests/data/styles/res/values/styles.xml | 10 +++ libs/androidfw/tests/data/styles/styles.apk | Bin 2550 -> 2774 bytes 6 files changed, 105 insertions(+), 51 deletions(-) diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 951970464bfe4..1b515ad41e682 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -1319,7 +1319,7 @@ void Theme::SetTo(const Theme& o) { typedef std::map SourceToDestinationRuntimePackageMap; std::map src_asset_cookie_id_map; - // Determine which ApkAssets are loaded in both theme AssetManagers + // Determine which ApkAssets are loaded in both theme AssetManagers. std::vector src_assets = o.asset_manager_->GetApkAssets(); for (size_t i = 0; i < src_assets.size(); i++) { const ApkAssets* src_asset = src_assets[i]; @@ -1328,7 +1328,7 @@ void Theme::SetTo(const Theme& o) { for (size_t j = 0; j < dest_assets.size(); j++) { const ApkAssets* dest_asset = dest_assets[j]; - // Map the runtime package of the source apk asset to the destination apk asset + // Map the runtime package of the source apk asset to the destination apk asset. if (src_asset->GetPath() == dest_asset->GetPath()) { const std::vector>& src_packages = src_asset->GetLoadedArsc()->GetPackages(); @@ -1353,15 +1353,14 @@ void Theme::SetTo(const Theme& o) { package_map[src_package_id] = dest_package_id; } - src_to_dest_asset_cookies.insert(std::pair(i, j)); - src_asset_cookie_id_map.insert( - std::pair(i, package_map)); + src_to_dest_asset_cookies.insert(std::make_pair(i, j)); + src_asset_cookie_id_map.insert(std::make_pair(i, package_map)); break; } } } - // Reset the data in the destination theme + // Reset the data in the destination theme. for (size_t p = 0; p < packages_.size(); p++) { if (packages_[p] != nullptr) { packages_[p].reset(); @@ -1387,16 +1386,17 @@ void Theme::SetTo(const Theme& o) { continue; } - // If the attribute value represents an attribute or reference, the package id of the - // value needs to be rewritten to the package id of the value in the destination - uint32_t attribue_data = entry.value.data; - if ((entry.value.dataType == Res_value::TYPE_ATTRIBUTE - || entry.value.dataType == Res_value::TYPE_REFERENCE - || entry.value.dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE - || entry.value.dataType == Res_value::TYPE_DYNAMIC_REFERENCE) - && attribue_data != 0x0) { + bool is_reference = (entry.value.dataType == Res_value::TYPE_ATTRIBUTE + || entry.value.dataType == Res_value::TYPE_REFERENCE + || entry.value.dataType == Res_value::TYPE_DYNAMIC_ATTRIBUTE + || entry.value.dataType == Res_value::TYPE_DYNAMIC_REFERENCE) + && entry.value.data != 0x0; - // Determine the package id of the reference in the destination AssetManager + // If the attribute value represents an attribute or reference, the package id of the + // value needs to be rewritten to the package id of the value in the destination. + uint32_t attribute_data = entry.value.data; + if (is_reference) { + // Determine the package id of the reference in the destination AssetManager. auto value_package_map = src_asset_cookie_id_map.find(entry.cookie); if (value_package_map == src_asset_cookie_id_map.end()) { continue; @@ -1408,14 +1408,28 @@ void Theme::SetTo(const Theme& o) { continue; } - attribue_data = fix_package_id(entry.value.data, value_dest_package->second); + attribute_data = fix_package_id(entry.value.data, value_dest_package->second); + } + + // Find the cookie of the value in the destination. If the source apk is not loaded in the + // destination, only copy resources that do not reference resources in the source. + ApkAssetsCookie data_dest_cookie; + auto value_dest_cookie = src_to_dest_asset_cookies.find(entry.cookie); + if (value_dest_cookie != src_to_dest_asset_cookies.end()) { + data_dest_cookie = value_dest_cookie->second; + } else { + if (is_reference || entry.value.dataType == Res_value::TYPE_STRING) { + continue; + } else { + data_dest_cookie = 0x0; + } } // The package id of the attribute needs to be rewritten to the package id of the - // attribute in the destination + // attribute in the destination. int attribute_dest_package_id = p; if (attribute_dest_package_id != 0x01) { - // Find the cookie of the attribute resource id + // Find the cookie of the attribute resource id in the source AssetManager FindEntryResult attribute_entry_result; ApkAssetsCookie attribute_cookie = o.asset_manager_->FindEntry(make_resid(p, t, e), 0 /* density_override */ , @@ -1423,7 +1437,7 @@ void Theme::SetTo(const Theme& o) { true /* ignore_configuration */, &attribute_entry_result); - // Determine the package id of the attribute in the destination AssetManager + // Determine the package id of the attribute in the destination AssetManager. auto attribute_package_map = src_asset_cookie_id_map.find(attribute_cookie); if (attribute_package_map == src_asset_cookie_id_map.end()) { continue; @@ -1436,13 +1450,13 @@ void Theme::SetTo(const Theme& o) { attribute_dest_package_id = attribute_dest_package->second; } - // Lazily instantiate the destination package + // Lazily instantiate the destination package. std::unique_ptr& dest_package = packages_[attribute_dest_package_id]; if (dest_package == nullptr) { dest_package.reset(new Package()); } - // Lazily instantiate and resize the destination type + // Lazily instantiate and resize the destination type. util::unique_cptr& dest_type = dest_package->types[t]; if (dest_type == nullptr || dest_type->entry_count < type->entry_count) { const size_t type_alloc_size = sizeof(ThemeType) @@ -1450,7 +1464,7 @@ void Theme::SetTo(const Theme& o) { void* dest_data = malloc(type_alloc_size); memset(dest_data, 0, type->entry_count * sizeof(ThemeEntry)); - // Copy the existing destination type values if the type is resized + // Copy the existing destination type values if the type is resized. if (dest_type != nullptr) { memcpy(dest_data, type, sizeof(ThemeType) + (dest_type->entry_count * sizeof(ThemeEntry))); @@ -1460,15 +1474,9 @@ void Theme::SetTo(const Theme& o) { dest_type->entry_count = type->entry_count; } - // Find the cookie of the value in the destination - auto value_dest_cookie = src_to_dest_asset_cookies.find(entry.cookie); - if (value_dest_cookie == src_to_dest_asset_cookies.end()) { - continue; - } - - dest_type->entries[e].cookie = value_dest_cookie->second; + dest_type->entries[e].cookie = data_dest_cookie; dest_type->entries[e].value.dataType = entry.value.dataType; - dest_type->entries[e].value.data = attribue_data; + dest_type->entries[e].value.data = attribute_data; dest_type->entries[e].type_spec_flags = entry.type_spec_flags; } } diff --git a/libs/androidfw/tests/Theme_test.cpp b/libs/androidfw/tests/Theme_test.cpp index 2c39ceead123a..be5ecd94a588b 100644 --- a/libs/androidfw/tests/Theme_test.cpp +++ b/libs/androidfw/tests/Theme_test.cpp @@ -282,48 +282,81 @@ TEST_F(ThemeTest, CopyThemeSameAssetManager) { } TEST_F(ThemeTest, OnlyCopySameAssetsThemeWhenAssetManagersDiffer) { - AssetManager2 assetmanager_one; - assetmanager_one.SetApkAssets({system_assets_.get(), lib_one_assets_.get(), style_assets_.get(), + AssetManager2 assetmanager_dst; + assetmanager_dst.SetApkAssets({system_assets_.get(), lib_one_assets_.get(), style_assets_.get(), libclient_assets_.get()}); - AssetManager2 assetmanager_two; - assetmanager_two.SetApkAssets({system_assets_.get(), lib_two_assets_.get(), lib_one_assets_.get(), + AssetManager2 assetmanager_src; + assetmanager_src.SetApkAssets({system_assets_.get(), lib_two_assets_.get(), lib_one_assets_.get(), style_assets_.get()}); - auto theme_one = assetmanager_one.NewTheme(); - ASSERT_TRUE(theme_one->ApplyStyle(app::R::style::StyleOne)); + auto theme_dst = assetmanager_dst.NewTheme(); + ASSERT_TRUE(theme_dst->ApplyStyle(app::R::style::StyleOne)); - auto theme_two = assetmanager_two.NewTheme(); - ASSERT_TRUE(theme_two->ApplyStyle(R::style::Theme_One)); - ASSERT_TRUE(theme_two->ApplyStyle(app::R::style::StyleTwo)); - ASSERT_TRUE(theme_two->ApplyStyle(fix_package_id(lib_one::R::style::Theme, 0x03), + auto theme_src = assetmanager_src.NewTheme(); + ASSERT_TRUE(theme_src->ApplyStyle(R::style::Theme_One)); + ASSERT_TRUE(theme_src->ApplyStyle(app::R::style::StyleTwo)); + ASSERT_TRUE(theme_src->ApplyStyle(fix_package_id(lib_one::R::style::Theme, 0x03), false /*force*/)); - ASSERT_TRUE(theme_two->ApplyStyle(fix_package_id(lib_two::R::style::Theme, 0x02), + ASSERT_TRUE(theme_src->ApplyStyle(fix_package_id(lib_two::R::style::Theme, 0x02), false /*force*/)); - theme_one->SetTo(*theme_two); + theme_dst->SetTo(*theme_src); Res_value value; uint32_t flags; - // System resources (present in destination asset manager) - EXPECT_EQ(0, theme_one->GetAttribute(R::attr::foreground, &value, &flags)); + // System resources (present in destination asset manager). + EXPECT_EQ(0, theme_dst->GetAttribute(R::attr::foreground, &value, &flags)); // The cookie of the style asset is 3 in the source and 2 in the destination. - // Check that the cookie has been rewritten to the destination values - EXPECT_EQ(2, theme_one->GetAttribute(app::R::attr::attr_one, &value, &flags)); + // Check that the cookie has been rewritten to the destination values. + EXPECT_EQ(2, theme_dst->GetAttribute(app::R::attr::attr_one, &value, &flags)); // The cookie of the lib_one asset is 2 in the source and 1 in the destination. // The package id of the lib_one package is 0x03 in the source and 0x02 in the destination - // Check that the cookie and packages have been rewritten to the destination values - EXPECT_EQ(1, theme_one->GetAttribute(fix_package_id(lib_one::R::attr::attr1, 0x02), &value, + // Check that the cookie and packages have been rewritten to the destination values. + EXPECT_EQ(1, theme_dst->GetAttribute(fix_package_id(lib_one::R::attr::attr1, 0x02), &value, &flags)); - EXPECT_EQ(1, theme_one->GetAttribute(fix_package_id(lib_one::R::attr::attr2, 0x02), &value, + EXPECT_EQ(1, theme_dst->GetAttribute(fix_package_id(lib_one::R::attr::attr2, 0x02), &value, &flags)); // attr2 references an attribute in lib_one. Check that the resolution of the attribute value is - // correct after the value of attr2 had its package id rewritten to the destination package id + // correct after the value of attr2 had its package id rewritten to the destination package id. EXPECT_EQ(700, value.data); } +TEST_F(ThemeTest, CopyNonReferencesWhenPackagesDiffer) { + AssetManager2 assetmanager_dst; + assetmanager_dst.SetApkAssets({system_assets_.get()}); + + AssetManager2 assetmanager_src; + assetmanager_src.SetApkAssets({system_assets_.get(), style_assets_.get()}); + + auto theme_dst = assetmanager_dst.NewTheme(); + auto theme_src = assetmanager_src.NewTheme(); + ASSERT_TRUE(theme_src->ApplyStyle(app::R::style::StyleSeven)); + theme_dst->SetTo(*theme_src); + + Res_value value; + uint32_t flags; + + // Allow inline resource values to be copied even if the source apk asset is not present in the + // destination. + EXPECT_EQ(0, theme_dst->GetAttribute(0x0101021b /* android:versionCode */, &value, &flags)); + + // Do not copy strings since the data is an index into the values string pool of the source apk + // asset. + EXPECT_EQ(-1, theme_dst->GetAttribute(0x01010001 /* android:label */, &value, &flags)); + + // Do not copy values that reference another resource if the resource is not present in the + // destination. + EXPECT_EQ(-1, theme_dst->GetAttribute(0x01010002 /* android:icon */, &value, &flags)); + EXPECT_EQ(-1, theme_dst->GetAttribute(0x010100d1 /* android:tag */, &value, &flags)); + + // Allow @empty to and @null to be copied. + EXPECT_EQ(0, theme_dst->GetAttribute(0x010100d0 /* android:id */, &value, &flags)); + EXPECT_EQ(0, theme_dst->GetAttribute(0x01010000 /* android:theme */, &value, &flags)); +} + } // namespace android diff --git a/libs/androidfw/tests/data/styles/R.h b/libs/androidfw/tests/data/styles/R.h index 538a84717176e..f11486fe924a7 100644 --- a/libs/androidfw/tests/data/styles/R.h +++ b/libs/androidfw/tests/data/styles/R.h @@ -51,6 +51,7 @@ struct R { StyleFour = 0x7f020003u, StyleFive = 0x7f020004u, StyleSix = 0x7f020005u, + StyleSeven = 0x7f020006u, }; }; }; diff --git a/libs/androidfw/tests/data/styles/build b/libs/androidfw/tests/data/styles/build index 1ef8e6e192088..7b7c1f7aa962d 100755 --- a/libs/androidfw/tests/data/styles/build +++ b/libs/androidfw/tests/data/styles/build @@ -2,5 +2,7 @@ set -e +PATH_TO_FRAMEWORK_RES=${ANDROID_BUILD_TOP}/prebuilts/sdk/current/public/android.jar + aapt2 compile -o compiled.flata --dir res -aapt2 link -o styles.apk --manifest AndroidManifest.xml compiled.flata +aapt2 link -o styles.apk --manifest AndroidManifest.xml -I $PATH_TO_FRAMEWORK_RES compiled.flata diff --git a/libs/androidfw/tests/data/styles/res/values/styles.xml b/libs/androidfw/tests/data/styles/res/values/styles.xml index 1a231768dadee..06774a8a60053 100644 --- a/libs/androidfw/tests/data/styles/res/values/styles.xml +++ b/libs/androidfw/tests/data/styles/res/values/styles.xml @@ -79,4 +79,14 @@ 3 + + + diff --git a/libs/androidfw/tests/data/styles/styles.apk b/libs/androidfw/tests/data/styles/styles.apk index cd5c7a1c6c12c06a3d7528b527b3a8028c21c641..92e9bf90101e805fbf6ded291d8b08656a141f37 100644 GIT binary patch delta 639 zcmew+d`)zMVg1yJwtj~l1X}OkGBh-i&}8*ksj1=N&nXh(!O^MZ+riu^^#6pJt*ZJ1 z9+_t0Le?K;HzyoX5_>-s9@;a0m>I@5o3>CR{J&5AqNc`I1u6&x-z zhd*#g7CjY`wX9WbSLmvp?>=Zw!;VQSXX5C(hNEAHJ!2?qtZ5^oY#+ z<<_~WIWe1-3j5C7anSy9L+s)imy8!L`se!nZvKNW#W}9IZy%l2Kfcb{@5=)%Df4(v zb2sx#i)a59NOQar5W3)8$vXA&&n2hl{$ITF-%tDVYGF-lm6Ed*CSN$duk*`Eu8k9` z*SqLv?}@ekVO#d(epP0!-ZcG5`=0k)-91~Ir)FRG+lqCYFXd#c-GBR1+uEPgif8B+ z*E0rqvvb6}T6a{Qk%3_a)5dp#jGP>XcWxK3GcXiP=3tUxESRjy6wM^UzPX91jZshs zs7#On3V`f{$&xJYjE5)3vaC}GE~(5(4Nfgf&12x=DlRF?%uA2Y&r4-sW6Mdb%r7n3 zEYI4>$f*O=%ml>jlQ*(SPj+BsnH<2%!n*@#7FZN0Q!qJ^)qS!Gy9ncl$)@a|0|XeD z8Tg=9vVmzvMh+ki)B?m{HZw>s6NC+9GS**!ut7=~F2dQ&_0mj?jFS~ODj4}E&*Z39 z697sBjba9=3-D$HnF8bkMH=QaB0^=d6=xt*0^{UfPIVc0-~a;%<|vRZ28KkY$rm^k J*ciA#8~~Nx&J6$n delta 422 zcmca6`b~I(VSVWYU%sXQo|gUUQa67Ji|g4mau+^4R3Y7DFQF{-fI;qnbA`qGg_oDR z9kDpO@Rn~bjBDf%FJ&&!>Rrj=B@z=$yKAtN@ zEVY$fHQi6S&5QWD{{2qW7T&z{{r+_4{ffcczY0(Nzc0ovN-6$~->QdioX&Es zJ0Fp$aF2Czl-tYGv+_7X`bFM-mG{ZM%~e1BK+%O8-@om&FP97OX6L9_c%o%J0|P?| z1kQ&maJl0s|mBW^yNs`{XMu zt3`@SiZb)k5fq@yQj3L0A5u_Bz2Z}VzXGFMmvJ_V!(+7sh dxm@Zpa5n?p%?DBr2Ok+HZ{Sj3Q{V)N007+bheZGY