From 211bec2871da597f9f3fd81df7faffea1754437e Mon Sep 17 00:00:00 2001
From: Jeremy Meyer <jakmcbane@google.com>
Date: Tue, 4 Jun 2024 14:22:03 -0700
Subject: First pass at flagged resources

This gets the main parts of resource flagging in place and the basic use
case of flagging with an xml attribute working.

Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: Id2b5ba450d05da00a922e98ca204b6e5aa6c6c24
---
 tools/aapt2/ResourceParser.cpp | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

(limited to 'tools/aapt2/ResourceParser.cpp')

diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 6af39b739e9b..2df941834063 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 #include "ResourceParser.h"
 
 #include <functional>
@@ -108,6 +107,7 @@ struct ParsedResource {
   Visibility::Level visibility_level = Visibility::Level::kUndefined;
   bool staged_api = false;
   bool allow_new = false;
+  FlagStatus flag_status;
   std::optional<OverlayableItem> overlayable_item;
   std::optional<StagedId> staged_alias;
 
@@ -161,6 +161,8 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia
     res_builder.SetStagedId(res->staged_alias.value());
   }
 
+  res_builder.SetFlagStatus(res->flag_status);
+
   bool error = false;
   if (!res->name.entry.empty()) {
     if (!table->AddResource(res_builder.Build(), diag)) {
@@ -544,6 +546,30 @@ 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;
+  }
 
   // The value format accepted for this resource.
   uint32_t resource_format = 0u;
-- 
cgit v1.2.3-59-g8ed1b


From efa42b1e27efc2ff8cb39cf47141f9d59ca6d375 Mon Sep 17 00:00:00 2001
From: Jeremy Meyer <jakmcbane@google.com>
Date: Wed, 17 Jul 2024 14:57:33 -0700
Subject: Dont parse resource values behind disabled flags

This change makes it so that when we are parsing resources and there is
one behind a disabled flag we don't parse the value. This is primarily
to prevent disabled strings from ending up in the string pool.

Test: automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I4d8683ac820a3fd20cd92c223464f912b0ac422d
---
 tools/aapt2/ResourceParser.cpp      |  4 +++-
 tools/aapt2/ResourceParser_test.cpp | 22 ++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

(limited to 'tools/aapt2/ResourceParser.cpp')

diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 2df941834063..45bf8e38c5ce 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -690,7 +690,9 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser,
         resource_format = item_iter->second.format;
       }
 
