Persist whether xml resources have flags
This stores whether an xml document has feature flags in it in a
ResTable_entry so that at runtime we can know not to do extra work
looking for flags.
Test: Automation
Bug: 377974898
Flag: android.content.res.layout_readwrite_flags
Change-Id: Id43b2d9941d1fab8c654d081bf19df5a33a464f3
diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index a18c5f5..8ecd6ba 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -6520,41 +6520,79 @@
}
bool ResTable::getResourceFlags(uint32_t resID, uint32_t* outFlags) const {
- if (mError != NO_ERROR) {
- return false;
- }
+ if (mError != NO_ERROR) {
+ return false;
+ }
- const ssize_t p = getResourcePackageIndex(resID);
- const int t = Res_GETTYPE(resID);
- const int e = Res_GETENTRY(resID);
+ const ssize_t p = getResourcePackageIndex(resID);
+ const int t = Res_GETTYPE(resID);
+ const int e = Res_GETENTRY(resID);
- if (p < 0) {
- if (Res_GETPACKAGE(resID)+1 == 0) {
- ALOGW("No package identifier when getting flags for resource number 0x%08x", resID);
- } else {
- ALOGW("No known package when getting flags for resource number 0x%08x", resID);
- }
- return false;
+ if (p < 0) {
+ if (Res_GETPACKAGE(resID)+1 == 0) {
+ ALOGW("No package identifier when getting flags for resource number 0x%08x", resID);
+ } else {
+ ALOGW("No known package when getting flags for resource number 0x%08x", resID);
}
- if (t < 0) {
- ALOGW("No type identifier when getting flags for resource number 0x%08x", resID);
- return false;
- }
+ return false;
+ }
+ if (t < 0) {
+ ALOGW("No type identifier when getting flags for resource number 0x%08x", resID);
+ return false;
+ }
- const PackageGroup* const grp = mPackageGroups[p];
- if (grp == NULL) {
- ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID);
- return false;
- }
+ const PackageGroup* const grp = mPackageGroups[p];
+ if (grp == NULL) {
+ ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID);
+ return false;
+ }
- Entry entry;
- status_t err = getEntry(grp, t, e, NULL, &entry);
- if (err != NO_ERROR) {
- return false;
- }
+ Entry entry;
+ status_t err = getEntry(grp, t, e, NULL, &entry);
+ if (err != NO_ERROR) {
+ return false;
+ }
- *outFlags = entry.specFlags;
- return true;
+ *outFlags = entry.specFlags;
+ return true;
+}
+
+bool ResTable::getResourceEntryFlags(uint32_t resID, uint32_t* outFlags) const {
+ if (mError != NO_ERROR) {
+ return false;
+ }
+
+ const ssize_t p = getResourcePackageIndex(resID);
+ const int t = Res_GETTYPE(resID);
+ const int e = Res_GETENTRY(resID);
+
+ if (p < 0) {
+ if (Res_GETPACKAGE(resID)+1 == 0) {
+ ALOGW("No package identifier when getting flags for resource number 0x%08x", resID);
+ } else {
+ ALOGW("No known package when getting flags for resource number 0x%08x", resID);
+ }
+ return false;
+ }
+ if (t < 0) {
+ ALOGW("No type identifier when getting flags for resource number 0x%08x", resID);
+ return false;
+ }
+
+ const PackageGroup* const grp = mPackageGroups[p];
+ if (grp == NULL) {
+ ALOGW("Bad identifier when getting flags for resource number 0x%08x", resID);
+ return false;
+ }
+
+ Entry entry;
+ status_t err = getEntry(grp, t, e, NULL, &entry);
+ if (err != NO_ERROR) {
+ return false;
+ }
+
+ *outFlags = entry.entry->flags();
+ return true;
}
bool ResTable::isPackageDynamic(uint8_t packageID) const {
diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h
index 0d45149..63b28da 100644
--- a/libs/androidfw/include/androidfw/ResourceTypes.h
+++ b/libs/androidfw/include/androidfw/ResourceTypes.h
@@ -1593,6 +1593,8 @@
// If set, this is a compact entry with data type and value directly
// encoded in the this entry, see ResTable_entry::compact
FLAG_COMPACT = 0x0008,
+ // If set, this entry relies on read write android feature flags
+ FLAG_USES_FEATURE_FLAGS = 0x0010,
};
struct Full {
@@ -1622,6 +1624,7 @@
uint16_t flags() const { return dtohs(full.flags); };
bool is_compact() const { return flags() & FLAG_COMPACT; }
bool is_complex() const { return flags() & FLAG_COMPLEX; }
+ bool uses_feature_flags() const { return flags() & FLAG_USES_FEATURE_FLAGS; }
size_t size() const {
return is_compact() ? sizeof(ResTable_entry) : dtohs(this->full.size);
@@ -2039,6 +2042,8 @@
bool getResourceFlags(uint32_t resID, uint32_t* outFlags) const;
+ bool getResourceEntryFlags(uint32_t resID, uint32_t* outFlags) const;
+
/**
* Returns whether or not the package for the given resource has been dynamically assigned.
* If the resource can't be found, returns 'false'.
diff --git a/tools/aapt2/Resource.h b/tools/aapt2/Resource.h
index 0d261ab..e51477c 100644
--- a/tools/aapt2/Resource.h
+++ b/tools/aapt2/Resource.h
@@ -249,6 +249,8 @@
// Flag
std::optional<FeatureFlagAttribute> flag;
+
+ bool uses_readwrite_feature_flags = false;
};
/**
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 5435cba2..db7dddc 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -664,6 +664,7 @@
if (!config_value->value) {
// Resource does not exist, add it now.
config_value->value = std::move(res.value);
+ config_value->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags;
} else {
// When validation is enabled, ensure that a resource cannot have multiple values defined for
// the same configuration unless protected by flags.
@@ -681,12 +682,14 @@
ConfigKey{&res.config, res.product}, lt_config_key_ref()),
util::make_unique<ResourceConfigValue>(res.config, res.product));
(*it)->value = std::move(res.value);
+ (*it)->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags;
break;
}
case CollisionResult::kTakeNew:
// Take the incoming value.
config_value->value = std::move(res.value);
+ config_value->uses_readwrite_feature_flags = res.uses_readwrite_feature_flags;
break;
case CollisionResult::kConflict:
@@ -843,6 +846,12 @@
return *this;
}
+NewResourceBuilder& NewResourceBuilder::SetUsesReadWriteFeatureFlags(
+ bool uses_readwrite_feature_flags) {
+ res_.uses_readwrite_feature_flags = uses_readwrite_feature_flags;
+ return *this;
+}
+
NewResource NewResourceBuilder::Build() {
return std::move(res_);
}
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index b0e1855..778b43a 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -104,6 +104,9 @@
// The actual Value.
std::unique_ptr<Value> value;
+ // Whether the value uses read/write feature flags
+ bool uses_readwrite_feature_flags = false;
+
ResourceConfigValue(const android::ConfigDescription& config, android::StringPiece product)
: config(config), product(product) {
}
@@ -284,6 +287,7 @@
std::optional<AllowNew> allow_new;
std::optional<StagedId> staged_id;
bool allow_mangled = false;
+ bool uses_readwrite_feature_flags = false;
};
struct NewResourceBuilder {
@@ -297,6 +301,7 @@
NewResourceBuilder& SetAllowNew(AllowNew allow_new);
NewResourceBuilder& SetStagedId(StagedId id);
NewResourceBuilder& SetAllowMangled(bool allow_mangled);
+ NewResourceBuilder& SetUsesReadWriteFeatureFlags(bool uses_feature_flags);
NewResource Build();
private:
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 2a79216..755dbb6 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -673,11 +673,13 @@
// Update the output format of this XML file.
file_ref->type = XmlFileTypeForOutputFormat(options_.output_format);
- bool result = table->AddResource(NewResourceBuilder(file.name)
- .SetValue(std::move(file_ref), file.config)
- .SetAllowMangled(true)
- .Build(),
- context_->GetDiagnostics());
+ bool result = table->AddResource(
+ NewResourceBuilder(file.name)
+ .SetValue(std::move(file_ref), file.config)
+ .SetAllowMangled(true)
+ .SetUsesReadWriteFeatureFlags(doc->file.uses_readwrite_feature_flags)
+ .Build(),
+ context_->GetDiagnostics());
if (!result) {
return false;
}
diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp
index 2e20e81..bac871b 100644
--- a/tools/aapt2/format/binary/BinaryResourceParser.cpp
+++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp
@@ -414,6 +414,8 @@
.SetId(res_id, OnIdConflict::CREATE_ENTRY)
.SetAllowMangled(true);
+ res_builder.SetUsesReadWriteFeatureFlags(entry->uses_feature_flags());
+
if (entry->flags() & ResTable_entry::FLAG_PUBLIC) {
Visibility visibility{Visibility::Level::kPublic};
diff --git a/tools/aapt2/format/binary/ResEntryWriter.cpp b/tools/aapt2/format/binary/ResEntryWriter.cpp
index 9dc205f..0be3921 100644
--- a/tools/aapt2/format/binary/ResEntryWriter.cpp
+++ b/tools/aapt2/format/binary/ResEntryWriter.cpp
@@ -199,6 +199,10 @@
flags |= ResTable_entry::FLAG_WEAK;
}
+ if (entry->uses_readwrite_feature_flags) {
+ flags |= ResTable_entry::FLAG_USES_FEATURE_FLAGS;
+ }
+
if constexpr (std::is_same_v<ResTable_entry_ext, T>) {
flags |= ResTable_entry::FLAG_COMPLEX;
}
diff --git a/tools/aapt2/format/binary/ResEntryWriter.h b/tools/aapt2/format/binary/ResEntryWriter.h
index c11598e..f54b29a 100644
--- a/tools/aapt2/format/binary/ResEntryWriter.h
+++ b/tools/aapt2/format/binary/ResEntryWriter.h
@@ -38,6 +38,8 @@
// The entry string pool index to the entry's name.
uint32_t entry_key;
+
+ bool uses_readwrite_feature_flags;
};
// Pair of ResTable_entry and Res_value. These pairs are stored sequentially in values buffer.
diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp
index 1a82021..50144ae 100644
--- a/tools/aapt2/format/binary/TableFlattener.cpp
+++ b/tools/aapt2/format/binary/TableFlattener.cpp
@@ -502,7 +502,8 @@
// Group values by configuration.
for (auto& config_value : entry.values) {
config_to_entry_list_map[config_value->config].push_back(
- FlatEntry{&entry, config_value->value.get(), local_key_index});
+ FlatEntry{&entry, config_value->value.get(), local_key_index,
+ config_value->uses_readwrite_feature_flags});
}
}
diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp
index 0f11685..9156b96 100644
--- a/tools/aapt2/format/binary/TableFlattener_test.cpp
+++ b/tools/aapt2/format/binary/TableFlattener_test.cpp
@@ -1069,4 +1069,23 @@
testing::IsTrue());
}
+TEST_F(TableFlattenerTest, UsesReadWriteFeatureFlagSerializesCorrectly) {
+ std::unique_ptr<ResourceTable> table =
+ test::ResourceTableBuilder()
+ .Add(NewResourceBuilder("com.app.a:color/foo")
+ .SetValue(util::make_unique<BinaryPrimitive>(
+ uint8_t(Res_value::TYPE_INT_COLOR_ARGB8), 0xffaabbcc))
+ .SetUsesReadWriteFeatureFlags(true)
+ .SetId(0x7f020000)
+ .Build())
+ .Build();
+ ResTable res_table;
+ TableFlattenerOptions options;
+ ASSERT_TRUE(Flatten(context_.get(), options, table.get(), &res_table));
+
+ uint32_t flags;
+ ASSERT_TRUE(res_table.getResourceEntryFlags(0x7f020000, &flags));
+ ASSERT_EQ(flags, ResTable_entry::FLAG_USES_FEATURE_FLAGS);
+}
+
} // namespace aapt
diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp
index dbef776..47a71fe 100644
--- a/tools/aapt2/link/FlaggedResources_test.cpp
+++ b/tools/aapt2/link/FlaggedResources_test.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#include <regex>
+#include <string>
+
#include "LoadedApk.h"
#include "cmd/Dump.h"
#include "io/StringStream.h"
@@ -183,4 +186,49 @@
"Only read only flags may be used with resources: test.package.rwFlag"));
}
+TEST_F(FlaggedResourcesTest, ReadWriteFlagInXmlGetsFlagged) {
+ auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"});
+ auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag);
+
+ std::string output;
+ DumpChunksToString(loaded_apk.get(), &output);
+
+ // The actual line looks something like:
+ // [ResTable_entry] id: 0x0000 name: layout1 keyIndex: 14 size: 8 flags: 0x0010
+ //
+ // This regex matches that line and captures the name and the flag value for checking.
+ std::regex regex("[0-9a-zA-Z:_\\]\\[ ]+name: ([0-9a-zA-Z]+)[0-9a-zA-Z: ]+flags: (0x\\d{4})");
+ std::smatch match;
+
+ std::stringstream ss(output);
+ std::string line;
+ bool found = false;
+ int fields_flagged = 0;
+ while (std::getline(ss, line)) {
+ bool first_line = false;
+ if (line.contains("config: v36")) {
+ std::getline(ss, line);
+ first_line = true;
+ }
+ if (!line.contains("flags")) {
+ continue;
+ }
+ if (std::regex_search(line, match, regex) && (match.size() == 3)) {
+ unsigned int hex_value;
+ std::stringstream hex_ss;
+ hex_ss << std::hex << match[2];
+ hex_ss >> hex_value;
+ if (hex_value & android::ResTable_entry::FLAG_USES_FEATURE_FLAGS) {
+ fields_flagged++;
+ if (first_line && match[1] == "layout1") {
+ found = true;
+ }
+ }
+ }
+ }
+ ASSERT_TRUE(found) << "No entry for layout1 at v36 with FLAG_USES_FEATURE_FLAGS bit set";
+ // There should only be 1 entry that has the FLAG_USES_FEATURE_FLAGS bit of flags set to 1
+ ASSERT_EQ(fields_flagged, 1);
+}
+
} // namespace aapt
diff --git a/tools/aapt2/link/FlaggedXmlVersioner.cpp b/tools/aapt2/link/FlaggedXmlVersioner.cpp
index 75c6f17..8a3337c 100644
--- a/tools/aapt2/link/FlaggedXmlVersioner.cpp
+++ b/tools/aapt2/link/FlaggedXmlVersioner.cpp
@@ -66,6 +66,28 @@
bool had_flags_ = false;
};
+// An xml visitor that goes through the a doc and determines if any elements are behind a flag.
+class FindFlagsVisitor : public xml::Visitor {
+ public:
+ void Visit(xml::Element* node) override {
+ if (had_flags_) {
+ return;
+ }
+ auto* attr = node->FindAttribute(xml::kSchemaAndroid, xml::kAttrFeatureFlag);
+ if (attr != nullptr) {
+ had_flags_ = true;
+ return;
+ }
+ VisitChildren(node);
+ }
+
+ bool HadFlags() const {
+ return had_flags_;
+ }
+
+ bool had_flags_ = false;
+};
+
std::vector<std::unique_ptr<xml::XmlResource>> FlaggedXmlVersioner::Process(IAaptContext* context,
xml::XmlResource* doc) {
std::vector<std::unique_ptr<xml::XmlResource>> docs;
@@ -74,15 +96,20 @@
// Support for read/write flags was added in baklava so if the doc will only get used on
// baklava or later we can just return the original doc.
docs.push_back(doc->Clone());
+ FindFlagsVisitor visitor;
+ doc->root->Accept(&visitor);
+ docs.back()->file.uses_readwrite_feature_flags = visitor.HadFlags();
} else {
auto preBaklavaVersion = doc->Clone();
AllDisabledFlagsVisitor visitor;
preBaklavaVersion->root->Accept(&visitor);
+ preBaklavaVersion->file.uses_readwrite_feature_flags = false;
docs.push_back(std::move(preBaklavaVersion));
if (visitor.HadFlags()) {
auto baklavaVersion = doc->Clone();
baklavaVersion->file.config.sdkVersion = SDK_BAKLAVA;
+ baklavaVersion->file.uses_readwrite_feature_flags = true;
docs.push_back(std::move(baklavaVersion));
}
}