AAPT2: Support id reference chaining from AAPT

AAPT would allow for ids to be declared in the form:

<item name="name" type="id>@id/other</item>

@id/name should hold a reference to @id/other. When
getResources().getValue() is called on R.id.name with resolveRefs
enabled, the resuling reference should be R.id.other.

Bug: 69445910
Test: Created tests for correct parsing of id references and correct
resolving of deep references

Change-Id: Id1feb37b2565c213dc6a19b4c401906260d7fc14

diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp
index 3118009..f1cc569 100644 (file)
--- a/libs/androidfw/tests/AssetManager2_test.cpp
+++ b/libs/androidfw/tests/AssetManager2_test.cpp
@@ -386,6 +386,38 @@ TEST_F(AssetManager2Test, ResolveReferenceToBag) {
   EXPECT_EQ(basic::R::array::integerArray1, last_ref);
 }
 
+TEST_F(AssetManager2Test, ResolveDeepIdReference) {
+  AssetManager2 assetmanager;
+  assetmanager.SetApkAssets({basic_assets_.get()});
+
+  // Set up the resource ids
+  const uint32_t high_ref = assetmanager
+      .GetResourceId("@id/high_ref", "values", "com.android.basic");
+  ASSERT_NE(high_ref, 0u);
+  const uint32_t middle_ref = assetmanager
+      .GetResourceId("@id/middle_ref", "values", "com.android.basic");
+  ASSERT_NE(middle_ref, 0u);
+  const uint32_t low_ref = assetmanager
+      .GetResourceId("@id/low_ref", "values", "com.android.basic");
+  ASSERT_NE(low_ref, 0u);
+
+  // Retrieve the most shallow resource
+  Res_value value;
+  ResTable_config config;
+  uint32_t flags;
+  ApkAssetsCookie cookie = assetmanager.GetResource(high_ref, false /*may_be_bag*/,
+                                                    0 /*density_override*/,
+                                                    &value, &config, &flags);
+  ASSERT_NE(kInvalidCookie, cookie);
+  EXPECT_EQ(Res_value::TYPE_REFERENCE, value.dataType);
+  EXPECT_EQ(middle_ref, value.data);
+
+  // Check that resolving the reference resolves to the deepest id
+  uint32_t last_ref = high_ref;
+  assetmanager.ResolveReference(cookie, &value, &config, &flags, &last_ref);
+  EXPECT_EQ(last_ref, low_ref);
+}
+
 TEST_F(AssetManager2Test, KeepLastReferenceIdUnmodifiedIfNoReferenceIsResolved) {
   AssetManager2 assetmanager;
   assetmanager.SetApkAssets({basic_assets_.get()});

diff --git a/libs/androidfw/tests/data/basic/basic.apk b/libs/androidfw/tests/data/basic/basic.apk
index 1733b6a..b721ebf 100644 (file)
Binary files a/libs/androidfw/tests/data/basic/basic.apk and b/libs/androidfw/tests/data/basic/basic.apk differ

diff --git a/libs/androidfw/tests/data/basic/res/values/values.xml b/libs/androidfw/tests/data/basic/res/values/values.xml
index b343562..d4b2683 100644 (file)
--- a/libs/androidfw/tests/data/basic/res/values/values.xml
+++ b/libs/androidfw/tests/data/basic/res/values/values.xml
@@ -78,4 +78,8 @@
         <item type="string" name="test2" />
         <item type="array" name="integerArray1" />
     </overlayable>
+
+    <item name="high_ref" type="id">@id/middle_ref</item>
+    <item name="middle_ref" type="id">@id/low_ref</item>
+    <item name="low_ref" type="id"/>
 </resources>

diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index 1b6f882..19c6c31 100644 (file)
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -586,7 +586,29 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser,
 
     out_resource->name.type = ResourceType::kId;
     out_resource->name.entry = maybe_name.value().to_string();
-    out_resource->value = util::make_unique<Id>();
+
+    // Ids either represent a unique resource id or reference another resource id
+    auto item = ParseItem(parser, out_resource, resource_format);
+    if (!item) {
+      return false;
+    }
+
+    String* empty = ValueCast<String>(out_resource->value.get());
+    if (empty && *empty->value == "") {
+      // If no inner element exists, represent a unique identifier
+      out_resource->value = util::make_unique<Id>();
+    } else {
+      // If an inner element exists, the inner element must be a reference to
+      // another resource id
+      Reference* ref = ValueCast<Reference>(out_resource->value.get());
+      if (!ref || ref->name.value().type != ResourceType::kId) {
+        diag_->Error(DiagMessage(out_resource->source)
+                         << "<" << parser->element_name()
+                         << "> inner element must either be a resource reference or empty");
+        return false;
+      }
+    }
+
     return true;
   }
 

diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp
index fc1aeaa..c12b9fa 100644 (file)
--- a/tools/aapt2/ResourceParser_test.cpp
+++ b/tools/aapt2/ResourceParser_test.cpp
@@ -933,4 +933,32 @@ TEST_F(ResourceParserTest, DuplicateOverlayableIsError) {
   EXPECT_FALSE(TestParse(input));
 }
 
+TEST_F(ResourceParserTest, ParseIdItem) {
+  std::string input = R"(
+    <item name="foo" type="id">@id/bar</item>
+    <item name="bar" type="id"/>
+    <item name="baz" type="id"></item>)";
+  ASSERT_TRUE(TestParse(input));
+
+  ASSERT_THAT(test::GetValue<Reference>(&table_, "id/foo"), NotNull());
+  ASSERT_THAT(test::GetValue<Id>(&table_, "id/bar"), NotNull());
+  ASSERT_THAT(test::GetValue<Id>(&table_, "id/baz"), NotNull());
+
+  // Reject attribute references
+  input = R"(<item name="foo2" type="id">?attr/bar"</item>)";
+  ASSERT_FALSE(TestParse(input));
+
+  // Reject non-references
+  input = R"(<item name="foo3" type="id">0x7f010001</item>)";
+  ASSERT_FALSE(TestParse(input));
+  input = R"(<item name="foo4" type="id">@drawable/my_image</item>)";
+  ASSERT_FALSE(TestParse(input));
+  input = R"(<item name="foo5" type="id"><string name="biz"></string></item>)";
+  ASSERT_FALSE(TestParse(input));
+
+  // Ids that reference other resource ids cannot be public
+  input = R"(<public name="foo6" type="id">@id/bar6</item>)";
+  ASSERT_FALSE(TestParse(input));
+}
+
 }  // namespace aapt