diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp index 717978eaa226..b525758004f0 100644 --- a/tools/aapt2/link/Link.cpp +++ b/tools/aapt2/link/Link.cpp @@ -1356,8 +1356,8 @@ class LinkCommand { application_el->attributes.push_back( xml::Attribute{xml::kSchemaAndroid, "hasCode", "false"}); - manifest_el->AddChild(std::move(application_el)); - namespace_android->AddChild(std::move(manifest_el)); + manifest_el->AppendChild(std::move(application_el)); + namespace_android->AppendChild(std::move(manifest_el)); doc->root = std::move(namespace_android); return doc; } diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp index a418fc815d49..36a34941347f 100644 --- a/tools/aapt2/link/ManifestFixer.cpp +++ b/tools/aapt2/link/ManifestFixer.cpp @@ -309,10 +309,12 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { if ((options_.min_sdk_version_default || options_.target_sdk_version_default) && root->FindChild({}, "uses-sdk") == nullptr) { - // Auto insert a element. + // Auto insert a element. This must be inserted before the + // tag. The device runtime PackageParser will make SDK version + // decisions while parsing . std::unique_ptr uses_sdk = util::make_unique(); uses_sdk->name = "uses-sdk"; - root->AddChild(std::move(uses_sdk)); + root->InsertChild(0, std::move(uses_sdk)); } xml::XmlActionExecutor executor; @@ -327,7 +329,8 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) { if (options_.rename_manifest_package) { // Rename manifest package outside of the XmlActionExecutor. - // We need to extract the old package name and FullyQualify all class names. + // We need to extract the old package name and FullyQualify all class + // names. if (!RenameManifestPackage(options_.rename_manifest_package.value(), root)) { return false; diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp index 0e29ba6b6071..e9bc64acc542 100644 --- a/tools/aapt2/link/ManifestFixer_test.cpp +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -168,6 +168,50 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { EXPECT_EQ("22", attr->value); } +TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) { + ManifestFixerOptions options = {std::string("8"), std::string("22")}; + std::unique_ptr doc = VerifyWithOptions(R"EOF( + + + )EOF", + options); + ASSERT_NE(nullptr, doc); + + xml::Element* manifest_el = xml::FindRootElement(doc.get()); + ASSERT_NE(nullptr, manifest_el); + ASSERT_EQ("manifest", manifest_el->name); + + xml::Element* application_el = manifest_el->FindChild("", "application"); + ASSERT_NE(nullptr, application_el); + + xml::Element* uses_sdk_el = manifest_el->FindChild("", "uses-sdk"); + ASSERT_NE(nullptr, uses_sdk_el); + + // Check that the uses_sdk_el comes before application_el in the children + // vector. + // Since there are no namespaces here, these children are direct descendants + // of manifest. + auto uses_sdk_iter = + std::find_if(manifest_el->children.begin(), manifest_el->children.end(), + [&](const std::unique_ptr& child) { + return child.get() == uses_sdk_el; + }); + + auto application_iter = + std::find_if(manifest_el->children.begin(), manifest_el->children.end(), + [&](const std::unique_ptr& child) { + return child.get() == application_el; + }); + + ASSERT_NE(manifest_el->children.end(), uses_sdk_iter); + ASSERT_NE(manifest_el->children.end(), application_iter); + + // The distance should be positive, meaning uses_sdk_iter comes before + // application_iter. + EXPECT_GT(std::distance(uses_sdk_iter, application_iter), 0); +} + TEST_F(ManifestFixerTest, RenameManifestPackageAndFullyQualifyClasses) { ManifestFixerOptions options; options.rename_manifest_package = std::string("com.android"); diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp index 567418edf56b..960d3614305e 100644 --- a/tools/aapt2/xml/XmlDom.cpp +++ b/tools/aapt2/xml/XmlDom.cpp @@ -66,7 +66,7 @@ static void AddToStack(Stack* stack, XML_Parser parser, Node* this_node = node.get(); if (!stack->node_stack.empty()) { - stack->node_stack.top()->AddChild(std::move(node)); + stack->node_stack.top()->AppendChild(std::move(node)); } else { stack->root = std::move(node); } @@ -325,7 +325,7 @@ std::unique_ptr Inflate(const void* data, size_t data_len, root = std::move(new_node); } else { CHECK(!node_stack.empty()) << "node stack should not be empty"; - node_stack.top()->AddChild(std::move(new_node)); + node_stack.top()->AppendChild(std::move(new_node)); } if (!NodeCast(this_node)) { @@ -346,7 +346,7 @@ std::unique_ptr Namespace::Clone() { ns->children.reserve(children.size()); for (const std::unique_ptr& child : children) { - ns->AddChild(child->Clone()); + ns->AppendChild(child->Clone()); } return std::move(ns); } @@ -372,11 +372,16 @@ Element* FindRootElement(Node* node) { return el; } -void Node::AddChild(std::unique_ptr child) { +void Node::AppendChild(std::unique_ptr child) { child->parent = this; children.push_back(std::move(child)); } +void Node::InsertChild(size_t index, std::unique_ptr child) { + child->parent = this; + children.insert(children.begin() + index, std::move(child)); +} + Attribute* Element::FindAttribute(const StringPiece& ns, const StringPiece& name) { for (auto& attr : attributes) { @@ -456,7 +461,7 @@ std::unique_ptr Element::Clone() { el->children.reserve(children.size()); for (const std::unique_ptr& child : children) { - el->AddChild(child->Clone()); + el->AppendChild(child->Clone()); } return std::move(el); } diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h index e771d8735ac0..720fe357bfa1 100644 --- a/tools/aapt2/xml/XmlDom.h +++ b/tools/aapt2/xml/XmlDom.h @@ -47,7 +47,8 @@ class Node { virtual ~Node() = default; - void AddChild(std::unique_ptr child); + void AppendChild(std::unique_ptr child); + void InsertChild(size_t index, std::unique_ptr child); virtual void Accept(RawVisitor* visitor) = 0; virtual std::unique_ptr Clone() = 0; };