From 3f1f4fc11d2e19c4b297a97d21293d05ac3db622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Fri, 2 Mar 2018 09:34:18 +0100 Subject: [PATCH 1/3] libandroidfw: add resource ID iterator Add an iterator to LoadedPackage which allows the caller to iterate over the resource IDs in the package. This will be used by idmap when constructing the idmap file. Bug: 78815803 Test: make libandroidfw_tests Change-Id: Ia47daa21390d67ea2ef3665e88eb407837c4764f --- libs/androidfw/LoadedArsc.cpp | 34 +++++++++++++ libs/androidfw/include/androidfw/LoadedArsc.h | 50 +++++++++++++++++++ libs/androidfw/tests/LoadedArsc_test.cpp | 48 ++++++++++++++++++ 3 files changed, 132 insertions(+) diff --git a/libs/androidfw/LoadedArsc.cpp b/libs/androidfw/LoadedArsc.cpp index 04d506a2d71cc..21f023dc0f05b 100644 --- a/libs/androidfw/LoadedArsc.cpp +++ b/libs/androidfw/LoadedArsc.cpp @@ -203,6 +203,39 @@ static bool VerifyResTableEntry(const ResTable_type* type, uint32_t entry_offset return true; } +LoadedPackage::iterator::iterator(const LoadedPackage* lp, size_t ti, size_t ei) + : loadedPackage_(lp), + typeIndex_(ti), + entryIndex_(ei), + typeIndexEnd_(lp->resource_ids_.size() + 1) { + while (typeIndex_ < typeIndexEnd_ && loadedPackage_->resource_ids_[typeIndex_] == 0) { + typeIndex_++; + } +} + +LoadedPackage::iterator& LoadedPackage::iterator::operator++() { + while (typeIndex_ < typeIndexEnd_) { + if (entryIndex_ + 1 < loadedPackage_->resource_ids_[typeIndex_]) { + entryIndex_++; + break; + } + entryIndex_ = 0; + typeIndex_++; + if (typeIndex_ < typeIndexEnd_ && loadedPackage_->resource_ids_[typeIndex_] != 0) { + break; + } + } + return *this; +} + +uint32_t LoadedPackage::iterator::operator*() const { + if (typeIndex_ >= typeIndexEnd_) { + return 0; + } + return make_resid(loadedPackage_->package_id_, typeIndex_ + loadedPackage_->type_id_offset_, + entryIndex_); +} + const ResTable_entry* LoadedPackage::GetEntry(const ResTable_type* type_chunk, uint16_t entry_index) { uint32_t entry_offset = GetEntryOffset(type_chunk, entry_index); @@ -488,6 +521,7 @@ std::unique_ptr LoadedPackage::Load(const Chunk& chunk, std::unique_ptr& builder_ptr = type_builder_map[type_spec->id - 1]; if (builder_ptr == nullptr) { builder_ptr = util::make_unique(type_spec, idmap_entry_header); + loaded_package->resource_ids_.set(type_spec->id, entry_count); } else { LOG(WARNING) << StringPrintf("RES_TABLE_TYPE_SPEC_TYPE already defined for ID %02x", type_spec->id); diff --git a/libs/androidfw/include/androidfw/LoadedArsc.h b/libs/androidfw/include/androidfw/LoadedArsc.h index 35ae5fcd9e7bb..349b379778a63 100644 --- a/libs/androidfw/include/androidfw/LoadedArsc.h +++ b/libs/androidfw/include/androidfw/LoadedArsc.h @@ -78,6 +78,55 @@ using TypeSpecPtr = util::unique_cptr; class LoadedPackage { public: + class iterator { + public: + iterator& operator=(const iterator& rhs) { + loadedPackage_ = rhs.loadedPackage_; + typeIndex_ = rhs.typeIndex_; + entryIndex_ = rhs.entryIndex_; + return *this; + } + + bool operator==(const iterator& rhs) const { + return loadedPackage_ == rhs.loadedPackage_ && + typeIndex_ == rhs.typeIndex_ && + entryIndex_ == rhs.entryIndex_; + } + + bool operator!=(const iterator& rhs) const { + return !(*this == rhs); + } + + iterator operator++(int) { + size_t prevTypeIndex_ = typeIndex_; + size_t prevEntryIndex_ = entryIndex_; + operator++(); + return iterator(loadedPackage_, prevTypeIndex_, prevEntryIndex_); + } + + iterator& operator++(); + + uint32_t operator*() const; + + private: + friend class LoadedPackage; + + iterator(const LoadedPackage* lp, size_t ti, size_t ei); + + const LoadedPackage* loadedPackage_; + size_t typeIndex_; + size_t entryIndex_; + const size_t typeIndexEnd_; // STL style end, so one past the last element + }; + + iterator begin() const { + return iterator(this, 0, 0); + } + + iterator end() const { + return iterator(this, resource_ids_.size() + 1, 0); + } + static std::unique_ptr Load(const Chunk& chunk, const LoadedIdmap* loaded_idmap, bool system, bool load_as_shared_library); @@ -182,6 +231,7 @@ class LoadedPackage { bool overlay_ = false; ByteBucketArray type_specs_; + ByteBucketArray resource_ids_; std::vector dynamic_package_map_; }; diff --git a/libs/androidfw/tests/LoadedArsc_test.cpp b/libs/androidfw/tests/LoadedArsc_test.cpp index cae632ddea300..ffa48367c2523 100644 --- a/libs/androidfw/tests/LoadedArsc_test.cpp +++ b/libs/androidfw/tests/LoadedArsc_test.cpp @@ -278,4 +278,52 @@ TEST(LoadedArscTest, LoadOverlay) { // sizeof(Res_value) might not be backwards compatible. TEST(LoadedArscTest, LoadingShouldBeForwardsAndBackwardsCompatible) { ASSERT_TRUE(false); } +TEST(LoadedArscTest, ResourceIdentifierIterator) { + std::string contents; + ASSERT_TRUE( + ReadFileFromZipToString(GetTestDataPath() + "/basic/basic.apk", "resources.arsc", &contents)); + + std::unique_ptr loaded_arsc = LoadedArsc::Load(StringPiece(contents)); + ASSERT_NE(nullptr, loaded_arsc); + + const std::vector>& packages = loaded_arsc->GetPackages(); + ASSERT_EQ(1u, packages.size()); + EXPECT_EQ(std::string("com.android.basic"), packages[0]->GetPackageName()); + + const auto& loaded_package = packages[0]; + auto iter = loaded_package->begin(); + auto end = loaded_package->end(); + + ASSERT_NE(end, iter); + ASSERT_EQ(0x7f010000u, *iter++); + ASSERT_EQ(0x7f010001u, *iter++); + ASSERT_EQ(0x7f020000u, *iter++); + ASSERT_EQ(0x7f020001u, *iter++); + ASSERT_EQ(0x7f030000u, *iter++); + ASSERT_EQ(0x7f030001u, *iter++); + ASSERT_EQ(0x7f030002u, *iter++); // note: string without default, excluded by aapt2 dump + ASSERT_EQ(0x7f040000u, *iter++); + ASSERT_EQ(0x7f040001u, *iter++); + ASSERT_EQ(0x7f040002u, *iter++); + ASSERT_EQ(0x7f040003u, *iter++); + ASSERT_EQ(0x7f040004u, *iter++); + ASSERT_EQ(0x7f040005u, *iter++); + ASSERT_EQ(0x7f040006u, *iter++); + ASSERT_EQ(0x7f040007u, *iter++); + ASSERT_EQ(0x7f040008u, *iter++); + ASSERT_EQ(0x7f040009u, *iter++); + ASSERT_EQ(0x7f04000au, *iter++); + ASSERT_EQ(0x7f04000bu, *iter++); + ASSERT_EQ(0x7f04000cu, *iter++); + ASSERT_EQ(0x7f04000du, *iter++); + ASSERT_EQ(0x7f050000u, *iter++); + ASSERT_EQ(0x7f050001u, *iter++); + ASSERT_EQ(0x7f060000u, *iter++); + ASSERT_EQ(0x7f070000u, *iter++); + ASSERT_EQ(0x7f070001u, *iter++); + ASSERT_EQ(0x7f070002u, *iter++); + ASSERT_EQ(0x7f070003u, *iter++); + ASSERT_EQ(end, iter); +} + } // namespace android From f99eda450f172f13affa1c08ebea9c3be00ac4b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Mon, 11 Jun 2018 14:13:37 +0200 Subject: [PATCH 2/3] AAPT2: optionally keep resources without default value Teach "aapt2 link" about a new flag --no-resource-removal. When given, aapt2 will not filter out resources that lack default values. This is useful mostly when building overlay packages that define resources for non-default configurations, such as only for values-sv. Test: manual: build package with resource only in values-vs, verify apk with aapt2 dump Change-Id: Idc513bcb3f43bbff7f073163562c3dfccdb9bc9b Merged-In: Idc513bcb3f43bbff7f073163562c3dfccdb9bc9b --- .../device/test-apps/AppOverlayOne/Android.mk | 2 ++ .../device/test-apps/AppOverlayTwo/Android.mk | 2 ++ .../device/test-apps/FrameworkOverlay/Android.mk | 2 ++ .../host/test-apps/UpdateOverlay/Android.mk | 2 ++ tools/aapt2/cmd/Link.cpp | 15 +++++++++++---- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/core/tests/overlaytests/device/test-apps/AppOverlayOne/Android.mk b/core/tests/overlaytests/device/test-apps/AppOverlayOne/Android.mk index edad4b26314c3..97a3d0078e26c 100644 --- a/core/tests/overlaytests/device/test-apps/AppOverlayOne/Android.mk +++ b/core/tests/overlaytests/device/test-apps/AppOverlayOne/Android.mk @@ -20,4 +20,6 @@ LOCAL_PACKAGE_NAME := OverlayDeviceTests_AppOverlayOne LOCAL_SDK_VERSION := current LOCAL_COMPATIBILITY_SUITE := device-tests LOCAL_CERTIFICATE := platform +LOCAL_USE_AAPT2 := true +LOCAL_AAPT_FLAGS := --no-resource-removal include $(BUILD_PACKAGE) diff --git a/core/tests/overlaytests/device/test-apps/AppOverlayTwo/Android.mk b/core/tests/overlaytests/device/test-apps/AppOverlayTwo/Android.mk index 3fae8e19fc5b0..a3470255997d3 100644 --- a/core/tests/overlaytests/device/test-apps/AppOverlayTwo/Android.mk +++ b/core/tests/overlaytests/device/test-apps/AppOverlayTwo/Android.mk @@ -20,4 +20,6 @@ LOCAL_PACKAGE_NAME := OverlayDeviceTests_AppOverlayTwo LOCAL_SDK_VERSION := current LOCAL_COMPATIBILITY_SUITE := device-tests LOCAL_CERTIFICATE := platform +LOCAL_USE_AAPT2 := true +LOCAL_AAPT_FLAGS := --no-resource-removal include $(BUILD_PACKAGE) diff --git a/core/tests/overlaytests/device/test-apps/FrameworkOverlay/Android.mk b/core/tests/overlaytests/device/test-apps/FrameworkOverlay/Android.mk index c352c0550034f..e4819e138eba8 100644 --- a/core/tests/overlaytests/device/test-apps/FrameworkOverlay/Android.mk +++ b/core/tests/overlaytests/device/test-apps/FrameworkOverlay/Android.mk @@ -20,4 +20,6 @@ LOCAL_PACKAGE_NAME := OverlayDeviceTests_FrameworkOverlay LOCAL_SDK_VERSION := current LOCAL_COMPATIBILITY_SUITE := device-tests LOCAL_CERTIFICATE := platform +LOCAL_USE_AAPT2 := true +LOCAL_AAPT_FLAGS := --no-resource-removal include $(BUILD_PACKAGE) diff --git a/core/tests/overlaytests/host/test-apps/UpdateOverlay/Android.mk b/core/tests/overlaytests/host/test-apps/UpdateOverlay/Android.mk index ab3faf0b158a8..8656781e31e58 100644 --- a/core/tests/overlaytests/host/test-apps/UpdateOverlay/Android.mk +++ b/core/tests/overlaytests/host/test-apps/UpdateOverlay/Android.mk @@ -21,6 +21,8 @@ LOCAL_PACKAGE_NAME := OverlayHostTests_UpdateOverlay LOCAL_SDK_VERSION := current LOCAL_COMPATIBILITY_SUITE := general-tests LOCAL_STATIC_JAVA_LIBRARIES := android-support-test +LOCAL_USE_AAPT2 := true +LOCAL_AAPT_FLAGS := --no-resource-removal include $(BUILD_PACKAGE) my_package_prefix := com.android.server.om.hosttest.framework_overlay diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index db42e7cb3e021..670a2de3cfd28 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -105,6 +105,7 @@ struct LinkOptions { bool no_version_vectors = false; bool no_version_transitions = false; bool no_resource_deduping = false; + bool no_resource_removal = false; bool no_xml_namespaces = false; bool do_not_compress_anything = false; std::unordered_set extensions_to_not_compress; @@ -1806,10 +1807,12 @@ class LinkCommand { // Before we process anything, remove the resources whose default values don't exist. // We want to force any references to these to fail the build. - if (!NoDefaultResourceRemover{}.Consume(context_, &final_table_)) { - context_->GetDiagnostics()->Error(DiagMessage() - << "failed removing resources with no defaults"); - return 1; + if (!options_.no_resource_removal) { + if (!NoDefaultResourceRemover{}.Consume(context_, &final_table_)) { + context_->GetDiagnostics()->Error(DiagMessage() + << "failed removing resources with no defaults"); + return 1; + } } ReferenceLinker linker; @@ -2084,6 +2087,10 @@ int Link(const std::vector& args, IDiagnostics* diagnostics) { "Disables automatic deduping of resources with\n" "identical values across compatible configurations.", &options.no_resource_deduping) + .OptionalSwitch("--no-resource-removal", + "Disables automatic removal of resources without defaults. Use this only\n" + "when building runtime resource overlay packages.", + &options.no_resource_removal) .OptionalSwitch("--enable-sparse-encoding", "Enables encoding sparse entries using a binary search tree.\n" "This decreases APK size at the cost of resource retrieval performance.", From 668ec5bd3bd9d5fe6ebf46985eacd3d8d81af937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Kongstad?= Date: Mon, 11 Jun 2018 14:11:33 +0200 Subject: [PATCH 3/3] AssetManager2: optionally keep non-matching configs AssetManager2 maintains a set of configurations [as specified in the resource blob] compatible with the currently set configuration [as specified via SetConfiguration]. This helps optimize future resource lookups by limiting the set of configurations to iterate over. However, when creating idmaps, all configurations must be considered, including those not compatible with the currently set configuration. Add an optional flag to SetApkAssets to disable the optimization described above. Test: manual (will be tested by upcoming idmap2 implementation) Change-Id: I7526a323ddf90e2f2f49c36e8c110a2cec25357e --- libs/androidfw/AssetManager2.cpp | 8 ++++---- libs/androidfw/include/androidfw/AssetManager2.h | 9 +++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 9c1629bc36f58..04cc5bb30ade1 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -67,10 +67,10 @@ AssetManager2::AssetManager2() { } bool AssetManager2::SetApkAssets(const std::vector& apk_assets, - bool invalidate_caches) { + bool invalidate_caches, bool filter_incompatible_configs) { apk_assets_ = apk_assets; BuildDynamicRefTable(); - RebuildFilterList(); + RebuildFilterList(filter_incompatible_configs); if (invalidate_caches) { InvalidateCaches(static_cast(-1)); } @@ -825,7 +825,7 @@ uint32_t AssetManager2::GetResourceId(const std::string& resource_name, return 0u; } -void AssetManager2::RebuildFilterList() { +void AssetManager2::RebuildFilterList(bool filter_incompatible_configs) { for (PackageGroup& group : package_groups_) { for (ConfiguredPackage& impl : group.packages_) { // Destroy it. @@ -841,7 +841,7 @@ void AssetManager2::RebuildFilterList() { for (auto iter = spec->types; iter != iter_end; ++iter) { ResTable_config this_config; this_config.copyFromDtoH((*iter)->config); - if (this_config.match(configuration_)) { + if (!filter_incompatible_configs || this_config.match(configuration_)) { group.configurations.push_back(this_config); group.types.push_back(*iter); } diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h index ad31f69404389..2f0ee01639fe9 100644 --- a/libs/androidfw/include/androidfw/AssetManager2.h +++ b/libs/androidfw/include/androidfw/AssetManager2.h @@ -96,7 +96,12 @@ class AssetManager2 { // 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(const std::vector& apk_assets, bool invalidate_caches = true); + // + // Only pass in filter_incompatible_configs=false when you want to load all + // configurations (including incompatible ones) such as when constructing an + // idmap. + bool SetApkAssets(const std::vector& apk_assets, bool invalidate_caches = true, + bool filter_incompatible_configs = true); inline const std::vector GetApkAssets() const { return apk_assets_; @@ -274,7 +279,7 @@ class AssetManager2 { // Triggers the re-construction of lists of types that match the set configuration. // This should always be called when mutating the AssetManager's configuration or ApkAssets set. - void RebuildFilterList(); + void RebuildFilterList(bool filter_incompatible_configs = true); // AssetManager2::GetBag(resid) wraps this function to track which resource ids have already // been seen while traversing bag parents.