AAPT2: Fix overzealous AndroidManifest fully qualified class names

ManifestFixer would go and fully qualify all elements with the attribute
'android:name', which is not correct, especially for cases like

  <uses-split android:name="foo" />

Test: make aapt2_tests
Change-Id: I4bea2550d0025179d2d48dca8c64e0cbf4451e99

diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index de4fb73..eaaefd5 100644 (file)
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -346,30 +346,15 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor,
   return true;
 }
 
-class FullyQualifiedClassNameVisitor : public xml::Visitor {
- public:
-  using xml::Visitor::Visit;
-
-  explicit FullyQualifiedClassNameVisitor(const StringPiece& package) : package_(package) {}
-
-  void Visit(xml::Element* el) override {
-    for (xml::Attribute& attr : el->attributes) {
-      if (attr.namespace_uri == xml::kSchemaAndroid &&
-          class_attributes_.find(attr.name) != class_attributes_.end()) {
-        if (Maybe<std::string> new_value = util::GetFullyQualifiedClassName(package_, attr.value)) {
-          attr.value = std::move(new_value.value());
-        }
-      }
+static void FullyQualifyClassName(const StringPiece& package, const StringPiece& attr_ns,
+                                  const StringPiece& attr_name, xml::Element* el) {
+  xml::Attribute* attr = el->FindAttribute(attr_ns, attr_name);
+  if (attr != nullptr) {
+    if (Maybe<std::string> new_value = util::GetFullyQualifiedClassName(package, attr->value)) {
+      attr->value = std::move(new_value.value());
     }
-
-    // Super implementation to iterate over the children.
-    xml::Visitor::Visit(el);
   }
-
- private:
-  StringPiece package_;
-  std::unordered_set<StringPiece> class_attributes_ = {"name"};
-};
+}
 
 static bool RenameManifestPackage(const StringPiece& package_override, xml::Element* manifest_el) {
   xml::Attribute* attr = manifest_el->FindAttribute({}, "package");
@@ -381,8 +366,25 @@ static bool RenameManifestPackage(const StringPiece& package_override, xml::Elem
   std::string original_package = std::move(attr->value);
   attr->value = package_override.to_string();
 
-  FullyQualifiedClassNameVisitor visitor(original_package);
-  manifest_el->Accept(&visitor);
+  xml::Element* application_el = manifest_el->FindChild({}, "application");
+  if (application_el != nullptr) {
+    FullyQualifyClassName(original_package, xml::kSchemaAndroid, "name", application_el);
+    FullyQualifyClassName(original_package, xml::kSchemaAndroid, "backupAgent", application_el);
+
+    for (xml::Element* child_el : application_el->GetChildElements()) {
+      if (child_el->namespace_uri.empty()) {
+        if (child_el->name == "activity" || child_el->name == "activity-alias" ||
+            child_el->name == "provider" || child_el->name == "receiver" ||
+            child_el->name == "service") {
+          FullyQualifyClassName(original_package, xml::kSchemaAndroid, "name", child_el);
+        }
+
+        if (child_el->name == "activity-alias") {
+          FullyQualifyClassName(original_package, xml::kSchemaAndroid, "targetActivity", child_el);
+        }
+      }
+    }
+  }
   return true;
 }
 

diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp
index da7f410..40085ea 100644 (file)
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -240,6 +240,7 @@ TEST_F(ManifestFixerTest, RenameManifestPackageAndFullyQualifyClasses) {
   std::unique_ptr<xml::XmlResource> doc = VerifyWithOptions(R"EOF(
       <manifest xmlns:android="http://schemas.android.com/apk/res/android"
                 package="android">
+        <uses-split android:name="feature_a" />
         <application android:name=".MainApplication" text="hello">
           <activity android:name=".activity.Start" />
           <receiver android:name="com.google.android.Receiver" />
@@ -248,35 +249,41 @@ TEST_F(ManifestFixerTest, RenameManifestPackageAndFullyQualifyClasses) {
                                                             options);
   ASSERT_NE(nullptr, doc);
 
-  xml::Element* manifestEl = doc->root.get();
-  ASSERT_NE(nullptr, manifestEl);
+  xml::Element* manifest_el = doc->root.get();
+  ASSERT_NE(nullptr, manifest_el);
 
   xml::Attribute* attr = nullptr;
 
-  attr = manifestEl->FindAttribute({}, "package");
+  attr = manifest_el->FindAttribute({}, "package");
   ASSERT_NE(nullptr, attr);
   EXPECT_EQ(std::string("com.android"), attr->value);
 
-  xml::Element* applicationEl = manifestEl->FindChild({}, "application");
-  ASSERT_NE(nullptr, applicationEl);
+  xml::Element* uses_split_el = manifest_el->FindChild({}, "uses-split");
+  ASSERT_NE(nullptr, uses_split_el);
+  attr = uses_split_el->FindAttribute(xml::kSchemaAndroid, "name");
+  ASSERT_NE(nullptr, attr);
+  EXPECT_EQ(std::string("feature_a"), attr->value);
+
+  xml::Element* application_el = manifest_el->FindChild({}, "application");
+  ASSERT_NE(nullptr, application_el);
 
-  attr = applicationEl->FindAttribute(xml::kSchemaAndroid, "name");
+  attr = application_el->FindAttribute(xml::kSchemaAndroid, "name");
   ASSERT_NE(nullptr, attr);
   EXPECT_EQ(std::string("android.MainApplication"), attr->value);
 
-  attr = applicationEl->FindAttribute({}, "text");
+  attr = application_el->FindAttribute({}, "text");
   ASSERT_NE(nullptr, attr);
   EXPECT_EQ(std::string("hello"), attr->value);
 
   xml::Element* el;
-  el = applicationEl->FindChild({}, "activity");
+  el = application_el->FindChild({}, "activity");
   ASSERT_NE(nullptr, el);
 
   attr = el->FindAttribute(xml::kSchemaAndroid, "name");
   ASSERT_NE(nullptr, el);
   EXPECT_EQ(std::string("android.activity.Start"), attr->value);
 
-  el = applicationEl->FindChild({}, "receiver");
+  el = application_el->FindChild({}, "receiver");
   ASSERT_NE(nullptr, el);
 
   attr = el->FindAttribute(xml::kSchemaAndroid, "name");