From c9a2926e661df75ba2517da166bf3b561fd678ec Mon Sep 17 00:00:00 2001
From: Adam Lesinski <adamlesinski@google.com>
Date: Thu, 1 Mar 2018 20:04:00 -0800
Subject: [PATCH] AAPT2: Allow to inline XML into custom attribute

Previously, doing something like

<parent xmlns:app="http://schemas.android.com/apk/res-auto"
        xmlns:aapt="http://schemas.android.com/aapt">
    <aapt:attr name="app:foo">
        <child />
    </aapt:attr>
</parent>

would result in something like:

<parent xmlns:app="http://schemas.android.com/apk/res-auto"
        xmlns:aapt="http://schemas.android.com/aapt"
        foo="@generated_name" />

while it should result in:

<parent xmlns:app="http://schemas.android.com/apk/res-auto"
        xmlns:aapt="http://schemas.android.com/aapt"
        app:foo="@generated_name" />

Bug: 36809755
Test: make aapt2_tests
Change-Id: I72ea4b402e196ca05b53b788e4768a265190a0dc
---
 tools/aapt2/compile/InlineXmlFormatParser.cpp | 15 +++++---
 .../compile/InlineXmlFormatParser_test.cpp    | 38 +++++++++++++++++++
 tools/aapt2/xml/XmlDom_test.cpp               | 10 ++++-
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/tools/aapt2/compile/InlineXmlFormatParser.cpp b/tools/aapt2/compile/InlineXmlFormatParser.cpp
index 238e339c05af3..79b0933a089f9 100644
--- a/tools/aapt2/compile/InlineXmlFormatParser.cpp
+++ b/tools/aapt2/compile/InlineXmlFormatParser.cpp
@@ -65,10 +65,6 @@ class Visitor : public xml::PackageAwareVisitor {
     }
 
     const ResourceName& name = ref.value().name.value();
-
-    // Use an empty string for the compilation package because we don't want to default to
-    // the local package if the user specified name="style" or something. This should just
-    // be the default namespace.
     Maybe<xml::ExtractedPackage> maybe_pkg = TransformPackageAlias(name.package);
     if (!maybe_pkg) {
       context_->GetDiagnostics()->Error(DiagMessage(src)
@@ -83,8 +79,15 @@ class Visitor : public xml::PackageAwareVisitor {
     InlineDeclaration decl;
     decl.el = el;
     decl.attr_name = name.entry;
-    if (!pkg.package.empty()) {
-      decl.attr_namespace_uri = xml::BuildPackageNamespace(pkg.package, private_namespace);
+
+    // We need to differentiate between no-namespace defined, or the alias resolves to an empty
+    // package, which means we must use the res-auto schema.
+    if (!name.package.empty()) {
+      if (pkg.package.empty()) {
+        decl.attr_namespace_uri = xml::kSchemaAuto;
+      } else {
+        decl.attr_namespace_uri = xml::BuildPackageNamespace(pkg.package, private_namespace);
+      }
     }
 
     inline_declarations_.push_back(std::move(decl));
diff --git a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp
index 2b4ab96d5c3a8..ca4e01a302154 100644
--- a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp
+++ b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp
@@ -184,4 +184,42 @@ TEST(InlineXmlFormatParserTest, ExtractNestedXmlResources) {
   // Confirm that all of the nested inline xmls are parsed out.
   ASSERT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(8u));
 }
+
+TEST(InlineXmlFormatParserTest, ExtractIntoAppAttribute) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(
+      <parent xmlns:app="http://schemas.android.com/apk/res-auto"
+              xmlns:aapt="http://schemas.android.com/aapt">
+            <aapt:attr name="app:foo">
+                <child />
+            </aapt:attr>
+      </parent>)");
+
+  doc->file.name = test::ParseNameOrDie("layout/main");
+
+  InlineXmlFormatParser parser;
+  ASSERT_TRUE(parser.Consume(context.get(), doc.get()));
+
+  ASSERT_THAT(doc->root, NotNull());
+  EXPECT_THAT(doc->root->FindAttribute(xml::kSchemaAuto, "foo"), NotNull());
+}
+
+TEST(InlineXmlFormatParserTest, ExtractIntoNoNamespaceAttribute) {
+  std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
+  std::unique_ptr<xml::XmlResource> doc = test::BuildXmlDom(R"(
+      <parent xmlns:aapt="http://schemas.android.com/aapt">
+            <aapt:attr name="foo">
+                <child />
+            </aapt:attr>
+      </parent>)");
+
+  doc->file.name = test::ParseNameOrDie("layout/main");
+
+  InlineXmlFormatParser parser;
+  ASSERT_TRUE(parser.Consume(context.get(), doc.get()));
+
+  ASSERT_THAT(doc->root, NotNull());
+  EXPECT_THAT(doc->root->FindAttribute({}, "foo"), NotNull());
+}
+
 }  // namespace aapt
diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp
index 486b53ada6bbe..ca46d539fe3c3 100644
--- a/tools/aapt2/xml/XmlDom_test.cpp
+++ b/tools/aapt2/xml/XmlDom_test.cpp
@@ -154,6 +154,12 @@ class TestVisitor : public PackageAwareVisitor {
       EXPECT_THAT(TransformPackageAlias("two"), Eq(make_value(ExtractedPackage{"com.two", false})));
       EXPECT_THAT(TransformPackageAlias("three"),
                   Eq(make_value(ExtractedPackage{"com.three", false})));
+    } else if (el->name == "View4") {
+      EXPECT_THAT(TransformPackageAlias("one"), Eq(make_value(ExtractedPackage{"com.one", false})));
+      EXPECT_THAT(TransformPackageAlias("two"), Eq(make_value(ExtractedPackage{"com.two", false})));
+      EXPECT_THAT(TransformPackageAlias("three"),
+                  Eq(make_value(ExtractedPackage{"com.three", false})));
+      EXPECT_THAT(TransformPackageAlias("four"), Eq(make_value(ExtractedPackage{"", true})));
     }
   }
 };
@@ -162,7 +168,9 @@ TEST(XmlDomTest, PackageAwareXmlVisitor) {
   std::unique_ptr<XmlResource> doc = test::BuildXmlDom(R"(
       <View1 xmlns:one="http://schemas.android.com/apk/res/com.one">
         <View2 xmlns:two="http://schemas.android.com/apk/res/com.two">
-          <View3 xmlns:three="http://schemas.android.com/apk/res/com.three" />
+          <View3 xmlns:three="http://schemas.android.com/apk/res/com.three">
+            <View4 xmlns:four="http://schemas.android.com/apk/res-auto" />
+          </View3>
         </View2>
       </View1>)");