Remove flag disabled strings from string pool
Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I627feff5774f44a398a8337733498ede601d07a4
diff --git a/core/tests/resourceflaggingtests/Android.bp b/core/tests/resourceflaggingtests/Android.bp
index dd86094..efb8437 100644
--- a/core/tests/resourceflaggingtests/Android.bp
+++ b/core/tests/resourceflaggingtests/Android.bp
@@ -22,54 +22,6 @@
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 @@
"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/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index ad8542e..a3d8d3d 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 @@
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 3f9016b..7b227ce 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -189,6 +189,7 @@
"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 9444dd9..1c85e9f 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -690,9 +690,7 @@
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 2e6ad13..b59b165 100644
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -69,13 +69,8 @@
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 @@
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 642a561..88cc4da 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1840,11 +1840,51 @@
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 0000000..efa8e04
--- /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/core/tests/resourceflaggingtests/TestAppAndroidManifest.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml
similarity index 100%
rename from core/tests/resourceflaggingtests/TestAppAndroidManifest.xml
rename to tools/aapt2/integration-tests/FlaggedResourcesTest/AndroidManifest.xml
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
similarity index 100%
rename from core/tests/resourceflaggingtests/flagged_resources_res/values/bools.xml
rename to tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
diff --git a/core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml
similarity index 100%
rename from core/tests/resourceflaggingtests/flagged_resources_res/values/bools2.xml
rename to tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml
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 0000000..5c0fca1
--- /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 0000000..03c4ac9
--- /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