Revert^2 "Cache resolved theme values"
6ca48473e533a8b89abac6294a0bb8130b8c8c89
Change-Id: Icb295186b85e1edcdcebc1d746f7ff0d6ef66829
Merged-In: Icb295186b85e1edcdcebc1d746f7ff0d6ef66829
diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp
index 8bab73c..a545b3d 100644
--- a/libs/androidfw/AssetManager2.cpp
+++ b/libs/androidfw/AssetManager2.cpp
@@ -963,14 +963,25 @@
}
base::expected<std::monostate, NullOrIOError> AssetManager2::ResolveReference(
- AssetManager2::SelectedValue& value) const {
+ AssetManager2::SelectedValue& value, bool cache_value) const {
if (value.type != Res_value::TYPE_REFERENCE || value.data == 0U) {
// Not a reference. Nothing to do.
return {};
}
- uint32_t combined_flags = value.flags;
- uint32_t resolve_resid = value.data;
+ const uint32_t original_flags = value.flags;
+ const uint32_t original_resid = value.data;
+ if (cache_value) {
+ auto cached_value = cached_resolved_values_.find(value.data);
+ if (cached_value != cached_resolved_values_.end()) {
+ value = cached_value->second;
+ value.flags |= original_flags;
+ return {};
+ }
+ }
+
+ uint32_t combined_flags = 0U;
+ uint32_t resolve_resid = original_resid;
constexpr const uint32_t kMaxIterations = 20;
for (uint32_t i = 0U;; i++) {
auto result = GetResource(resolve_resid, true /*may_be_bag*/);
@@ -988,6 +999,13 @@
result->data == Res_value::DATA_NULL_UNDEFINED ||
result->data == resolve_resid || i == kMaxIterations) {
// This reference can't be resolved, so exit now and let the caller deal with it.
+ if (cache_value) {
+ cached_resolved_values_[original_resid] = value;
+ }
+
+ // Above value is cached without original_flags to ensure they don't get included in future
+ // queries that hit the cache
+ value.flags |= original_flags;
return {};
}
@@ -1357,6 +1375,8 @@
++iter;
}
}
+
+ cached_resolved_values_.clear();
}
uint8_t AssetManager2::GetAssignedPackageId(const LoadedPackage* package) const {
@@ -1537,7 +1557,7 @@
return base::unexpected(std::nullopt);
}
- auto resolve_result = asset_manager_->ResolveReference(*result);
+ auto resolve_result = asset_manager_->ResolveReference(*result, true /* cache_value */);
if (resolve_result.has_value()) {
result->flags |= value.flags;
value = *result;
diff --git a/libs/androidfw/AttributeResolution.cpp b/libs/androidfw/AttributeResolution.cpp
index d07b452..c188712 100644
--- a/libs/androidfw/AttributeResolution.cpp
+++ b/libs/androidfw/AttributeResolution.cpp
@@ -39,8 +39,7 @@
: public BackTrackingAttributeFinder<XmlAttributeFinder, size_t> {
public:
explicit XmlAttributeFinder(const ResXMLParser* parser)
- : BackTrackingAttributeFinder(
- 0, parser != nullptr ? parser->getAttributeCount() : 0),
+ : BackTrackingAttributeFinder(0, parser != nullptr ? parser->getAttributeCount() : 0),
parser_(parser) {}
inline uint32_t GetAttribute(size_t index) const {
@@ -178,7 +177,7 @@
value = *attr_value;
DEBUG_LOG("-> From theme: type=0x%x, data=0x%08x", value.type, value.data);
- const auto result = assetmanager->ResolveReference(value);
+ const auto result = assetmanager->ResolveReference(value, true /* cache_value */);
if (UNLIKELY(IsIOError(result))) {
return base::unexpected(GetIOError(result.error()));
}
@@ -310,7 +309,7 @@
value = *attr_value;
DEBUG_LOG("-> From theme: type=0x%x, data=0x%08x", value.type, value.data);
- auto result = assetmanager->ResolveReference(value);
+ auto result = assetmanager->ResolveReference(value, true /* cache_value */);
if (UNLIKELY(IsIOError(result))) {
return base::unexpected(GetIOError(result.error()));
}
diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h
index 4e993b0..a92694c 100644
--- a/libs/androidfw/include/androidfw/AssetManager2.h
+++ b/libs/androidfw/include/androidfw/AssetManager2.h
@@ -264,11 +264,14 @@
// Resolves the resource referenced in `value` if the type is Res_value::TYPE_REFERENCE.
//
// If the data type is not Res_value::TYPE_REFERENCE, no work is done. Configuration flags of the
- // values pointed to by the reference are OR'd into `value.flags`.
+ // values pointed to by the reference are OR'd into `value.flags`. If `cache_value` is true, then
+ // the resolved value will be cached and used when attempting to resolve the resource id specified
+ // in `value`.
//
// Returns a null error if the resource could not be resolved, or an I/O error if reading
// resource data failed.
- base::expected<std::monostate, NullOrIOError> ResolveReference(SelectedValue& value) const;
+ base::expected<std::monostate, NullOrIOError> ResolveReference(SelectedValue& value,
+ bool cache_value = false) const;
// Retrieves the best matching bag/map resource with ID `resid`.
//
@@ -446,13 +449,14 @@
// a number of times for each view during View inspection.
mutable std::unordered_map<uint32_t, std::vector<uint32_t>> cached_bag_resid_stacks_;
+ // Cached set of resolved resource values.
+ mutable std::unordered_map<uint32_t, SelectedValue> cached_resolved_values_;
+
// Whether or not to save resource resolution steps
bool resource_resolution_logging_enabled_ = false;
struct Resolution {
-
struct Step {
-
enum class Type {
INITIAL,
BETTER_MATCH,
diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp
index 0f5afd4..471b0ee 100644
--- a/libs/androidfw/tests/AssetManager2_test.cpp
+++ b/libs/androidfw/tests/AssetManager2_test.cpp
@@ -458,6 +458,37 @@
EXPECT_EQ(basic::R::string::test1, value.resid);
}
+TEST_F(AssetManager2Test, ResolveReferenceMissingResourceDoNotCacheFlags) {
+ AssetManager2 assetmanager;
+ assetmanager.SetApkAssets({basic_assets_.get()});
+ {
+ AssetManager2::SelectedValue value{};
+ value.data = basic::R::string::test1;
+ value.type = Res_value::TYPE_REFERENCE;
+ value.flags = ResTable_config::CONFIG_KEYBOARD;
+
+ auto result = assetmanager.ResolveReference(value);
+ ASSERT_TRUE(result.has_value());
+ EXPECT_EQ(Res_value::TYPE_STRING, value.type);
+ EXPECT_EQ(0, value.cookie);
+ EXPECT_EQ(basic::R::string::test1, value.resid);
+ EXPECT_EQ(ResTable_typeSpec::SPEC_PUBLIC | ResTable_config::CONFIG_KEYBOARD, value.flags);
+ }
+ {
+ AssetManager2::SelectedValue value{};
+ value.data = basic::R::string::test1;
+ value.type = Res_value::TYPE_REFERENCE;
+ value.flags = ResTable_config::CONFIG_COLOR_MODE;
+
+ auto result = assetmanager.ResolveReference(value);
+ ASSERT_TRUE(result.has_value());
+ EXPECT_EQ(Res_value::TYPE_STRING, value.type);
+ EXPECT_EQ(0, value.cookie);
+ EXPECT_EQ(basic::R::string::test1, value.resid);
+ EXPECT_EQ(ResTable_typeSpec::SPEC_PUBLIC | ResTable_config::CONFIG_COLOR_MODE, value.flags);
+ }
+}
+
TEST_F(AssetManager2Test, ResolveReferenceMissingResource) {
AssetManager2 assetmanager;
assetmanager.SetApkAssets({basic_assets_.get()});