Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1c42e7ca authored by Jeremy Meyer's avatar Jeremy Meyer
Browse files

Allow read/write flags on xml elements 

This changes aapt2 so that it now allows xml elements with the
android:featureFlag attribute even when that attribute is read/write (or
not found altogether). This also makes it so that attribute is removed
when the flag is read only.

Bug: 377974898
Test: Automated
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: I330eefe897ab6dd1301b073d7f69443e3428b5b6
parent a10033c1

tools/aapt2/cmd/Link.cpp

+2 −1
Original line number Diff line number Diff line
@@ -674,7 +674,8 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 
            }



            FeatureFlagsFilterOptions flags_filter_options;

            flags_filter_options.flags_must_be_readonly = true;

            flags_filter_options.fail_on_unrecognized_flags = false;

            flags_filter_options.flags_must_have_value = false;

            FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options);

            if (!flags_filter.Consume(context_, doc.get())) {

              return 1;

tools/aapt2/cmd/Link_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -1026,7 +1026,7 @@ TEST_F(LinkTest, FeatureFlagDisabled_SdkAtMostUDC) { 
                           .SetManifestFile(app_manifest)

                           .AddParameter("-I", android_apk)

                           .AddParameter("--java", app_java)

                           .AddParameter("--feature-flags", "flag=false");

                           .AddParameter("--feature-flags", "flag:ro=false");



  const std::string app_apk = GetTestPath("app.apk");

  BuildApk({}, app_apk, std::move(app_link_args), this, &diag);

tools/aapt2/integration-tests/FlaggedResourcesTest/res/layout/layout1.xml

+3 −1
Original line number Diff line number Diff line
@@ -6,12 +6,14 @@ 


    <TextView android:id="@+id/text1"

        android:layout_width="wrap_content"

        android:layout_height="wrap_content"/>

        android:layout_height="wrap_content"

        android:featureFlag="test.package.readWriteFlag"/>

    <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:text="FIND_ME"

        android:layout_width="wrap_content"

        android:layout_height="wrap_content"

        android:featureFlag="test.package.trueFlag" />

tools/aapt2/link/FeatureFlagsFilter.cpp

+7 −3
Original line number Diff line number Diff line
@@ -50,7 +50,7 @@ class FlagsVisitor : public xml::Visitor { 


 private:

  bool ShouldRemove(std::unique_ptr<xml::Node>& node) {

    if (const auto* el = NodeCast<Element>(node.get())) {

    if (auto* el = NodeCast<Element>(node.get())) {

      auto* attr = el->FindAttribute(xml::kSchemaAndroid, "featureFlag");

      if (attr == nullptr) {

        return false;
@@ -72,9 +72,13 @@ class FlagsVisitor : public xml::Visitor { 
            has_error_ = true;

            return false;

          }

          if (options_.remove_disabled_elements) {

          if (options_.remove_disabled_elements && it->second.read_only) {

            // Remove if flag==true && attr=="!flag" (negated) OR flag==false && attr=="flag"

            return *it->second.enabled == negated;

            bool remove = *it->second.enabled == negated;

            if (!remove) {

              el->RemoveAttribute(xml::kSchemaAndroid, "featureFlag");

            }

            return remove;

          }

        } else if (options_.flags_must_have_value) {

          diagnostics_->Error(android::DiagMessage(node->line_number)

tools/aapt2/link/FeatureFlagsFilter_test.cpp

+14 −14
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ TEST(FeatureFlagsFilterTest, NoFeatureFlagAttributes) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, false}}});

                    {{"flag", FeatureFlagProperties{true, false}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -60,7 +60,7 @@ TEST(FeatureFlagsFilterTest, RemoveElementWithDisabledFlag) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="flag" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, false}}});

                    {{"flag", FeatureFlagProperties{true, false}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -73,7 +73,7 @@ TEST(FeatureFlagsFilterTest, RemoveElementWithNegatedEnabledFlag) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="!flag" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -86,7 +86,7 @@ TEST(FeatureFlagsFilterTest, KeepElementWithEnabledFlag) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="flag" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -102,7 +102,7 @@ TEST(FeatureFlagsFilterTest, SideBySideEnabledAndDisabled) { 
      <permission android:name="FOO" android:featureFlag="flag"

                  android:protectionLevel="dangerous" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -123,7 +123,7 @@ TEST(FeatureFlagsFilterTest, RemoveDeeplyNestedElement) { 
        </activity>

      </application>

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -145,7 +145,7 @@ TEST(FeatureFlagsFilterTest, KeepDeeplyNestedElement) { 
        </activity>

      </application>

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();

  ASSERT_THAT(root, NotNull());
@@ -162,7 +162,7 @@ TEST(FeatureFlagsFilterTest, FailOnEmptyFeatureFlagAttribute) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag=" " />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, false}}});

                    {{"flag", FeatureFlagProperties{true, false}}});

  ASSERT_THAT(doc, IsNull());

}
@@ -171,7 +171,7 @@ TEST(FeatureFlagsFilterTest, FailOnFlagWithNoGivenValue) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="flag" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, std::nullopt}}});

                    {{"flag", FeatureFlagProperties{true, std::nullopt}}});

  ASSERT_THAT(doc, IsNull());

}
@@ -180,7 +180,7 @@ TEST(FeatureFlagsFilterTest, FailOnUnrecognizedFlag) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="unrecognized" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, true}}});

                    {{"flag", FeatureFlagProperties{true, true}}});

  ASSERT_THAT(doc, IsNull());

}
@@ -190,7 +190,7 @@ TEST(FeatureFlagsFilterTest, FailOnMultipleValidationErrors) { 
      <permission android:name="FOO" android:featureFlag="bar" />

      <permission android:name="FOO" android:featureFlag="unrecognized" />

    </manifest>)EOF",

                    {{"flag", FeatureFlagProperties{false, std::nullopt}}});

                    {{"flag", FeatureFlagProperties{true, std::nullopt}}});

  ASSERT_THAT(doc, IsNull());

}
@@ -199,7 +199,7 @@ TEST(FeatureFlagsFilterTest, OptionRemoveDisabledElementsIsFalse) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="flag" />

    </manifest>)EOF",

                               {{"flag", FeatureFlagProperties{false, false}}},

                               {{"flag", FeatureFlagProperties{true, false}}},

                               {.remove_disabled_elements = false});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();
@@ -213,7 +213,7 @@ TEST(FeatureFlagsFilterTest, OptionFlagsMustHaveValueIsFalse) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="flag" />

    </manifest>)EOF",

                               {{"flag", FeatureFlagProperties{false, std::nullopt}}},

                               {{"flag", FeatureFlagProperties{true, std::nullopt}}},

                               {.flags_must_have_value = false});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();
@@ -227,7 +227,7 @@ TEST(FeatureFlagsFilterTest, OptionFailOnUnrecognizedFlagsIsFalse) { 
    <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android">

      <permission android:name="FOO" android:featureFlag="unrecognized" />

    </manifest>)EOF",

                               {{"flag", FeatureFlagProperties{false, true}}},

                               {{"flag", FeatureFlagProperties{true, true}}},

                               {.fail_on_unrecognized_flags = false});

  ASSERT_THAT(doc, NotNull());

  auto root = doc->root.get();