Revert^2 "Cache resolved theme values"

6ca48473e5

Change-Id: Icb295186b85e1edcdcebc1d746f7ff0d6ef66829
Merged-In: Icb295186b85e1edcdcebc1d746f7ff0d6ef66829

libs/androidfw/AssetManager2.cpp

@@ -963,14 +963,25 @@ base::expected<AssetManager2::SelectedValue, NullOrIOError> AssetManager2::GetRe
 }
 
 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 @@ base::expected<std::monostate, NullOrIOError> AssetManager2::ResolveReference(
         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 @@ void AssetManager2::InvalidateCaches(uint32_t diff) {
       ++iter;
     }
   }
+
+  cached_resolved_values_.clear();
 }
 
 uint8_t AssetManager2::GetAssignedPackageId(const LoadedPackage* package) const {
@@ -1537,7 +1557,7 @@ base::expected<std::monostate, NullOrIOError> Theme::ResolveAttributeReference(
     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;

libs/androidfw/AttributeResolution.cpp

@@ -39,8 +39,7 @@ class XmlAttributeFinder
     : 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 @@ base::expected<std::monostate, IOError> ResolveAttrs(Theme* theme, uint32_t def_
         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 @@ base::expected<std::monostate, IOError> ApplyStyle(Theme* theme, ResXMLParser* x
         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()));
         }

libs/androidfw/include/androidfw/AssetManager2.h

@@ -264,11 +264,14 @@ class AssetManager2 {
   // 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 @@ class AssetManager2 {
   // 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,

libs/androidfw/tests/AssetManager2_test.cpp

@@ -458,6 +458,37 @@ TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved)
   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()});