-      if (!ParseItem(parser, out_resource, resource_format)) {
+      // Don't bother parsing the item if it is behind a disabled flag
+      if (out_resource->flag_status != FlagStatus::Disabled &&
+          !ParseItem(parser, out_resource, resource_format)) {
         return false;
       }
       return true;
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index b59b16574c42..2e6ad13d99de 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -69,8 +69,13 @@ class ResourceParserTest : public ::testing::Test {
     return TestParse(str, ConfigDescription{});
   }
 
-  ::testing::AssertionResult TestParse(StringPiece str, const ConfigDescription& config) {
-    ResourceParserOptions parserOptions;
+  ::testing::AssertionResult TestParse(StringPiece str, ResourceParserOptions parserOptions) {
+    return TestParse(str, ConfigDescription{}, parserOptions);
+  }
+
+  ::testing::AssertionResult TestParse(
+      StringPiece str, const ConfigDescription& config,
+      ResourceParserOptions parserOptions = ResourceParserOptions()) {
     ResourceParser parser(context_->GetDiagnostics(), &table_, android::Source{"test"}, config,
                           parserOptions);
 
@@ -242,6 +247,19 @@ TEST_F(ResourceParserTest, ParseStringTranslatableAttribute) {
   EXPECT_FALSE(TestParse(R"(<string name="foo4" translatable="yes">Translate</string>)"));
 }
 
+TEST_F(ResourceParserTest, ParseStringBehindDisabledFlag) {
+  FeatureFlagProperties flag_properties(true, false);
+  ResourceParserOptions options;
+  options.feature_flag_values = {{"falseFlag", flag_properties}};
+  ASSERT_TRUE(TestParse(
+      R"(<string name="foo" android:featureFlag="falseFlag"
+              xmlns:android="http://schemas.android.com/apk/res/android">foo</string>)",
+      options));
+
+  String* str = test::GetValue<String>(&table_, "string/foo");
+  ASSERT_THAT(str, IsNull());
+}
+
 TEST_F(ResourceParserTest, IgnoreXliffTagsOtherThanG) {
   std::string input = R"(
       <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
-- 
cgit v1.2.3-59-g8ed1b


From d7f86e5fa714ffe0a3b6984724000100ad2150f2 Mon Sep 17 00:00:00 2001
From: Jeremy Meyer <jakmcbane@google.com>
Date: Wed, 24 Jul 2024 16:00:44 -0700
Subject: Consider flags when mergine resource tables

Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I372e3005c0d44ba2e7f3805a43bfc5b0a1bc1200
---
 core/tests/resourceflaggingtests/Android.bp                  | 12 ++++++++++++
 .../flagged_resources_res/values/bools.xml                   |  2 ++
 .../flagged_resources_res/values/bools2.xml                  |  4 ++++
 .../android/resourceflaggingtests/ResourceFlaggingTest.java  |  5 +++++
 tools/aapt2/ResourceParser.cpp                               |  2 +-
 tools/aapt2/ResourceTable.h                                  |  4 ++--
 tools/aapt2/link/TableMerger.cpp                             |  7 ++++++-
 7 files changed, 32 insertions(+), 4 deletions(-)
 create mode 100644 core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml

(limited to 'tools/aapt2/ResourceParser.cpp')

diff --git a/core/tests/resourceflaggingtests/Android.bp b/core/tests/resourceflaggingtests/Android.bp
index e8bb71010006..dd86094127da 100644
--- a/core/tests/resourceflaggingtests/Android.bp
+++ b/core/tests/resourceflaggingtests/Android.bp
@@ -33,6 +33,17 @@ genrule {
         "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true",
 }
 
+genrule {
+    name: "resource-flagging-test-app-resources-compile2",
+    tools: ["aapt2"],
+    srcs: [
+        "flagged_resources_res/values/bools2.xml",
+    ],
+    out: ["values_bools2.arsc.flat"],
+    cmd: "$(location aapt2) compile $(in) -o $(genDir) " +
+        "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true",
+}
+
 genrule {
     name: "resource-flagging-test-app-apk",
     tools: ["aapt2"],
@@ -40,6 +51,7 @@ genrule {
     srcs: [
         "TestAppAndroidManifest.xml",
         ":resource-flagging-test-app-resources-compile",
+        ":resource-flagging-test-app-resources-compile2",
     ],
     out: ["resapp.apk"],
     cmd: "$(location aapt2) link -o $(out) --manifest $(in)",
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
index f4defd94831c..8d0146511d1d 100644
--- a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
+++ b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
@@ -5,4 +5,6 @@
 
     <bool name="res2">false</bool>
     <bool name="res2" android:featureFlag="test.package.trueFlag">true</bool>
+
+    <bool name="res3">false</bool>
 </resources>
\ No newline at end of file
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
new file mode 100644
index 000000000000..e7563aa0fbdd
--- /dev/null
+++ b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+    <bool name="res3" android:featureFlag="test.package.trueFlag">true</bool>
+</resources>
\ No newline at end of file
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index a0cbe3cf05c3..ad8542e0f6b3 100644
--- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
+++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
@@ -63,6 +63,11 @@ public class ResourceFlaggingTest {
         assertThat(getBoolean("res2")).isTrue();
     }
 
+    @Test
+    public void testFlagEnabledDifferentCompilationUnit() {
+        assertThat(getBoolean("res3")).isTrue();
+    }
+
     private boolean getBoolean(String name) {
         int resId = mResources.getIdentifier(name, "bool", "com.android.intenal.flaggedresources");
         assertThat(resId).isNotEqualTo(0);
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 45bf8e38c5ce..9444dd968f5f 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -107,7 +107,7 @@ struct ParsedResource {
   Visibility::Level visibility_level = Visibility::Level::kUndefined;
   bool staged_api = false;
   bool allow_new = false;
-  FlagStatus flag_status;
+  FlagStatus flag_status = FlagStatus::NoFlag;
   std::optional<OverlayableItem> overlayable_item;
   std::optional<StagedId> staged_alias;
 
diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h
index 9530c1750c79..4f76e7d3a2b7 100644
--- a/tools/aapt2/ResourceTable.h
+++ b/tools/aapt2/ResourceTable.h
@@ -104,7 +104,7 @@ class ResourceConfigValue {
   // The actual Value.
   std::unique_ptr<Value> value;
 
-  FlagStatus flag_status;
+  FlagStatus flag_status = FlagStatus::NoFlag;
 
   ResourceConfigValue(const android::ConfigDescription& config, android::StringPiece product)
       : config(config), product(product) {
@@ -271,7 +271,7 @@ struct NewResource {
   std::optional<AllowNew> allow_new;
   std::optional<StagedId> staged_id;
   bool allow_mangled = false;
-  FlagStatus flag_status;
+  FlagStatus flag_status = FlagStatus::NoFlag;
 };
 
 struct NewResourceBuilder {
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 67a48283e8b6..1942fc11c32e 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/tools/aapt2/link/TableMerger.cpp
@@ -212,7 +212,11 @@ static ResourceTable::CollisionResult MergeConfigValue(
     collision_result =
         ResolveMergeCollision(override_styles_instead_of_overlaying, dst_value, src_value, pool);
   } else {
-    collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
+    collision_result = ResourceTable::ResolveFlagCollision(dst_config_value->flag_status,
+                                                           src_config_value->flag_status);
+    if (collision_result == CollisionResult::kConflict) {
+      collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
+    }
   }
 
   if (collision_result == CollisionResult::kConflict) {
@@ -291,6 +295,7 @@ 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.
-- 
cgit v1.2.3-59-g8ed1b


From 16c83afa9b790d3fde434232771f6cb089291c44 Mon Sep 17 00:00:00 2001
From: Jeremy Meyer <jakmcbane@google.com>
Date: Fri, 26 Jul 2024 16:21:49 -0700
Subject: Remove flag disabled strings from string pool

Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools

Change-Id: I627feff5774f44a398a8337733498ede601d07a4
---
 core/tests/resourceflaggingtests/Android.bp        | 50 +----------------
 .../TestAppAndroidManifest.xml                     |  4 --
 .../flagged_resources_res/values/bools.xml         | 10 ----
 .../flagged_resources_res/values/bools2.xml        |  4 --
 .../ResourceFlaggingTest.java                      | 19 ++++++-
 tools/aapt2/Android.bp                             |  1 +
 tools/aapt2/ResourceParser.cpp                     |  4 +-
 tools/aapt2/ResourceParser_test.cpp                | 22 +-------
 tools/aapt2/cmd/Link.cpp                           | 40 +++++++++++++
 .../FlaggedResourcesTest/Android.bp                | 65 ++++++++++++++++++++++
 .../FlaggedResourcesTest/AndroidManifest.xml       |  4 ++
 .../FlaggedResourcesTest/res/values/bools.xml      | 10 ++++
 .../FlaggedResourcesTest/res/values/bools2.xml     |  4 ++
 .../FlaggedResourcesTest/res/values/strings.xml    |  6 ++
 tools/aapt2/link/FlaggedResources_test.cpp         | 54 ++++++++++++++++++
 15 files changed, 206 insertions(+), 91 deletions(-)
 delete mode 100644 core/tests/resourceflaggingtests/TestAppAndroidManifest.xml
 delete mode 100644 core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
 delete mode 100644 core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml
 create mode 100644 tools/aapt2/link/FlaggedResources_test.cpp

(limited to 'tools/aapt2/ResourceParser.cpp')

diff --git a/core/tests/resourceflaggingtests/Android.bp b/core/tests/resourceflaggingtests/Android.bp
index dd86094127da..efb843735aef 100644
--- a/core/tests/resourceflaggingtests/Android.bp
+++ b/core/tests/resourceflaggingtests/Android.bp
@@ -22,54 +22,6 @@ package {
     default_team: "trendy_team_android_resources",
 }
 
-genrule {
-    name: "resource-flagging-test-app-resources-compile",
-    tools: ["aapt2"],
-    srcs: [
-        "flagged_resources_res/values/bools.xml",
-    ],
-    out: ["values_bools.arsc.flat"],
-    cmd: "$(location aapt2) compile $(in) -o $(genDir) " +
-        "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true",
-}
-
-genrule {
-    name: "resource-flagging-test-app-resources-compile2",
-    tools: ["aapt2"],
-    srcs: [
-        "flagged_resources_res/values/bools2.xml",
-    ],
-    out: ["values_bools2.arsc.flat"],
-    cmd: "$(location aapt2) compile $(in) -o $(genDir) " +
-        "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true",
-}
-
-genrule {
-    name: "resource-flagging-test-app-apk",
-    tools: ["aapt2"],
-    // The first input file in the list must be the manifest
-    srcs: [
-        "TestAppAndroidManifest.xml",
-        ":resource-flagging-test-app-resources-compile",
-        ":resource-flagging-test-app-resources-compile2",
-    ],
-    out: ["resapp.apk"],
-    cmd: "$(location aapt2) link -o $(out) --manifest $(in)",
-}
-
-java_genrule {
-    name: "resource-flagging-apk-as-resource",
-    srcs: [
-        ":resource-flagging-test-app-apk",
-    ],
-    out: ["apks_as_resources.res.zip"],
-    tools: ["soong_zip"],
-
-    cmd: "mkdir -p $(genDir)/res/raw && " +
-        "cp $(in) $(genDir)/res/raw/$$(basename $(in)) && " +
-        "$(location soong_zip) -o $(out) -C $(genDir)/res -D $(genDir)/res",
-}
-
 android_test {
     name: "ResourceFlaggingTests",
     srcs: [
@@ -82,6 +34,6 @@ android_test {
         "testng",
         "compatibility-device-util-axt",
     ],
-    resource_zips: [":resource-flagging-apk-as-resource"],
+    resource_zips: [":resource-flagging-test-app-apk-as-resource"],
     test_suites: ["device-tests"],
 }
diff --git a/core/tests/resourceflaggingtests/TestAppAndroidManifest.xml b/core/tests/resourceflaggingtests/TestAppAndroidManifest.xml
deleted file mode 100644
index d6cdeb7b5231..000000000000
--- a/core/tests/resourceflaggingtests/TestAppAndroidManifest.xml
+++ /dev/null
@@ -1,4 +0,0 @@
-<manifest xmlns:android="http://schemas.android.com/apk/res/android"
-    package="com.android.intenal.flaggedresources">
-    <application/>
-</manifest>
\ No newline at end of file
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
deleted file mode 100644
index 8d0146511d1d..000000000000
--- a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
+++ /dev/null
@@ -1,10 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<resources xmlns:android="http://schemas.android.com/apk/res/android">
-    <bool name="res1">true</bool>
-    <bool name="res1" android:featureFlag="test.package.falseFlag">false</bool>
-
-    <bool name="res2">false</bool>
-    <bool name="res2" android:featureFlag="test.package.trueFlag">true</bool>
-
-    <bool name="res3">false</bool>
-</resources>
\ No newline at end of file
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml b/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
deleted file mode 100644
index e7563aa0fbdd..000000000000
--- a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
+++ /dev/null
@@ -1,4 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<resources xmlns:android="http://schemas.android.com/apk/res/android">
-    <bool name="res3" android:featureFlag="test.package.trueFlag">true</bool>
-</resources>
\ No newline at end of file
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index ad8542e0f6b3..a3d8d3dbf31a 100644
--- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
+++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
@@ -68,12 +68,29 @@ public class ResourceFlaggingTest {
         assertThat(getBoolean("res3")).isTrue();
     }
 
+    @Test
+    public void testFlagDisabledNoValue() {
+        assertThat(getString("str1")).isEqualTo("");
+    }
+
     private boolean getBoolean(String name) {
-        int resId = mResources.getIdentifier(name, "bool", "com.android.intenal.flaggedresources");
+        int resId = mResources.getIdentifier(
+                name,
+                "bool",
+                "com.android.intenal.flaggedresources");
         assertThat(resId).isNotEqualTo(0);
         return mResources.getBoolean(resId);
     }
 
+    private String getString(String name) {
+        int resId = mResources.getIdentifier(
+                name,
+                "string",
+                "com.android.intenal.flaggedresources");
+        assertThat(resId).isNotEqualTo(0);
+        return mResources.getString(resId);
+    }
+
     private String extractApkAndGetPath(int id) throws Exception {
         final Resources resources = mContext.getResources();
         try (InputStream is = resources.openRawResource(id)) {
diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index 3f9016ba4852..7b227cebe2ae 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -189,6 +189,7 @@ cc_test_host {
         "integration-tests/CommandTests/**/*",
         "integration-tests/ConvertTest/**/*",
         "integration-tests/DumpTest/**/*",
+        ":resource-flagging-test-app-apk",
     ],
 }
 
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 9444dd968f5f..1c85e9ff231b 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -690,9 +690,7 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser,
         resource_format = item_iter->second.format;
       }
 
-      // Don't bother parsing the item if it is behind a disabled flag
-      if (out_resource->flag_status != FlagStatus::Disabled &&
-          !ParseItem(parser, out_resource, resource_format)) {
+      if (!ParseItem(parser, out_resource, resource_format)) {
         return false;
       }
       return true;
diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index 2e6ad13d99de..b59b16574c42 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -69,13 +69,8 @@ class ResourceParserTest : public ::testing::Test {
     return TestParse(str, ConfigDescription{});
   }
 
-  ::testing::AssertionResult TestParse(StringPiece str, ResourceParserOptions parserOptions) {
-    return TestParse(str, ConfigDescription{}, parserOptions);
-  }
-
-  ::testing::AssertionResult TestParse(
-      StringPiece str, const ConfigDescription& config,
-      ResourceParserOptions parserOptions = ResourceParserOptions()) {
+  ::testing::AssertionResult TestParse(StringPiece str, const ConfigDescription& config) {
+    ResourceParserOptions parserOptions;
     ResourceParser parser(context_->GetDiagnostics(), &table_, android::Source{"test"}, config,
                           parserOptions);
 
@@ -247,19 +242,6 @@ TEST_F(ResourceParserTest, ParseStringTranslatableAttribute) {
   EXPECT_FALSE(TestParse(R"(<string name="foo4" translatable="yes">Translate</string>)"));
 }
 
-TEST_F(ResourceParserTest, ParseStringBehindDisabledFlag) {
-  FeatureFlagProperties flag_properties(true, false);
-  ResourceParserOptions options;
-  options.feature_flag_values = {{"falseFlag", flag_properties}};
-  ASSERT_TRUE(TestParse(
-      R"(<string name="foo" android:featureFlag="falseFlag"
-              xmlns:android="http://schemas.android.com/apk/res/android">foo</string>)",
-      options));
-
-  String* str = test::GetValue<String>(&table_, "string/foo");
-  ASSERT_THAT(str, IsNull());
-}
-
 TEST_F(ResourceParserTest, IgnoreXliffTagsOtherThanG) {
   std::string input = R"(
       <string name="foo" xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 642a5618b6ad..88cc4dab2a96 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1840,11 +1840,51 @@ class Linker {
     return validate(attr->value);
   }
 
+  class FlagDisabledStringVisitor : public DescendingValueVisitor {
+   public:
+    using DescendingValueVisitor::Visit;
+
+    explicit FlagDisabledStringVisitor(android::StringPool& string_pool)
+        : string_pool_(string_pool) {
+    }
+
+    void Visit(RawString* value) override {
+      value->value = string_pool_.MakeRef("");
+    }
+
+    void Visit(String* value) override {
+      value->value = string_pool_.MakeRef("");
+    }
+
+    void Visit(StyledString* value) override {
+      value->value = string_pool_.MakeRef(android::StyleString{{""}, {}});
+    }
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(FlagDisabledStringVisitor);
+    android::StringPool& string_pool_;
+  };
+
   // Writes the AndroidManifest, ResourceTable, and all XML files referenced by the ResourceTable
   // to the IArchiveWriter.
   bool WriteApk(IArchiveWriter* writer, proguard::KeepSet* keep_set, xml::XmlResource* manifest,
                 ResourceTable* table) {
     TRACE_CALL();
+
+    FlagDisabledStringVisitor visitor(table->string_pool);
+
+    for (auto& package : table->packages) {
+      for (auto& type : package->types) {
+        for (auto& entry : type->entries) {
+          for (auto& config_value : entry->values) {
+            if (config_value->flag_status == FlagStatus::Disabled) {
+              config_value->value->Accept(&visitor);
+            }
+          }
+        }
+      }
+    }
+
     const bool keep_raw_values = (context_->GetPackageType() == PackageType::kStaticLib)
                                  || options_.keep_raw_values;
     bool result = FlattenXml(context_, *manifest, kAndroidManifestPath, keep_raw_values,
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
new file mode 100644
index 000000000000..efa8e0494856
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
@@ -0,0 +1,65 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package {
+    // See: http://go/android-license-faq
+    // A large-scale-change added 'default_applicable_licenses' to import
+    // all of the 'license_kinds' from "frameworks_base_license"
+    // to get the below license kinds:
+    //   SPDX-license-identifier-Apache-2.0
+    default_applicable_licenses: ["frameworks_base_license"],
+    default_team: "trendy_team_android_resources",
+}
+
+genrule {
+    name: "resource-flagging-test-app-compile",
+    tools: ["aapt2"],
+    srcs: [
+        "res/values/bools.xml",
+        "res/values/bools2.xml",
+        "res/values/strings.xml",
+    ],
+    out: [
+        "values_bools.arsc.flat",
+        "values_bools2.arsc.flat",
+        "values_strings.arsc.flat",
+    ],
+    cmd: "$(location aapt2) compile $(in) -o $(genDir) " +
+        "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true",
+}
+
+genrule {
+    name: "resource-flagging-test-app-apk",
+    tools: ["aapt2"],
+    // The first input file in the list must be the manifest
+    srcs: [
+        "AndroidManifest.xml",
+        ":resource-flagging-test-app-compile",
+    ],
+    out: ["resapp.apk"],
+    cmd: "$(location aapt2) link -o $(out) --manifest $(in)",
+}
+
+java_genrule {
+    name: "resource-flagging-test-app-apk-as-resource",
+    srcs: [
+        ":resource-flagging-test-app-apk",
+    ],
+    out: ["apks_as_resources.res.zip"],
+    tools: ["soong_zip"],
+
+    cmd: "mkdir -p $(genDir)/res/raw && " +
+        "cp $(in) $(genDir)/res/raw/$$(basename $(in)) && " +
+        "$(location soong_zip) -o $(out) -C $(genDir)/res -D $(genDir)/res",
+}
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml
new file mode 100644
index 000000000000..d6cdeb7b5231
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml
@@ -0,0 +1,4 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.android.intenal.flaggedresources">
+    <application/>
+</manifest>
\ No newline at end of file
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
new file mode 100644
index 000000000000..8d0146511d1d
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+    <bool name="res1">true</bool>
+    <bool name="res1" android:featureFlag="test.package.falseFlag">false</bool>
+
+    <bool name="res2">false</bool>
+    <bool name="res2" android:featureFlag="test.package.trueFlag">true</bool>
+
+    <bool name="res3">false</bool>
+</resources>
\ No newline at end of file
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml
new file mode 100644
index 000000000000..e7563aa0fbdd
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+    <bool name="res3" android:featureFlag="test.package.trueFlag">true</bool>
+</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
new file mode 100644
index 000000000000..5c0fca16fe39
--- /dev/null
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/strings.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<resources xmlns:android="http://schemas.android.com/apk/res/android">
+    <string name="str">plain string</string>
+
+    <string name="str1" android:featureFlag="test.package.falseFlag">DONTFIND</string>
+</resources>
\ No newline at end of file
diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp
new file mode 100644
index 000000000000..03c4ac9c83a2
--- /dev/null
+++ b/tools/aapt2/link/FlaggedResources_test.cpp
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "LoadedApk.h"
+#include "cmd/Dump.h"
+#include "io/StringStream.h"
+#include "test/Test.h"
+#include "text/Printer.h"
+
+using ::aapt::io::StringOutputStream;
+using ::aapt::text::Printer;
+using testing::Eq;
+using testing::Ne;
+
+namespace aapt {
+
+using FlaggedResourcesTest = CommandTestFixture;
+
+static android::NoOpDiagnostics noop_diag;
+
+void DumpStringPoolToString(LoadedApk* loaded_apk, std::string* output) {
+  StringOutputStream output_stream(output);
+  Printer printer(&output_stream);
+
+  DumpStringsCommand command(&printer, &noop_diag);
+  ASSERT_EQ(command.Dump(loaded_apk), 0);
+  output_stream.Flush();
+}
+
+TEST_F(FlaggedResourcesTest, DisabledStringRemoved) {
+  auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"});
+  auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag);
+
+  std::string output;
+  DumpStringPoolToString(loaded_apk.get(), &output);
+
+  std::string excluded = "DONTFIND";
+  ASSERT_EQ(output.find(excluded), std::string::npos);
+}
+
+}  // namespace aapt
-- 
cgit v1.2.3-59-g8ed1b


From d52bd6858a6b2913f46e57bca0d96d5212416f10 Mon Sep 17 00:00:00 2001
From: Jeremy Meyer <jakmcbane@google.com>
Date: Wed, 14 Aug 2024 11:16:58 -0700
Subject: Flag support for bag resource types

Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I891c93c3ffcab172d28701b44a80c50f1e24d99e
---
 .../ResourceFlaggingTest.java                      | 25 +++++++--
 tools/aapt2/ResourceParser.cpp                     | 61 ++++++++++++++--------
 tools/aapt2/ResourceParser.h                       |  2 +
 tools/aapt2/ResourceTable.cpp                      |  7 ++-
 tools/aapt2/ResourceTable.h                        |  2 -
 tools/aapt2/ResourceValues.cpp                     | 11 ++++
 tools/aapt2/ResourceValues.h                       | 14 +++++
 tools/aapt2/Resources.proto                        |  5 +-
 tools/aapt2/cmd/Link.cpp                           |  2 +-
 tools/aapt2/format/proto/ProtoDeserialize.cpp      | 26 ++++++---
 tools/aapt2/format/proto/ProtoSerialize.cpp        |  5 +-
 .../FlaggedResourcesTest/Android.bp                |  2 +
 .../FlaggedResourcesTest/res/values/ints.xml       |  9 ++++
 .../FlaggedResourcesTest/res/values/strings.xml    |  7 +++
 tools/aapt2/link/FlagDisabledResourceRemover.cpp   |  7 ++-
 tools/aapt2/link/TableMerger.cpp                   |  5 +-
 16 files changed, 144 insertions(+), 46 deletions(-)
 create mode 100644 tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/ints.xml

(limited to 'tools/aapt2/ResourceParser.cpp')

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.
-- 
cgit v1.2.3-59-g8ed1b