diff options
10 files changed, 107 insertions, 40 deletions
diff --git a/core/tests/resourceflaggingtests/Android.bp b/core/tests/resourceflaggingtests/Android.bp index efb843735aef..40bdc2bc61e1 100644 --- a/core/tests/resourceflaggingtests/Android.bp +++ b/core/tests/resourceflaggingtests/Android.bp @@ -26,6 +26,7 @@ android_test { name: "ResourceFlaggingTests", srcs: [ "src/**/*.java", + ":resource-flagging-test-app-r-java", ], platform_apis: true, certificate: "platform", diff --git a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java index 471b4021545a..005538a4d401 100644 --- a/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java +++ b/core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java @@ -19,15 +19,22 @@ package com.android.resourceflaggingtests; import static com.google.common.truth.Truth.assertThat; import android.content.Context; +import android.content.ContextWrapper; +import android.content.res.ApkAssets; import android.content.res.AssetManager; import android.content.res.Configuration; import android.content.res.Resources; import android.os.FileUtils; import android.util.DisplayMetrics; +import android.view.LayoutInflater; +import android.view.View; +import android.widget.LinearLayout; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; +import com.android.intenal.flaggedresources.R; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -46,7 +53,14 @@ public class ResourceFlaggingTest { public void setUp() throws Exception { mContext = InstrumentationRegistry.getTargetContext(); AssetManager assets = new AssetManager(); - assertThat(assets.addAssetPath(extractApkAndGetPath(R.raw.resapp))).isNotEqualTo(0); + assets.setApkAssets( + new ApkAssets[]{ + ApkAssets.loadFromPath( + extractApkAndGetPath( + com.android.resourceflaggingtests.R.raw.resapp + ) + ) + }, true); final DisplayMetrics dm = new DisplayMetrics(); dm.setToDefaults(); @@ -55,54 +69,60 @@ public class ResourceFlaggingTest { @Test public void testFlagDisabled() { - assertThat(getBoolean("res1")).isTrue(); + assertThat(mResources.getBoolean(R.bool.bool1)).isTrue(); } @Test public void testFlagEnabled() { - assertThat(getBoolean("res2")).isTrue(); + assertThat(mResources.getBoolean(R.bool.bool2)).isTrue(); } @Test public void testFlagEnabledDifferentCompilationUnit() { - assertThat(getBoolean("res3")).isTrue(); + assertThat(mResources.getBoolean(R.bool.bool3)).isTrue(); } @Test public void testFlagDisabledStringArrayElement() { - assertThat(getStringArray("strarr1")).isEqualTo(new String[]{"one", "two", "three"}); + assertThat(mResources.getStringArray(R.array.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, - "bool", - "com.android.intenal.flaggedresources"); - assertThat(resId).isNotEqualTo(0); - return mResources.getBoolean(resId); + assertThat(mResources.getIntArray(R.array.intarr1)).isEqualTo(new int[]{1, 2, 3}); } - private String[] getStringArray(String name) { - int resId = mResources.getIdentifier( - name, - "array", - "com.android.intenal.flaggedresources"); - assertThat(resId).isNotEqualTo(0); - return mResources.getStringArray(resId); + @Test + public void testLayoutWithDisabledElements() { + LinearLayout ll = (LinearLayout) getLayoutInflater().inflate(R.layout.layout1, null); + assertThat(ll).isNotNull(); + assertThat((View) ll.findViewById(R.id.text1)).isNotNull(); + assertThat((View) ll.findViewById(R.id.disabled_text)).isNull(); + assertThat((View) ll.findViewById(R.id.text2)).isNotNull(); } - private int[] getIntArray(String name) { - int resId = mResources.getIdentifier( - name, - "array", - "com.android.intenal.flaggedresources"); - assertThat(resId).isNotEqualTo(0); - return mResources.getIntArray(resId); + private LayoutInflater getLayoutInflater() { + ContextWrapper c = new ContextWrapper(mContext) { + private LayoutInflater mInflater; + + @Override + public Resources getResources() { + return mResources; + } + + @Override + public Object getSystemService(String name) { + if (LAYOUT_INFLATER_SERVICE.equals(name)) { + if (mInflater == null) { + mInflater = LayoutInflater.from(getBaseContext()).cloneInContext(this); + } + return mInflater; + } + return super.getSystemService(name); + } + }; + return LayoutInflater.from(c); } private String extractApkAndGetPath(int id) throws Exception { diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index be63f82b30cf..498e431097ad 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -306,6 +306,7 @@ struct ResourceFileFlattenerOptions { OutputFormat output_format = OutputFormat::kApk; std::unordered_set<std::string> extensions_to_not_compress; std::optional<std::regex> regex_to_not_compress; + FeatureFlagValues feature_flag_values; }; // A sampling of public framework resource IDs. @@ -672,6 +673,13 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv } } + FeatureFlagsFilterOptions flags_filter_options; + flags_filter_options.flags_must_be_readonly = true; + FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options); + if (!flags_filter.Consume(context_, doc.get())) { + return 1; + } + error |= !FlattenXml(context_, *doc, dst_path, options_.keep_raw_values, false /*utf16*/, options_.output_format, archive_writer); } @@ -1926,6 +1934,7 @@ class Linker { static_cast<bool>(options_.generate_proguard_rules_path); file_flattener_options.output_format = options_.output_format; file_flattener_options.do_not_fail_on_missing_resources = options_.merge_only; + file_flattener_options.feature_flag_values = options_.feature_flag_values; ResourceFileFlattener file_flattener(file_flattener_options, context_, keep_set); if (!file_flattener.Flatten(table, writer)) { diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp index 4866d2c83c71..c456e5c296d2 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/Android.bp @@ -30,12 +30,14 @@ genrule { "res/values/bools2.xml", "res/values/ints.xml", "res/values/strings.xml", + "res/layout/layout1.xml", ], out: [ "values_bools.arsc.flat", "values_bools2.arsc.flat", "values_ints.arsc.flat", "values_strings.arsc.flat", + "layout_layout1.xml.flat", ], cmd: "$(location aapt2) compile $(in) -o $(genDir) " + "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true", @@ -52,7 +54,10 @@ genrule { out: [ "resapp.apk", ], - cmd: "$(location aapt2) link -o $(out) --manifest $(in)", + cmd: "$(location aapt2) link -o $(out) --manifest $(in) " + + "-I $(location :current_android_jar) " + + "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true", + tool_files: [":current_android_jar"], } genrule { @@ -66,7 +71,10 @@ genrule { 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)", + cmd: "$(location aapt2) link -o $(genDir)/resapp.apk --java $(genDir)/resource-flagging-java --manifest $(in) " + + "-I $(location :current_android_jar) " + + "--feature-flags test.package.falseFlag:ro=false,test.package.trueFlag:ro=true", + tool_files: [":current_android_jar"], } java_genrule { diff --git a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout1.xml b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout1.xml new file mode 100644 index 000000000000..8b9ce134a9de --- /dev/null +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout1.xml @@ -0,0 +1,18 @@ +<?xml version="1.0" encoding="utf-8"?> +<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" + android:layout_width="match_parent" + android:layout_height="match_parent" + android:orientation="vertical" > + + <TextView android:id="@+id/text1" + android:layout_width="wrap_content" + android:layout_height="wrap_content"/> + <TextView android:id="@+id/disabled_text" + android:layout_width="wrap_content" + android:layout_height="wrap_content" + android:featureFlag="test.package.falseFlag" /> + <TextView android:id="@+id/text2" + android:layout_width="wrap_content" + android:layout_height="wrap_content" + android:featureFlag="test.package.trueFlag" /> +</LinearLayout>
\ 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 index 3e094fbd669c..1ed0c8a5f1e6 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools.xml @@ -1,12 +1,12 @@ <?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="bool1">true</bool> + <bool name="bool1" android:featureFlag="test.package.falseFlag">false</bool> - <bool name="res2">false</bool> - <bool name="res2" android:featureFlag="test.package.trueFlag">true</bool> + <bool name="bool2">false</bool> + <bool name="bool2" android:featureFlag="test.package.trueFlag">true</bool> - <bool name="res3">false</bool> + <bool name="bool3">false</bool> - <bool name="res4" android:featureFlag="test.package.falseFlag">true</bool> + <bool name="bool4" android:featureFlag="test.package.falseFlag">true</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 index e7563aa0fbdd..248c45f7fad1 100644 --- a/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml +++ b/tools/aapt2/integration-tests/FlaggedResourcesTest/res/values/bools2.xml @@ -1,4 +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> + <bool name="bool3" android:featureFlag="test.package.trueFlag">true</bool> </resources>
\ No newline at end of file diff --git a/tools/aapt2/link/FeatureFlagsFilter.cpp b/tools/aapt2/link/FeatureFlagsFilter.cpp index 9d40db521e13..4e7c1b4d8e54 100644 --- a/tools/aapt2/link/FeatureFlagsFilter.cpp +++ b/tools/aapt2/link/FeatureFlagsFilter.cpp @@ -65,6 +65,13 @@ class FlagsVisitor : public xml::Visitor { if (auto it = feature_flag_values_.find(flag_name); it != feature_flag_values_.end()) { if (it->second.enabled.has_value()) { + if (options_.flags_must_be_readonly && !it->second.read_only) { + diagnostics_->Error(android::DiagMessage(node->line_number) + << "attribute 'android:featureFlag' has flag '" << flag_name + << "' which must be readonly but is not"); + has_error_ = true; + return false; + } if (options_.remove_disabled_elements) { // Remove if flag==true && attr=="!flag" (negated) OR flag==false && attr=="flag" return *it->second.enabled == negated; diff --git a/tools/aapt2/link/FeatureFlagsFilter.h b/tools/aapt2/link/FeatureFlagsFilter.h index 1d342a71b996..61e4c8015f0e 100644 --- a/tools/aapt2/link/FeatureFlagsFilter.h +++ b/tools/aapt2/link/FeatureFlagsFilter.h @@ -38,6 +38,10 @@ struct FeatureFlagsFilterOptions { // If true, `Consume()` will return false (error) if a flag was found whose value in // `feature_flag_values` is not defined (std::nullopt). bool flags_must_have_value = true; + + // If true, `Consume()` will return false (error) if a flag was found whose value in + // `feature_flag_values` is not readonly. + bool flags_must_be_readonly = false; }; // Looks for the `android:featureFlag` attribute in each XML element, validates the flag names and diff --git a/tools/aapt2/link/FlaggedResources_test.cpp b/tools/aapt2/link/FlaggedResources_test.cpp index c901b5866279..3db37c2fa6f8 100644 --- a/tools/aapt2/link/FlaggedResources_test.cpp +++ b/tools/aapt2/link/FlaggedResources_test.cpp @@ -84,7 +84,7 @@ TEST_F(FlaggedResourcesTest, DisabledResourcesRemovedFromTableChunks) { std::string output; DumpChunksToString(loaded_apk.get(), &output); - ASSERT_EQ(output.find("res4"), std::string::npos); + ASSERT_EQ(output.find("bool4"), std::string::npos); ASSERT_EQ(output.find("str1"), std::string::npos); } @@ -94,7 +94,7 @@ TEST_F(FlaggedResourcesTest, DisabledResourcesInRJava) { 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 bool4"), std::string::npos); ASSERT_NE(r_contents.find("public static final int str1"), std::string::npos); } |