diff options
-rw-r--r-- | core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java | 25 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.cpp | 61 | ||||
-rw-r--r-- | tools/aapt2/ResourceParser.h | 2 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.cpp | 7 | ||||
-rw-r--r-- | tools/aapt2/ResourceTable.h | 2 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.cpp | 11 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.h | 14 | ||||
-rw-r--r-- | tools/aapt2/Resources.proto | 5 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/format/proto/ProtoDeserialize.cpp | 26 | ||||
-rw-r--r-- | tools/aapt2/format/proto/ProtoSerialize.cpp | 5 | ||||
-rw-r--r-- | tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp | 2 | ||||
-rw-r--r-- | tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/ints.xml | 9 | ||||
-rw-r--r-- | tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml | 7 | ||||
-rw-r--r-- | tools/aapt2/link/FlagDisabledResourceRemover.cpp | 7 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger.cpp | 5 |
16 files changed, 144 insertions, 46 deletions
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java index c1e357864fff..471b4021545a 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 @@ public class ResourceFlaggingTest { 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 @@ public class ResourceFlaggingTest { return mResources.getBoolean(resId); } - private String getString(String name) { + private String[] getStringArray(String name) { + int resId = mResources.getIdentifier( + name, + "array", + "com.android.intenal.flaggedresources"); + assertThat(resId).isNotEqualTo(0); + return mResources.getStringArray(resId); + } + + private int[] getIntArray(String name) { int resId = mResources.getIdentifier( name, - "string", + "array", "com.android.intenal.flaggedresources"); assertThat(resId).isNotEqualTo(0); - return mResources.getString(resId); + return mResources.getIntArray(resId); } private String extractApkAndGetPath(int id) throws Exception { diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 1c85e9ff231b..a5aecc855707 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -151,6 +151,7 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia } 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 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, }); 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 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, 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 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser, 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 45d41c193cb4..442dea89ef40 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -85,6 +85,8 @@ class ResourceParser { 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 1cdb71551d5d..7a4f40e471d2 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -605,12 +605,12 @@ bool ResourceTable::AddResource(NewResource&& res, android::IDiagnostics* diag) 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 @@ bool ResourceTable::AddResource(NewResource&& res, android::IDiagnostics* diag) // 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 4f76e7d3a2b7..cba6b70cfbd6 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -104,8 +104,6 @@ class ResourceConfigValue { // 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 166b01bd9154..b75e87c90128 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -971,6 +971,16 @@ void Array::Print(std::ostream* out) const { *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 @@ template <typename T> 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 5192c2be1f98..a1b1839b19ef 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -65,6 +65,14 @@ class Value { 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 @@ class Value { // 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 @@ class Value { 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 @@ struct Array : public TransformableValue<Array, BaseValue<Array>> { 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 2ecc82ae4792..5c6408940b34 100644 --- a/tools/aapt2/Resources.proto +++ b/tools/aapt2/Resources.proto @@ -246,7 +246,7 @@ message Entry { 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 @@ message Item { 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 56f52885b36d..be63f82b30cf 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -1878,7 +1878,7 @@ class Linker { 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 aaab3158f61e..55f5e5668a16 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -534,8 +534,6 @@ static bool DeserializePackageFromPb(const pb::Package& pb_package, const ResStr 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 @@ std::unique_ptr<Value> DeserializeValueFromPb(const pb::Value& pb_value, 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 @@ std::unique_ptr<Item> DeserializeItemFromPb(const pb::Item& pb_item, 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 c1e15bcf9f70..5772b3b0b3e6 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -426,7 +426,6 @@ void SerializeTableToPb(const ResourceTable& table, pb::ResourceTable* out_table 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 @@ void SerializeValueToPb(const Value& value, pb::Value* out_value, android::Strin 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 @@ void SerializeItemToPb(const Item& item, pb::Item* out_item) { 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 5932271d4d28..4866d2c83c71 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp @@ -28,11 +28,13 @@ genrule { 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 000000000000..26a5c40bb338 --- /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 5c0fca16fe39..3cbb928a64cc 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 e3289e2a173a..3ac17625023e 100644 --- a/tools/aapt2/link/FlagDisabledResourceRemover.cpp +++ b/tools/aapt2/link/FlagDisabledResourceRemover.cpp @@ -32,12 +32,17 @@ static bool KeepResourceEntry(const std::unique_ptr<ResourceEntry>& entry) { 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 1942fc11c32e..37a039e9528f 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -212,8 +212,8 @@ static ResourceTable::CollisionResult MergeConfigValue( 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 @@ bool TableMerger::DoMerge(const android::Source& src, ResourceTablePackage* src_ } 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. |