Flag support for bag resource types
Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I891c93c3ffcab172d28701b44a80c50f1e24d99e
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index c1e3578..471b402 100644
--- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
+++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
@@ -68,6 +68,16 @@
assertThat(getBoolean("res3")).isTrue();
}
+ @Test
+ public void testFlagDisabledStringArrayElement() {
+ assertThat(getStringArray("strarr1")).isEqualTo(new String[]{"one", "two", "three"});
+ }
+
+ @Test
+ public void testFlagDisabledIntArrayElement() {
+ assertThat(getIntArray("intarr1")).isEqualTo(new int[]{1, 2, 3});
+ }
+
private boolean getBoolean(String name) {
int resId = mResources.getIdentifier(
name,
@@ -77,13 +87,22 @@
return mResources.getBoolean(resId);
}
- private String getString(String name) {
+ private String[] getStringArray(String name) {
int resId = mResources.getIdentifier(
name,
- "string",
+ "array",
"com.android.intenal.flaggedresources");
assertThat(resId).isNotEqualTo(0);
- return mResources.getString(resId);
+ return mResources.getStringArray(resId);
+ }
+
+ private int[] getIntArray(String name) {
+ int resId = mResources.getIdentifier(
+ name,
+ "array",
+ "com.android.intenal.flaggedresources");
+ assertThat(resId).isNotEqualTo(0);
+ return mResources.getIntArray(resId);
}
private String extractApkAndGetPath(int id) throws Exception {
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 1c85e9f..a5aecc8 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -151,6 +151,7 @@
}
if (res->value != nullptr) {
+ res->value->SetFlagStatus(res->flag_status);
// Attach the comment, source and config to the value.
res->value->SetComment(std::move(res->comment));
res->value->SetSource(std::move(res->source));
@@ -546,30 +547,11 @@
});
std::string resource_type = parser->element_name();
- std::optional<StringPiece> flag =
- xml::FindAttribute(parser, "http://schemas.android.com/apk/res/android", "featureFlag");
- out_resource->flag_status = FlagStatus::NoFlag;
- if (flag) {
- auto flag_it = options_.feature_flag_values.find(flag.value());
- if (flag_it == options_.feature_flag_values.end()) {
- diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
- << "Resource flag value undefined");
- return false;
- }
- const auto& flag_properties = flag_it->second;
- if (!flag_properties.read_only) {
- diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
- << "Only read only flags may be used with resources");
- return false;
- }
- if (!flag_properties.enabled.has_value()) {
- diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
- << "Only flags with a value may be used with resources");
- return false;
- }
- out_resource->flag_status =
- flag_properties.enabled.value() ? FlagStatus::Enabled : FlagStatus::Disabled;
+ auto flag_status = GetFlagStatus(parser);
+ if (!flag_status) {
+ return false;
}
+ out_resource->flag_status = flag_status.value();
// The value format accepted for this resource.
uint32_t resource_format = 0u;
@@ -751,6 +733,33 @@
return false;
}
+std::optional<FlagStatus> ResourceParser::GetFlagStatus(xml::XmlPullParser* parser) {
+ auto flag_status = FlagStatus::NoFlag;
+
+ std::optional<StringPiece> flag = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag");
+ if (flag) {
+ auto flag_it = options_.feature_flag_values.find(flag.value());
+ if (flag_it == options_.feature_flag_values.end()) {
+ diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
+ << "Resource flag value undefined");
+ return {};
+ }
+ const auto& flag_properties = flag_it->second;
+ if (!flag_properties.read_only) {
+ diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
+ << "Only read only flags may be used with resources");
+ return {};
+ }
+ if (!flag_properties.enabled.has_value()) {
+ diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))
+ << "Only flags with a value may be used with resources");
+ return {};
+ }
+ flag_status = flag_properties.enabled.value() ? FlagStatus::Enabled : FlagStatus::Disabled;
+ }
+ return flag_status;
+}
+
bool ResourceParser::ParseItem(xml::XmlPullParser* parser,
ParsedResource* out_resource,
const uint32_t format) {
@@ -1657,12 +1666,18 @@
const std::string& element_namespace = parser->element_namespace();
const std::string& element_name = parser->element_name();
if (element_namespace.empty() && element_name == "item") {
+ auto flag_status = GetFlagStatus(parser);
+ if (!flag_status) {
+ error = true;
+ continue;
+ }
std::unique_ptr<Item> item = ParseXml(parser, typeMask, kNoRawString);
if (!item) {
diag_->Error(android::DiagMessage(item_source) << "could not parse array item");
error = true;
continue;
}
+ item->SetFlagStatus(flag_status.value());
item->SetSource(item_source);
array->elements.emplace_back(std::move(item));
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 45d41c1..442dea8 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -85,6 +85,8 @@
private:
DISALLOW_COPY_AND_ASSIGN(ResourceParser);
+ std::optional<FlagStatus> GetFlagStatus(xml::XmlPullParser* parser);
+
std::optional<FlattenedXmlSubTree> CreateFlattenSubTree(xml::XmlPullParser* parser);
// Parses the XML subtree as a StyleString (flattened XML representation for strings with
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 1cdb715..7a4f40e 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -605,12 +605,12 @@
if (!config_value->value) {
// Resource does not exist, add it now.
config_value->value = std::move(res.value);
- config_value->flag_status = res.flag_status;
} else {
// When validation is enabled, ensure that a resource cannot have multiple values defined for
// the same configuration unless protected by flags.
- auto result = validate ? ResolveFlagCollision(config_value->flag_status, res.flag_status)
- : CollisionResult::kKeepBoth;
+ auto result =
+ validate ? ResolveFlagCollision(config_value->value->GetFlagStatus(), res.flag_status)
+ : CollisionResult::kKeepBoth;
if (result == CollisionResult::kConflict) {
result = ResolveValueCollision(config_value->value.get(), res.value.get());
}
@@ -619,7 +619,6 @@
// Insert the value ignoring for duplicate configurations
entry->values.push_back(util::make_unique<ResourceConfigValue>(res.config, res.product));
entry->values.back()->value = std::move(res.value);
- entry->values.back()->flag_status = res.flag_status;
break;
case CollisionResult::kTakeNew:
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index 4f76e7d..cba6b70 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -104,8 +104,6 @@
// The actual Value.
std::unique_ptr<Value> value;
- FlagStatus flag_status = FlagStatus::NoFlag;
-
ResourceConfigValue(const android::ConfigDescription& config, android::StringPiece product)
: config(config), product(product) {
}
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 166b01b..b75e87c 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -971,6 +971,16 @@
*out << "(array) [" << util::Joiner(elements, ", ") << "]";
}
+void Array::RemoveFlagDisabledElements() {
+ const auto end_iter = elements.end();
+ const auto remove_iter = std::stable_partition(
+ elements.begin(), end_iter, [](const std::unique_ptr<Item>& item) -> bool {
+ return item->GetFlagStatus() != FlagStatus::Disabled;
+ });
+
+ elements.erase(remove_iter, end_iter);
+}
+
bool Plural::Equals(const Value* value) const {
const Plural* other = ValueCast<Plural>(value);
if (!other) {
@@ -1092,6 +1102,7 @@
std::unique_ptr<T> CopyValueFields(std::unique_ptr<T> new_value, const T* value) {
new_value->SetSource(value->GetSource());
new_value->SetComment(value->GetComment());
+ new_value->SetFlagStatus(value->GetFlagStatus());
return new_value;
}
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 5192c2b..a1b1839 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -65,6 +65,14 @@
return translatable_;
}
+ void SetFlagStatus(FlagStatus val) {
+ flag_status_ = val;
+ }
+
+ FlagStatus GetFlagStatus() const {
+ return flag_status_;
+ }
+
// Returns the source where this value was defined.
const android::Source& GetSource() const {
return source_;
@@ -109,6 +117,10 @@
// of brevity and readability. Default implementation just calls Print().
virtual void PrettyPrint(text::Printer* printer) const;
+ // Removes any part of the value that is beind a disabled flag.
+ virtual void RemoveFlagDisabledElements() {
+ }
+
friend std::ostream& operator<<(std::ostream& out, const Value& value);
protected:
@@ -116,6 +128,7 @@
std::string comment_;
bool weak_ = false;
bool translatable_ = true;
+ FlagStatus flag_status_ = FlagStatus::NoFlag;
private:
virtual Value* TransformValueImpl(ValueTransformer& transformer) const = 0;
@@ -346,6 +359,7 @@
bool Equals(const Value* value) const override;
void Print(std::ostream* out) const override;
+ void RemoveFlagDisabledElements() override;
};
struct Plural : public TransformableValue<Plural, BaseValue<Plural>> {
diff --git a/tools/aapt2/Resources.proto b/tools/aapt2/Resources.proto
index 2ecc82a..5c64089 100644
--- a/tools/aapt2/Resources.proto
+++ b/tools/aapt2/Resources.proto
@@ -246,7 +246,7 @@
message ConfigValue {
Configuration config = 1;
Value value = 2;
- uint32 flag_status = 3;
+ reserved 3;
}
// The generic meta-data for every value in a resource table.
@@ -280,6 +280,9 @@
Id id = 6;
Primitive prim = 7;
}
+
+ // The status of the flag the value is behind if any
+ uint32 flag_status = 8;
}
// A CompoundValue is an abstract type. It represents a value that is a made of other values.
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 56f5288..be63f82 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1878,7 +1878,7 @@
for (auto& type : package->types) {
for (auto& entry : type->entries) {
for (auto& config_value : entry->values) {
- if (config_value->flag_status == FlagStatus::Disabled) {
+ if (config_value->value->GetFlagStatus() == FlagStatus::Disabled) {
config_value->value->Accept(&visitor);
}
}
diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp
index aaab315..55f5e56 100644
--- a/tools/aapt2/format/proto/ProtoDeserialize.cpp
+++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp
@@ -534,8 +534,6 @@
return false;
}
- config_value->flag_status = (FlagStatus)pb_config_value.flag_status();
-
config_value->value = DeserializeValueFromPb(pb_config_value.value(), src_pool, config,
&out_table->string_pool, files, out_error);
if (config_value->value == nullptr) {
@@ -877,11 +875,12 @@
return value;
}
-std::unique_ptr<Item> DeserializeItemFromPb(const pb::Item& pb_item,
- const android::ResStringPool& src_pool,
- const ConfigDescription& config,
- android::StringPool* value_pool,
- io::IFileCollection* files, std::string* out_error) {
+std::unique_ptr<Item> DeserializeItemFromPbInternal(const pb::Item& pb_item,
+ const android::ResStringPool& src_pool,
+ const ConfigDescription& config,
+ android::StringPool* value_pool,
+ io::IFileCollection* files,
+ std::string* out_error) {
switch (pb_item.value_case()) {
case pb::Item::kRef: {
const pb::Reference& pb_ref = pb_item.ref();
@@ -1010,6 +1009,19 @@
return {};
}
+std::unique_ptr<Item> DeserializeItemFromPb(const pb::Item& pb_item,
+ const android::ResStringPool& src_pool,
+ const ConfigDescription& config,
+ android::StringPool* value_pool,
+ io::IFileCollection* files, std::string* out_error) {
+ auto item =
+ DeserializeItemFromPbInternal(pb_item, src_pool, config, value_pool, files, out_error);
+ if (item) {
+ item->SetFlagStatus((FlagStatus)pb_item.flag_status());
+ }
+ return item;
+}
+
std::unique_ptr<xml::XmlResource> DeserializeXmlResourceFromPb(const pb::XmlNode& pb_node,
std::string* out_error) {
if (!pb_node.has_element()) {
diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp
index c1e15bc..5772b3b 100644
--- a/tools/aapt2/format/proto/ProtoSerialize.cpp
+++ b/tools/aapt2/format/proto/ProtoSerialize.cpp
@@ -426,7 +426,6 @@
pb_config_value->mutable_config()->set_product(config_value->product);
SerializeValueToPb(*config_value->value, pb_config_value->mutable_value(),
source_pool.get());
- pb_config_value->set_flag_status((uint32_t)config_value->flag_status);
}
}
}
@@ -720,6 +719,9 @@
if (src_pool != nullptr) {
SerializeSourceToPb(value.GetSource(), src_pool, out_value->mutable_source());
}
+ if (out_value->has_item()) {
+ out_value->mutable_item()->set_flag_status((uint32_t)value.GetFlagStatus());
+ }
}
void SerializeItemToPb(const Item& item, pb::Item* out_item) {
@@ -727,6 +729,7 @@
ValueSerializer serializer(&value, nullptr);
item.Accept(&serializer);
out_item->MergeFrom(value.item());
+ out_item->set_flag_status((uint32_t)item.GetFlagStatus());
}
void SerializeCompiledFileToPb(const ResourceFile& file, pb::internal::CompiledFile* out_file) {
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
index 5932271..4866d2c 100644
--- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
@@ -28,11 +28,13 @@
srcs: [
"res/values/bools.xml",
"res/values/bools2.xml",
+ "res/values/ints.xml",
"res/values/strings.xml",
],
out: [
"values_bools.arsc.flat",
"values_bools2.arsc.flat",
+ "values_ints.arsc.flat",
"values_strings.arsc.flat",
],
cmd: "$(location aapt2) compile $(in) -o $(genDir) " +
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/ints.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/ints.xml
new file mode 100644
index 0000000..26a5c40
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/ints.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+ <integer-array name="intarr1">
+ <item>1</item>
+ <item>2</item>
+ <item android:featureFlag="test.package.falseFlag">666</item>
+ <item>3</item>
+ </integer-array>
+</resources>
\ No newline at end of file
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml
index 5c0fca1..3cbb928 100644
--- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml
@@ -3,4 +3,11 @@
<string name="str">plain string</string>
<string name="str1" android:featureFlag="test.package.falseFlag">DONTFIND</string>
+
+ <string-array name="strarr1">
+ <item>one</item>
+ <item>two</item>
+ <item android:featureFlag="test.package.falseFlag">remove</item>
+ <item android:featureFlag="test.package.trueFlag">three</item>
+ </string-array>
</resources>
\ No newline at end of file
diff --git a/tools/aapt2/link/FlagDisabledResourceRemover.cpp b/tools/aapt2/link/FlagDisabledResourceRemover.cpp
index e3289e2..3ac1762 100644
--- a/tools/aapt2/link/FlagDisabledResourceRemover.cpp
+++ b/tools/aapt2/link/FlagDisabledResourceRemover.cpp
@@ -32,12 +32,17 @@
const auto remove_iter =
std::stable_partition(entry->values.begin(), end_iter,
[](const std::unique_ptr<ResourceConfigValue>& value) -> bool {
- return value->flag_status != FlagStatus::Disabled;
+ return value->value->GetFlagStatus() != FlagStatus::Disabled;
});
bool keep = remove_iter != entry->values.begin();
entry->values.erase(remove_iter, end_iter);
+
+ for (auto& value : entry->values) {
+ value->value->RemoveFlagDisabledElements();
+ }
+
return keep;
}
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 1942fc11..37a039e 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -212,8 +212,8 @@
collision_result =
ResolveMergeCollision(override_styles_instead_of_overlaying, dst_value, src_value, pool);
} else {
- collision_result = ResourceTable::ResolveFlagCollision(dst_config_value->flag_status,
- src_config_value->flag_status);
+ collision_result =
+ ResourceTable::ResolveFlagCollision(dst_value->GetFlagStatus(), src_value->GetFlagStatus());
if (collision_result == CollisionResult::kConflict) {
collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
}
@@ -295,7 +295,6 @@
} else {
dst_config_value =
dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product);
- dst_config_value->flag_status = src_config_value->flag_status;
}
// Continue if we're taking the new resource.