Keep disabled resources out of final apk
Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: Id5fdb025f004788ea40bdbd0b0df0dbda181f2c7
diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
index a3d8d3d..c1e3578 100644
--- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
+++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java
@@ -68,11 +68,6 @@
assertThat(getBoolean("res3")).isTrue();
}
- @Test
- public void testFlagDisabledNoValue() {
- assertThat(getString("str1")).isEqualTo("");
- }
-
private boolean getBoolean(String name) {
int resId = mResources.getIdentifier(
name,
diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index 7b227ce..f43cf52 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -113,6 +113,7 @@
"io/ZipArchive.cpp",
"link/AutoVersioner.cpp",
"link/FeatureFlagsFilter.cpp",
+ "link/FlagDisabledResourceRemover.cpp",
"link/ManifestFixer.cpp",
"link/NoDefaultResourceRemover.cpp",
"link/PrivateAttributeMover.cpp",
@@ -190,6 +191,7 @@
"integration-tests/ConvertTest/**/*",
"integration-tests/DumpTest/**/*",
":resource-flagging-test-app-apk",
+ ":resource-flagging-test-app-r-java",
],
}
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 88cc4da..56f5288 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -57,6 +57,7 @@
#include "java/ManifestClassGenerator.h"
#include "java/ProguardRules.h"
#include "link/FeatureFlagsFilter.h"
+#include "link/FlagDisabledResourceRemover.h"
#include "link/Linkers.h"
#include "link/ManifestFixer.h"
#include "link/NoDefaultResourceRemover.h"
@@ -1885,6 +1886,12 @@
}
}
+ if (!FlagDisabledResourceRemover{}.Consume(context_, table)) {
+ context_->GetDiagnostics()->Error(android::DiagMessage()
+ << "failed removing resources behind disabled flags");
+ return 1;
+ }
+
const bool keep_raw_values = (context_->GetPackageType() == PackageType::kStaticLib)
|| options_.keep_raw_values;
bool result = FlattenXml(context_, *manifest, kAndroidManifestPath, keep_raw_values,
@@ -2371,6 +2378,12 @@
return 1;
};
+ if (options_.generate_java_class_path || options_.generate_text_symbols_path) {
+ if (!GenerateJavaClasses()) {
+ return 1;
+ }
+ }
+
if (!WriteApk(archive_writer.get(), &proguard_keep_set, manifest_xml.get(), &final_table_)) {
return 1;
}
@@ -2379,12 +2392,6 @@
return 1;
}
- if (options_.generate_java_class_path || options_.generate_text_symbols_path) {
- if (!GenerateJavaClasses()) {
- return 1;
- }
- }
-
if (!WriteProguardFile(options_.generate_proguard_rules_path, proguard_keep_set)) {
return 1;
}
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
index efa8e04..5932271 100644
--- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp
@@ -47,10 +47,26 @@
"AndroidManifest.xml",
":resource-flagging-test-app-compile",
],
- out: ["resapp.apk"],
+ out: [
+ "resapp.apk",
+ ],
cmd: "$(location aapt2) link -o $(out) --manifest $(in)",
}
+genrule {
+ name: "resource-flagging-test-app-r-java",
+ tools: ["aapt2"],
+ // The first input file in the list must be the manifest
+ srcs: [
+ "AndroidManifest.xml",
+ ":resource-flagging-test-app-compile",
+ ],
+ out: [
+ "resource-flagging-java/com/android/intenal/flaggedresources/R.java",
+ ],
+ cmd: "$(location aapt2) link -o $(genDir)/resapp.apk --java $(genDir)/resource-flagging-java --manifest $(in)",
+}
+
java_genrule {
name: "resource-flagging-test-app-apk-as-resource",
srcs: [
diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
index 8d01465..3e094fb 100644
--- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
+++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml
@@ -7,4 +7,6 @@
<bool name="res2" android:featureFlag="test.package.trueFlag">true</bool>
<bool name="res3">false</bool>
+
+ <bool name="res4" android:featureFlag="test.package.falseFlag">true</bool>
</resources>
\ No newline at end of file
diff --git a/tools/aapt2/link/FlagDisabledResourceRemover.cpp b/tools/aapt2/link/FlagDisabledResourceRemover.cpp
new file mode 100644
index 0000000..e3289e2
--- /dev/null
+++ b/tools/aapt2/link/FlagDisabledResourceRemover.cpp
@@ -0,0 +1,59 @@
+/*
+ * 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.
+ */
+
+#include "link/FlagDisabledResourceRemover.h"
+
+#include <algorithm>
+
+#include "ResourceTable.h"
+
+using android::ConfigDescription;
+
+namespace aapt {
+
+static bool KeepResourceEntry(const std::unique_ptr<ResourceEntry>& entry) {
+ if (entry->values.empty()) {
+ return true;
+ }
+ const auto end_iter = entry->values.end();
+ 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;
+ });
+
+ bool keep = remove_iter != entry->values.begin();
+
+ entry->values.erase(remove_iter, end_iter);
+ return keep;
+}
+
+bool FlagDisabledResourceRemover::Consume(IAaptContext* context, ResourceTable* table) {
+ for (auto& pkg : table->packages) {
+ for (auto& type : pkg->types) {
+ const auto end_iter = type->entries.end();
+ const auto remove_iter = std::stable_partition(
+ type->entries.begin(), end_iter, [](const std::unique_ptr<ResourceEntry>& entry) -> bool {
+ return KeepResourceEntry(entry);
+ });
+
+ type->entries.erase(remove_iter, end_iter);
+ }
+ }
+ return true;
+}
+
+} // namespace aapt
\ No newline at end of file
diff --git a/tools/aapt2/link/FlagDisabledResourceRemover.h b/tools/aapt2/link/FlagDisabledResourceRemover.h
new file mode 100644
index 0000000..2db2cb4
--- /dev/null
+++ b/tools/aapt2/link/FlagDisabledResourceRemover.h
@@ -0,0 +1,35 @@
+/*
+ * 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.
+ */
+
+#pragma once
+
+#include "android-base/macros.h"
+#include "process/IResourceTableConsumer.h"
+
+namespace aapt {
+
+// Removes any resource that are behind disabled flags.
+class FlagDisabledResourceRemover : public IResourceTableConsumer {
+ public:
+ FlagDisabledResourceRemover() = default;
+
+ bool Consume(IAaptContext* context, ResourceTable* table) override;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FlagDisabledResourceRemover);
+};
+
+} // namespace aapt
diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp
index 03c4ac9..c901b58 100644
--- a/tools/aapt2/link/FlaggedResources_test.cpp
+++ b/tools/aapt2/link/FlaggedResources_test.cpp
@@ -40,7 +40,25 @@
output_stream.Flush();
}
-TEST_F(FlaggedResourcesTest, DisabledStringRemoved) {
+void DumpResourceTableToString(LoadedApk* loaded_apk, std::string* output) {
+ StringOutputStream output_stream(output);
+ Printer printer(&output_stream);
+
+ DumpTableCommand command(&printer, &noop_diag);
+ ASSERT_EQ(command.Dump(loaded_apk), 0);
+ output_stream.Flush();
+}
+
+void DumpChunksToString(LoadedApk* loaded_apk, std::string* output) {
+ StringOutputStream output_stream(output);
+ Printer printer(&output_stream);
+
+ DumpChunks command(&printer, &noop_diag);
+ ASSERT_EQ(command.Dump(loaded_apk), 0);
+ output_stream.Flush();
+}
+
+TEST_F(FlaggedResourcesTest, DisabledStringRemovedFromPool) {
auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"});
auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag);
@@ -51,4 +69,33 @@
ASSERT_EQ(output.find(excluded), std::string::npos);
}
+TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTable) {
+ auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"});
+ auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag);
+
+ std::string output;
+ DumpResourceTableToString(loaded_apk.get(), &output);
+}
+
+TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTableChunks) {
+ auto apk_path = file::BuildPath({android::base::GetExecutableDirectory(), "resapp.apk"});
+ auto loaded_apk = LoadedApk::LoadApkFromPath(apk_path, &noop_diag);
+
+ std::string output;
+ DumpChunksToString(loaded_apk.get(), &output);
+
+ ASSERT_EQ(output.find("res4"), std::string::npos);
+ ASSERT_EQ(output.find("str1"), std::string::npos);
+}
+
+TEST_F(FlaggedResourcesTest, DisabledResourcesInRJava) {
+ auto r_path = file::BuildPath({android::base::GetExecutableDirectory(), "resource-flagging-java",
+ "com", "android", "intenal", "flaggedresources", "R.java"});
+ std::string r_contents;
+ ::android::base::ReadFileToString(r_path, &r_contents);
+
+ ASSERT_NE(r_contents.find("public static final int res4"), std::string::npos);
+ ASSERT_NE(r_contents.find("public static final int str1"), std::string::npos);
+}
+
} // namespace aapt