From 6b372991296c9f2bd6f8f8847dcd23d50970d06d Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Wed, 9 Aug 2017 10:54:23 -0700 Subject: [PATCH] AAPT2: Change XmlDom to exclude Namespace as a node In preparation for exporting an XML proto format for UAM to consume, this change brings the XML DOM API more in line with other APIs that do not make the Namespace a separate node. Treating Namespace declarations as just properties of an Element node makes the implementation of algorithms much simpler, as the constraints that Namespace nodes have only one child are now built in and traversing to find Element nodes is much simpler. Also made a bunch of quality of life improvements, like formatting and comment style. Test: make aapt2_tests Change-Id: Ib97ff1c4252b7907e2cc1f13a448dc4ca3b809a4 --- tools/aapt2/Debug.cpp | 64 ++-- tools/aapt2/cmd/Link.cpp | 2 +- tools/aapt2/cmd/Util.cpp | 27 +- tools/aapt2/compile/InlineXmlFormatParser.cpp | 108 +++---- tools/aapt2/compile/InlineXmlFormatParser.h | 41 ++- .../compile/InlineXmlFormatParser_test.cpp | 91 +++--- .../configuration/ConfigurationParser.cpp | 5 +- .../ConfigurationParser_test.cpp | 9 +- tools/aapt2/flatten/XmlFlattener.cpp | 57 ++-- tools/aapt2/java/ProguardRules.cpp | 61 ++-- tools/aapt2/link/ManifestFixer_test.cpp | 18 +- tools/aapt2/link/XmlCompatVersioner.cpp | 2 +- tools/aapt2/link/XmlCompatVersioner_test.cpp | 213 ++++++------- tools/aapt2/link/XmlNamespaceRemover.cpp | 38 +-- tools/aapt2/link/XmlNamespaceRemover_test.cpp | 42 +-- tools/aapt2/link/XmlReferenceLinker.cpp | 4 +- tools/aapt2/link/XmlReferenceLinker_test.cpp | 66 ++-- tools/aapt2/proto/ProtoHelpers.h | 15 +- tools/aapt2/test/Builders.cpp | 2 +- tools/aapt2/test/Common.h | 95 +++++- tools/aapt2/xml/XmlActionExecutor.cpp | 2 +- tools/aapt2/xml/XmlDom.cpp | 284 ++++++++---------- tools/aapt2/xml/XmlDom.h | 170 ++++++----- tools/aapt2/xml/XmlDom_test.cpp | 68 +++-- tools/aapt2/xml/XmlUtil.h | 83 ++--- 25 files changed, 788 insertions(+), 779 deletions(-) diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index b872ebbeb1598..49ed7780f9501 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -33,6 +33,8 @@ namespace aapt { +namespace { + class PrintVisitor : public ValueVisitor { public: using ValueVisitor::Visit; @@ -88,9 +90,13 @@ class PrintVisitor : public ValueVisitor { } } - void Visit(Array* array) override { array->Print(&std::cout); } + void Visit(Array* array) override { + array->Print(&std::cout); + } - void Visit(Plural* plural) override { plural->Print(&std::cout); } + void Visit(Plural* plural) override { + plural->Print(&std::cout); + } void Visit(Styleable* styleable) override { std::cout << "(styleable)"; @@ -110,11 +116,14 @@ class PrintVisitor : public ValueVisitor { } } - void VisitItem(Item* item) override { item->Print(&std::cout); } + void VisitItem(Item* item) override { + item->Print(&std::cout); + } }; -void Debug::PrintTable(ResourceTable* table, - const DebugPrintTableOptions& options) { +} // namespace + +void Debug::PrintTable(ResourceTable* table, const DebugPrintTableOptions& options) { PrintVisitor visitor; for (auto& package : table->packages) { @@ -148,10 +157,9 @@ void Debug::PrintTable(ResourceTable* table, } for (const ResourceEntry* entry : sorted_entries) { - ResourceId id(package->id ? package->id.value() : uint8_t(0), - type->id ? type->id.value() : uint8_t(0), - entry->id ? entry->id.value() : uint16_t(0)); - ResourceName name(package->name, type->type, entry->name); + const ResourceId id(package->id.value_or_default(0), type->id.value_or_default(0), + entry->id.value_or_default(0)); + const ResourceName name(package->name, type->type, entry->name); std::cout << " spec resource " << id << " " << name; switch (entry->symbol_status.state) { @@ -180,16 +188,14 @@ void Debug::PrintTable(ResourceTable* table, } } -static size_t GetNodeIndex(const std::vector& names, - const ResourceName& name) { +static size_t GetNodeIndex(const std::vector& names, const ResourceName& name) { auto iter = std::lower_bound(names.begin(), names.end(), name); CHECK(iter != names.end()); CHECK(*iter == name); return std::distance(names.begin(), iter); } -void Debug::PrintStyleGraph(ResourceTable* table, - const ResourceName& target_style) { +void Debug::PrintStyleGraph(ResourceTable* table, const ResourceName& target_style) { std::map> graph; std::queue styles_to_visit; @@ -223,8 +229,7 @@ void Debug::PrintStyleGraph(ResourceTable* table, std::cout << "digraph styles {\n"; for (const auto& name : names) { - std::cout << " node_" << GetNodeIndex(names, name) << " [label=\"" << name - << "\"];\n"; + std::cout << " node_" << GetNodeIndex(names, name) << " [label=\"" << name << "\"];\n"; } for (const auto& entry : graph) { @@ -243,8 +248,7 @@ void Debug::PrintStyleGraph(ResourceTable* table, void Debug::DumpHex(const void* data, size_t len) { const uint8_t* d = (const uint8_t*)data; for (size_t i = 0; i < len; i++) { - std::cerr << std::hex << std::setfill('0') << std::setw(2) << (uint32_t)d[i] - << " "; + std::cerr << std::hex << std::setfill('0') << std::setw(2) << (uint32_t)d[i] << " "; if (i % 8 == 7) { std::cerr << "\n"; } @@ -262,8 +266,15 @@ class XmlPrinter : public xml::Visitor { using xml::Visitor::Visit; void Visit(xml::Element* el) override { - std::cerr << prefix_; - std::cerr << "E: "; + const size_t previous_size = prefix_.size(); + + for (const xml::NamespaceDecl& decl : el->namespace_decls) { + std::cerr << prefix_ << "N: " << decl.prefix << "=" << decl.uri + << " (line=" << decl.line_number << ")\n"; + prefix_ += " "; + } + + std::cerr << prefix_ << "E: "; if (!el->namespace_uri.empty()) { std::cerr << el->namespace_uri << ":"; } @@ -283,26 +294,13 @@ class XmlPrinter : public xml::Visitor { std::cerr << "=" << attr.value << "\n"; } - const size_t previous_size = prefix_.size(); prefix_ += " "; xml::Visitor::Visit(el); prefix_.resize(previous_size); } - void Visit(xml::Namespace* ns) override { - std::cerr << prefix_; - std::cerr << "N: " << ns->namespace_prefix << "=" << ns->namespace_uri - << " (line=" << ns->line_number << ")\n"; - - const size_t previous_size = prefix_.size(); - prefix_ += " "; - xml::Visitor::Visit(ns); - prefix_.resize(previous_size); - } - void Visit(xml::Text* text) override { - std::cerr << prefix_; - std::cerr << "T: '" << text->text << "'\n"; + std::cerr << prefix_ << "T: '" << text->text << "'\n"; } private: diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 9005c5bf1a19e..db83d0074ba51 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -482,7 +482,7 @@ std::vector> ResourceFileFlattener::LinkAndVer if (options_.no_version_vectors || options_.no_version_transitions) { // Skip this if it is a vector or animated-vector. - xml::Element* el = xml::FindRootElement(doc); + xml::Element* el = doc->root.get(); if (el && el->namespace_uri.empty()) { if ((options_.no_version_vectors && IsVectorElement(el->name)) || (options_.no_version_transitions && IsTransitionElement(el->name))) { diff --git a/tools/aapt2/cmd/Util.cpp b/tools/aapt2/cmd/Util.cpp index e1c45d68f6115..d17858d45d08a 100644 --- a/tools/aapt2/cmd/Util.cpp +++ b/tools/aapt2/cmd/Util.cpp @@ -28,7 +28,7 @@ #include "util/Maybe.h" #include "util/Util.h" -using android::StringPiece; +using ::android::StringPiece; namespace aapt { @@ -134,19 +134,21 @@ static xml::AaptAttribute CreateAttributeWithId(const ResourceId& id) { return xml::AaptAttribute(Attribute(), id); } +static xml::NamespaceDecl CreateAndroidNamespaceDecl() { + xml::NamespaceDecl decl; + decl.prefix = "android"; + decl.uri = xml::kSchemaAndroid; + return decl; +} + std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, const SplitConstraints& constraints) { const ResourceId kVersionCode(0x0101021b); const ResourceId kRevisionCode(0x010104d5); const ResourceId kHasCode(0x0101000c); - std::unique_ptr doc = util::make_unique(); - - std::unique_ptr namespace_android = util::make_unique(); - namespace_android->namespace_uri = xml::kSchemaAndroid; - namespace_android->namespace_prefix = "android"; - std::unique_ptr manifest_el = util::make_unique(); + manifest_el->namespace_decls.push_back(CreateAndroidNamespaceDecl()); manifest_el->name = "manifest"; manifest_el->attributes.push_back(xml::Attribute{"", "package", app_info.package}); @@ -179,8 +181,8 @@ std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, xml::Attribute{"", "configForSplit", app_info.split_name.value()}); } - // Splits may contain more configurations than originally desired (fallback densities, etc.). - // This makes programmatic discovery of split targetting difficult. Encode the original + // Splits may contain more configurations than originally desired (fall-back densities, etc.). + // This makes programmatic discovery of split targeting difficult. Encode the original // split constraints intended for this split. std::stringstream target_config_str; target_config_str << util::Joiner(constraints.configs, ","); @@ -193,8 +195,9 @@ std::unique_ptr GenerateSplitManifest(const AppInfo& app_info, util::make_unique(android::Res_value::TYPE_INT_BOOLEAN, 0u)}); manifest_el->AppendChild(std::move(application_el)); - namespace_android->AppendChild(std::move(manifest_el)); - doc->root = std::move(namespace_android); + + std::unique_ptr doc = util::make_unique(); + doc->root = std::move(manifest_el); return doc; } @@ -284,7 +287,7 @@ static Maybe ExtractSdkVersion(xml::Attribute* attr, std::string* out_error Maybe ExtractAppInfoFromBinaryManifest(xml::XmlResource* xml_res, IDiagnostics* diag) { // Make sure the first element is with package attribute. - xml::Element* manifest_el = xml::FindRootElement(xml_res->root.get()); + xml::Element* manifest_el = xml_res->root.get(); if (manifest_el == nullptr) { return {}; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser.cpp b/tools/aapt2/compile/InlineXmlFormatParser.cpp index 786494b6ad1c3..857cdd5365a0c 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser.cpp +++ b/tools/aapt2/compile/InlineXmlFormatParser.cpp @@ -16,12 +16,8 @@ #include "compile/InlineXmlFormatParser.h" -#include #include -#include "android-base/macros.h" - -#include "Debug.h" #include "ResourceUtils.h" #include "util/Util.h" #include "xml/XmlDom.h" @@ -31,19 +27,17 @@ namespace aapt { namespace { -/** - * XML Visitor that will find all elements for extraction. - */ +struct InlineDeclaration { + xml::Element* el; + std::string attr_namespace_uri; + std::string attr_name; +}; + +// XML Visitor that will find all elements for extraction. class Visitor : public xml::PackageAwareVisitor { public: using xml::PackageAwareVisitor::Visit; - struct InlineDeclaration { - xml::Element* el; - std::string attr_namespace_uri; - std::string attr_name; - }; - explicit Visitor(IAaptContext* context, xml::XmlResource* xml_resource) : context_(context), xml_resource_(xml_resource) {} @@ -53,51 +47,44 @@ class Visitor : public xml::PackageAwareVisitor { return; } - const Source& src = xml_resource_->file.source.WithLine(el->line_number); + const Source src = xml_resource_->file.source.WithLine(el->line_number); xml::Attribute* attr = el->FindAttribute({}, "name"); if (!attr) { - context_->GetDiagnostics()->Error(DiagMessage(src) - << "missing 'name' attribute"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "missing 'name' attribute"); error_ = true; return; } Maybe ref = ResourceUtils::ParseXmlAttributeName(attr->value); if (!ref) { - context_->GetDiagnostics()->Error( - DiagMessage(src) << "invalid XML attribute '" << attr->value << "'"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "invalid XML attribute '" << attr->value + << "'"); error_ = true; return; } 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 + // 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 maybe_pkg = - TransformPackageAlias(name.package, {}); + Maybe maybe_pkg = TransformPackageAlias(name.package, {}); if (!maybe_pkg) { - context_->GetDiagnostics()->Error(DiagMessage(src) - << "invalid namespace prefix '" - << name.package << "'"); + context_->GetDiagnostics()->Error(DiagMessage(src) << "invalid namespace prefix '" + << name.package << "'"); error_ = true; return; } const xml::ExtractedPackage& pkg = maybe_pkg.value(); - const bool private_namespace = - pkg.private_namespace || ref.value().private_reference; + const bool private_namespace = pkg.private_namespace || ref.value().private_reference; InlineDeclaration decl; decl.el = el; decl.attr_name = name.entry; if (!pkg.package.empty()) { - decl.attr_namespace_uri = - xml::BuildPackageNamespace(pkg.package, private_namespace); + decl.attr_namespace_uri = xml::BuildPackageNamespace(pkg.package, private_namespace); } inline_declarations_.push_back(std::move(decl)); @@ -107,7 +94,9 @@ class Visitor : public xml::PackageAwareVisitor { return inline_declarations_; } - bool HasError() const { return error_; } + bool HasError() const { + return error_; + } private: DISALLOW_COPY_AND_ASSIGN(Visitor); @@ -120,8 +109,7 @@ class Visitor : public xml::PackageAwareVisitor { } // namespace -bool InlineXmlFormatParser::Consume(IAaptContext* context, - xml::XmlResource* doc) { +bool InlineXmlFormatParser::Consume(IAaptContext* context, xml::XmlResource* doc) { Visitor visitor(context, doc); doc->root->Accept(&visitor); if (visitor.HasError()) { @@ -129,69 +117,53 @@ bool InlineXmlFormatParser::Consume(IAaptContext* context, } size_t name_suffix_counter = 0; - for (const Visitor::InlineDeclaration& decl : - visitor.GetInlineDeclarations()) { + for (const InlineDeclaration& decl : visitor.GetInlineDeclarations()) { auto new_doc = util::make_unique(); new_doc->file.config = doc->file.config; new_doc->file.source = doc->file.source.WithLine(decl.el->line_number); new_doc->file.name = doc->file.name; // Modify the new entry name. We need to suffix the entry with a number to - // avoid - // local collisions, then mangle it with the empty package, such that it - // won't show up + // avoid local collisions, then mangle it with the empty package, such that it won't show up // in R.java. - - new_doc->file.name.entry = - NameMangler::MangleEntry({}, new_doc->file.name.entry + "__" + - std::to_string(name_suffix_counter)); + new_doc->file.name.entry = NameMangler::MangleEntry( + {}, new_doc->file.name.entry + "__" + std::to_string(name_suffix_counter)); // Extracted elements must be the only child of . // Make sure there is one root node in the children (ignore empty text). - for (auto& child : decl.el->children) { + for (std::unique_ptr& child : decl.el->children) { const Source child_source = doc->file.source.WithLine(child->line_number); if (xml::Text* t = xml::NodeCast(child.get())) { if (!util::TrimWhitespace(t->text).empty()) { - context->GetDiagnostics()->Error( - DiagMessage(child_source) - << "can't extract text into its own resource"); + context->GetDiagnostics()->Error(DiagMessage(child_source) + << "can't extract text into its own resource"); return false; } } else if (new_doc->root) { - context->GetDiagnostics()->Error( - DiagMessage(child_source) - << "inline XML resources must have a single root"); + context->GetDiagnostics()->Error(DiagMessage(child_source) + << "inline XML resources must have a single root"); return false; } else { - new_doc->root = std::move(child); + new_doc->root.reset(static_cast(child.release())); new_doc->root->parent = nullptr; } } - // Walk up and find the parent element. - xml::Node* node = decl.el; - xml::Element* parent_el = nullptr; - while (node->parent && - (parent_el = xml::NodeCast(node->parent)) == nullptr) { - node = node->parent; - } - + // Get the parent element of + xml::Element* parent_el = decl.el->parent; if (!parent_el) { - context->GetDiagnostics()->Error( - DiagMessage(new_doc->file.source) - << "no suitable parent for inheriting attribute"); + context->GetDiagnostics()->Error(DiagMessage(new_doc->file.source) + << "no suitable parent for inheriting attribute"); return false; } // Add the inline attribute to the parent. - parent_el->attributes.push_back( - xml::Attribute{decl.attr_namespace_uri, decl.attr_name, - "@" + new_doc->file.name.ToString()}); + parent_el->attributes.push_back(xml::Attribute{decl.attr_namespace_uri, decl.attr_name, + "@" + new_doc->file.name.ToString()}); // Delete the subtree. - for (auto iter = parent_el->children.begin(); - iter != parent_el->children.end(); ++iter) { - if (iter->get() == node) { + for (auto iter = parent_el->children.begin(); iter != parent_el->children.end(); ++iter) { + if (iter->get() == decl.el) { parent_el->children.erase(iter); break; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser.h b/tools/aapt2/compile/InlineXmlFormatParser.h index 1a658fd6a1808..4300023e77267 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser.h +++ b/tools/aapt2/compile/InlineXmlFormatParser.h @@ -26,35 +26,30 @@ namespace aapt { -/** - * Extracts Inline XML definitions into their own xml::XmlResource objects. - * - * Inline XML looks like: - * - * - * - * - * - * - * - * The will be extracted into its own XML file and - * will - * gain an attribute 'android:drawable' set to a reference to the extracted - * resource. - */ +// Extracts Inline XML definitions into their own xml::XmlResource objects. +// +// Inline XML looks like: +// +// +// +// +// +// +// +// The will be extracted into its own XML file and will +// gain an attribute 'android:drawable' set to a reference to the extracted resource. class InlineXmlFormatParser : public IXmlResourceConsumer { public: explicit InlineXmlFormatParser() = default; bool Consume(IAaptContext* context, xml::XmlResource* doc) override; - std::vector>& - GetExtractedInlineXmlDocuments() { + std::vector>& GetExtractedInlineXmlDocuments() { return queue_; } diff --git a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp index 348796c98c227..de7739ada407a 100644 --- a/tools/aapt2/compile/InlineXmlFormatParser_test.cpp +++ b/tools/aapt2/compile/InlineXmlFormatParser_test.cpp @@ -18,25 +18,32 @@ #include "test/Test.h" +using ::testing::Eq; +using ::testing::IsNull; +using ::testing::Not; +using ::testing::NotNull; +using ::testing::SizeIs; +using ::testing::StrEq; + namespace aapt { TEST(InlineXmlFormatParserTest, PassThrough) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( - )EOF"); + )"); InlineXmlFormatParser parser; ASSERT_TRUE(parser.Consume(context.get(), doc.get())); - EXPECT_EQ(0u, parser.GetExtractedInlineXmlDocuments().size()); + EXPECT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(0u)); } TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( @@ -44,7 +51,7 @@ TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { - )EOF"); + )"); doc->file.name = test::ParseNameOrDie("layout/main"); @@ -52,42 +59,38 @@ TEST(InlineXmlFormatParserTest, ExtractOneXmlResource) { ASSERT_TRUE(parser.Consume(context.get(), doc.get())); // One XML resource should have been extracted. - EXPECT_EQ(1u, parser.GetExtractedInlineXmlDocuments().size()); + EXPECT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(1u)); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_NE(nullptr, el); - - EXPECT_EQ("View1", el->name); + xml::Element* el = doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View1")); // The tag should be extracted. - EXPECT_EQ(nullptr, el->FindChild(xml::kSchemaAapt, "attr")); + EXPECT_THAT(el->FindChild(xml::kSchemaAapt, "attr"), IsNull()); // The 'android:text' attribute should be set with a reference. xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "text"); - ASSERT_NE(nullptr, attr); + ASSERT_THAT(attr, NotNull()); ResourceNameRef name_ref; ASSERT_TRUE(ResourceUtils::ParseReference(attr->value, &name_ref)); - xml::XmlResource* extracted_doc = - parser.GetExtractedInlineXmlDocuments()[0].get(); - ASSERT_NE(nullptr, extracted_doc); + xml::XmlResource* extracted_doc = parser.GetExtractedInlineXmlDocuments()[0].get(); + ASSERT_THAT(extracted_doc, NotNull()); // Make sure the generated reference is correct. - EXPECT_EQ(name_ref.package, extracted_doc->file.name.package); - EXPECT_EQ(name_ref.type, extracted_doc->file.name.type); - EXPECT_EQ(name_ref.entry, extracted_doc->file.name.entry); + EXPECT_THAT(extracted_doc->file.name, Eq(name_ref)); // Verify the structure of the extracted XML. - el = xml::FindRootElement(extracted_doc); - ASSERT_NE(nullptr, el); - EXPECT_EQ("View2", el->name); - EXPECT_NE(nullptr, el->FindChild({}, "View3")); + el = extracted_doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View2")); + EXPECT_THAT(el->FindChild({}, "View3"), NotNull()); } TEST(InlineXmlFormatParserTest, ExtractTwoXmlResources) { std::unique_ptr context = test::ContextBuilder().Build(); - std::unique_ptr doc = test::BuildXmlDom(R"EOF( + std::unique_ptr doc = test::BuildXmlDom(R"( @@ -99,45 +102,39 @@ TEST(InlineXmlFormatParserTest, ExtractTwoXmlResources) { - )EOF"); + )"); doc->file.name = test::ParseNameOrDie("layout/main"); InlineXmlFormatParser parser; ASSERT_TRUE(parser.Consume(context.get(), doc.get())); - ASSERT_EQ(2u, parser.GetExtractedInlineXmlDocuments().size()); + ASSERT_THAT(parser.GetExtractedInlineXmlDocuments(), SizeIs(2u)); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_NE(nullptr, el); - - EXPECT_EQ("View1", el->name); + xml::Element* el = doc->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->name, StrEq("View1")); xml::Attribute* attr_text = el->FindAttribute(xml::kSchemaAndroid, "text"); - ASSERT_NE(nullptr, attr_text); + ASSERT_THAT(attr_text, NotNull()); - xml::Attribute* attr_drawable = - el->FindAttribute(xml::kSchemaAndroid, "drawable"); - ASSERT_NE(nullptr, attr_drawable); + xml::Attribute* attr_drawable = el->FindAttribute(xml::kSchemaAndroid, "drawable"); + ASSERT_THAT(attr_drawable, NotNull()); // The two extracted resources should have different names. - EXPECT_NE(attr_text->value, attr_drawable->value); + EXPECT_THAT(attr_text->value, Not(Eq(attr_drawable->value))); // The child elements should be gone. - EXPECT_EQ(nullptr, el->FindChild(xml::kSchemaAapt, "attr")); + EXPECT_THAT(el->FindChild(xml::kSchemaAapt, "attr"), IsNull()); - xml::XmlResource* extracted_doc_text = - parser.GetExtractedInlineXmlDocuments()[0].get(); - ASSERT_NE(nullptr, extracted_doc_text); - el = xml::FindRootElement(extracted_doc_text); - ASSERT_NE(nullptr, el); - EXPECT_EQ("View2", el->name); + xml::XmlResource* extracted_doc_text = parser.GetExtractedInlineXmlDocuments()[0].get(); + ASSERT_THAT(extracted_doc_text, NotNull()); + ASSERT_THAT(extracted_doc_text->root, NotNull()); + EXPECT_THAT(extracted_doc_text->root->name, StrEq("View2")); - xml::XmlResource* extracted_doc_drawable = - parser.GetExtractedInlineXmlDocuments()[1].get(); - ASSERT_NE(nullptr, extracted_doc_drawable); - el = xml::FindRootElement(extracted_doc_drawable); - ASSERT_NE(nullptr, el); - EXPECT_EQ("vector", el->name); + xml::XmlResource* extracted_doc_drawable = parser.GetExtractedInlineXmlDocuments()[1].get(); + ASSERT_THAT(extracted_doc_drawable, NotNull()); + ASSERT_THAT(extracted_doc_drawable->root, NotNull()); + EXPECT_THAT(extracted_doc_drawable->root->name, StrEq("vector")); } } // namespace aapt diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp index b0ed792215df3..7ab690ce57891 100644 --- a/tools/aapt2/configuration/ConfigurationParser.cpp +++ b/tools/aapt2/configuration/ConfigurationParser.cpp @@ -53,7 +53,6 @@ using ::aapt::io::RegularFile; using ::aapt::io::StringInputStream; using ::aapt::util::TrimWhitespace; using ::aapt::xml::Element; -using ::aapt::xml::FindRootElement; using ::aapt::xml::NodeCast; using ::aapt::xml::XmlActionExecutor; using ::aapt::xml::XmlActionExecutorPolicy; @@ -197,13 +196,13 @@ ConfigurationParser::ConfigurationParser(std::string contents) Maybe ConfigurationParser::Parse() { StringInputStream in(contents_); - auto doc = xml::Inflate(&in, diag_, Source("config.xml")); + std::unique_ptr doc = xml::Inflate(&in, diag_, Source("config.xml")); if (!doc) { return {}; } // Strip any namespaces from the XML as the XmlActionExecutor ignores anything with a namespace. - auto* root = FindRootElement(doc.get()); + Element* root = doc->root.get(); if (root == nullptr) { diag_->Error(DiagMessage() << "Could not find the root element in the XML document"); return {}; diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp index ab3b7ec2003b2..34a6537d9f2a9 100644 --- a/tools/aapt2/configuration/ConfigurationParser_test.cpp +++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp @@ -18,9 +18,6 @@ #include -#include -#include - #include "androidfw/ResourceTypes.h" #include "test/Test.h" @@ -29,7 +26,7 @@ namespace aapt { namespace { -using android::ResTable_config; +using ::android::ResTable_config; using configuration::Abi; using configuration::AndroidSdk; using configuration::Artifact; @@ -38,7 +35,7 @@ using configuration::DeviceFeature; using configuration::GlTexture; using configuration::Locale; using configuration::AndroidManifest; -using testing::ElementsAre; +using ::testing::ElementsAre; using xml::Element; using xml::NodeCast; @@ -192,7 +189,7 @@ TEST_F(ConfigurationParserTest, ArtifactAction) { auto doc = test::BuildXmlDom(xml); PostProcessingConfiguration config; - bool ok = artifact_handler_(&config, NodeCast(doc.get()->root.get()), &diag_); + bool ok = artifact_handler_(&config, NodeCast(doc->root.get()), &diag_); ASSERT_TRUE(ok); EXPECT_EQ(1ul, config.artifacts.size()); diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp index 331ef784a7da0..b3b308a29fc50 100644 --- a/tools/aapt2/flatten/XmlFlattener.cpp +++ b/tools/aapt2/flatten/XmlFlattener.cpp @@ -38,12 +38,10 @@ namespace { constexpr uint32_t kLowPriority = 0xffffffffu; -static bool cmp_xml_attribute_by_id(const xml::Attribute* a, - const xml::Attribute* b) { +static bool cmp_xml_attribute_by_id(const xml::Attribute* a, const xml::Attribute* b) { if (a->compiled_attribute && a->compiled_attribute.value().id) { if (b->compiled_attribute && b->compiled_attribute.value().id) { - return a->compiled_attribute.value().id.value() < - b->compiled_attribute.value().id.value(); + return a->compiled_attribute.value().id.value() < b->compiled_attribute.value().id.value(); } return true; } else if (!b->compiled_attribute) { @@ -75,17 +73,6 @@ class XmlFlattenerVisitor : public xml::Visitor { XmlFlattenerVisitor(BigBuffer* buffer, XmlFlattenerOptions options) : buffer_(buffer), options_(options) {} - void Visit(xml::Namespace* node) override { - if (node->namespace_uri == xml::kSchemaTools) { - // Skip dedicated tools namespace. - xml::Visitor::Visit(node); - } else { - WriteNamespace(node, android::RES_XML_START_NAMESPACE_TYPE); - xml::Visitor::Visit(node); - WriteNamespace(node, android::RES_XML_END_NAMESPACE_TYPE); - } - } - void Visit(xml::Text* node) override { if (util::TrimWhitespace(node->text).empty()) { // Skip whitespace only text nodes. @@ -93,8 +80,7 @@ class XmlFlattenerVisitor : public xml::Visitor { } ChunkWriter writer(buffer_); - ResXMLTree_node* flat_node = - writer.StartChunk(RES_XML_CDATA_TYPE); + ResXMLTree_node* flat_node = writer.StartChunk(RES_XML_CDATA_TYPE); flat_node->lineNumber = util::HostToDevice32(node->line_number); flat_node->comment.index = util::HostToDevice32(-1); @@ -109,6 +95,13 @@ class XmlFlattenerVisitor : public xml::Visitor { } void Visit(xml::Element* node) override { + for (const xml::NamespaceDecl& decl : node->namespace_decls) { + // Skip dedicated tools namespace. + if (decl.uri != xml::kSchemaTools) { + WriteNamespace(decl, android::RES_XML_START_NAMESPACE_TYPE); + } + } + { ChunkWriter start_writer(buffer_); ResXMLTree_node* flat_node = @@ -116,19 +109,15 @@ class XmlFlattenerVisitor : public xml::Visitor { flat_node->lineNumber = util::HostToDevice32(node->line_number); flat_node->comment.index = util::HostToDevice32(-1); - ResXMLTree_attrExt* flat_elem = - start_writer.NextBlock(); + ResXMLTree_attrExt* flat_elem = start_writer.NextBlock(); - // A missing namespace must be null, not an empty string. Otherwise the - // runtime complains. + // A missing namespace must be null, not an empty string. Otherwise the runtime complains. AddString(node->namespace_uri, kLowPriority, &flat_elem->ns, true /* treat_empty_string_as_null */); - AddString(node->name, kLowPriority, &flat_elem->name, - true /* treat_empty_string_as_null */); + AddString(node->name, kLowPriority, &flat_elem->name, true /* treat_empty_string_as_null */); flat_elem->attributeStart = util::HostToDevice16(sizeof(*flat_elem)); - flat_elem->attributeSize = - util::HostToDevice16(sizeof(ResXMLTree_attribute)); + flat_elem->attributeSize = util::HostToDevice16(sizeof(ResXMLTree_attribute)); WriteAttributes(node, flat_elem, &start_writer); @@ -144,14 +133,20 @@ class XmlFlattenerVisitor : public xml::Visitor { flat_end_node->lineNumber = util::HostToDevice32(node->line_number); flat_end_node->comment.index = util::HostToDevice32(-1); - ResXMLTree_endElementExt* flat_end_elem = - end_writer.NextBlock(); + ResXMLTree_endElementExt* flat_end_elem = end_writer.NextBlock(); AddString(node->namespace_uri, kLowPriority, &flat_end_elem->ns, true /* treat_empty_string_as_null */); AddString(node->name, kLowPriority, &flat_end_elem->name); end_writer.Finish(); } + + for (auto iter = node->namespace_decls.rbegin(); iter != node->namespace_decls.rend(); ++iter) { + // Skip dedicated tools namespace. + if (iter->uri != xml::kSchemaTools) { + WriteNamespace(*iter, android::RES_XML_END_NAMESPACE_TYPE); + } + } } private: @@ -173,16 +168,16 @@ class XmlFlattenerVisitor : public xml::Visitor { string_refs.push_back(StringFlattenDest{ref, dest}); } - void WriteNamespace(xml::Namespace* node, uint16_t type) { + void WriteNamespace(const xml::NamespaceDecl& decl, uint16_t type) { ChunkWriter writer(buffer_); ResXMLTree_node* flatNode = writer.StartChunk(type); - flatNode->lineNumber = util::HostToDevice32(node->line_number); + flatNode->lineNumber = util::HostToDevice32(decl.line_number); flatNode->comment.index = util::HostToDevice32(-1); ResXMLTree_namespaceExt* flat_ns = writer.NextBlock(); - AddString(node->namespace_prefix, kLowPriority, &flat_ns->prefix); - AddString(node->namespace_uri, kLowPriority, &flat_ns->uri); + AddString(decl.prefix, kLowPriority, &flat_ns->prefix); + AddString(decl.uri, kLowPriority, &flat_ns->uri); writer.Finish(); } diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp index 5f61faeeebe75..10c46101123cc 100644 --- a/tools/aapt2/java/ProguardRules.cpp +++ b/tools/aapt2/java/ProguardRules.cpp @@ -29,18 +29,12 @@ namespace proguard { class BaseVisitor : public xml::Visitor { public: - BaseVisitor(const Source& source, KeepSet* keep_set) - : source_(source), keep_set_(keep_set) {} + using xml::Visitor::Visit; - virtual void Visit(xml::Text*) override{}; - - virtual void Visit(xml::Namespace* node) override { - for (const auto& child : node->children) { - child->Accept(this); - } + BaseVisitor(const Source& source, KeepSet* keep_set) : source_(source), keep_set_(keep_set) { } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (!node->namespace_uri.empty()) { Maybe maybe_package = xml::ExtractPackageFromNamespace(node->namespace_uri); @@ -78,10 +72,10 @@ class BaseVisitor : public xml::Visitor { class LayoutVisitor : public BaseVisitor { public: - LayoutVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + LayoutVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_class = false; bool check_name = false; if (node->namespace_uri.empty()) { @@ -119,7 +113,7 @@ class MenuVisitor : public BaseVisitor { MenuVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (node->namespace_uri.empty() && node->name == "item") { for (const auto& attr : node->attributes) { if (attr.namespace_uri == xml::kSchemaAndroid) { @@ -142,10 +136,10 @@ class MenuVisitor : public BaseVisitor { class XmlResourceVisitor : public BaseVisitor { public: - XmlResourceVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + XmlResourceVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_fragment = false; if (node->namespace_uri.empty()) { check_fragment = @@ -169,13 +163,12 @@ class XmlResourceVisitor : public BaseVisitor { class TransitionVisitor : public BaseVisitor { public: - TransitionVisitor(const Source& source, KeepSet* keep_set) - : BaseVisitor(source, keep_set) {} + TransitionVisitor(const Source& source, KeepSet* keep_set) : BaseVisitor(source, keep_set) { + } - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { bool check_class = - node->namespace_uri.empty() && - (node->name == "transition" || node->name == "pathMotion"); + node->namespace_uri.empty() && (node->name == "transition" || node->name == "pathMotion"); if (check_class) { xml::Attribute* attr = node->FindAttribute({}, "class"); if (attr && util::IsJavaClassName(attr->value)) { @@ -195,7 +188,7 @@ class ManifestVisitor : public BaseVisitor { ManifestVisitor(const Source& source, KeepSet* keep_set, bool main_dex_only) : BaseVisitor(source, keep_set), main_dex_only_(main_dex_only) {} - virtual void Visit(xml::Element* node) override { + void Visit(xml::Element* node) override { if (node->namespace_uri.empty()) { bool get_name = false; if (node->name == "manifest") { @@ -205,18 +198,15 @@ class ManifestVisitor : public BaseVisitor { } } else if (node->name == "application") { get_name = true; - xml::Attribute* attr = - node->FindAttribute(xml::kSchemaAndroid, "backupAgent"); + xml::Attribute* attr = node->FindAttribute(xml::kSchemaAndroid, "backupAgent"); if (attr) { - Maybe result = - util::GetFullyQualifiedClassName(package_, attr->value); + Maybe result = util::GetFullyQualifiedClassName(package_, attr->value); if (result) { AddClass(node->line_number, result.value()); } } if (main_dex_only_) { - xml::Attribute* default_process = - node->FindAttribute(xml::kSchemaAndroid, "process"); + xml::Attribute* default_process = node->FindAttribute(xml::kSchemaAndroid, "process"); if (default_process) { default_process_ = default_process->value; } @@ -226,8 +216,7 @@ class ManifestVisitor : public BaseVisitor { get_name = true; if (main_dex_only_) { - xml::Attribute* component_process = - node->FindAttribute(xml::kSchemaAndroid, "process"); + xml::Attribute* component_process = node->FindAttribute(xml::kSchemaAndroid, "process"); const std::string& process = component_process ? component_process->value : default_process_; @@ -242,8 +231,7 @@ class ManifestVisitor : public BaseVisitor { get_name = attr != nullptr; if (get_name) { - Maybe result = - util::GetFullyQualifiedClassName(package_, attr->value); + Maybe result = util::GetFullyQualifiedClassName(package_, attr->value); if (result) { AddClass(node->line_number, result.value()); } @@ -261,8 +249,7 @@ class ManifestVisitor : public BaseVisitor { std::string default_process_; }; -bool CollectProguardRulesForManifest(const Source& source, - xml::XmlResource* res, KeepSet* keep_set, +bool CollectProguardRulesForManifest(const Source& source, xml::XmlResource* res, KeepSet* keep_set, bool main_dex_only) { ManifestVisitor visitor(source, keep_set, main_dex_only); if (res->root) { @@ -272,8 +259,7 @@ bool CollectProguardRulesForManifest(const Source& source, return false; } -bool CollectProguardRules(const Source& source, xml::XmlResource* res, - KeepSet* keep_set) { +bool CollectProguardRules(const Source& source, xml::XmlResource* res, KeepSet* keep_set) { if (!res->root) { return false; } @@ -321,8 +307,7 @@ bool WriteKeepSet(std::ostream* out, const KeepSet& keep_set) { for (const Source& source : entry.second) { *out << "# Referenced at " << source << "\n"; } - *out << "-keepclassmembers class * { *** " << entry.first << "(...); }\n" - << std::endl; + *out << "-keepclassmembers class * { *** " << entry.first << "(...); }\n" << std::endl; } return true; } diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp index 80edb352f42c2..da7f410b8b086 100644 --- a/tools/aapt2/link/ManifestFixer_test.cpp +++ b/tools/aapt2/link/ManifestFixer_test.cpp @@ -122,7 +122,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { xml::Element* el; xml::Attribute* attr; - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -141,7 +141,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -160,7 +160,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -177,7 +177,7 @@ TEST_F(ManifestFixerTest, UseDefaultSdkVersionsIfNonePresent) { options); ASSERT_NE(nullptr, doc); - el = xml::FindRootElement(doc.get()); + el = doc->root.get(); ASSERT_NE(nullptr, el); el = el->FindChild({}, "uses-sdk"); ASSERT_NE(nullptr, el); @@ -199,7 +199,7 @@ TEST_F(ManifestFixerTest, UsesSdkMustComeBeforeApplication) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); ASSERT_EQ("manifest", manifest_el->name); @@ -248,7 +248,7 @@ TEST_F(ManifestFixerTest, RenameManifestPackageAndFullyQualifyClasses) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifestEl = xml::FindRootElement(doc.get()); + xml::Element* manifestEl = doc->root.get(); ASSERT_NE(nullptr, manifestEl); xml::Attribute* attr = nullptr; @@ -297,7 +297,7 @@ TEST_F(ManifestFixerTest, options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); xml::Element* instrumentation_el = @@ -321,7 +321,7 @@ TEST_F(ManifestFixerTest, UseDefaultVersionNameAndCode) { options); ASSERT_NE(nullptr, doc); - xml::Element* manifest_el = xml::FindRootElement(doc.get()); + xml::Element* manifest_el = doc->root.get(); ASSERT_NE(nullptr, manifest_el); xml::Attribute* attr = @@ -344,7 +344,7 @@ TEST_F(ManifestFixerTest, EnsureManifestAttributesAreTyped) { Verify(""); ASSERT_NE(nullptr, doc); - xml::Element* el = xml::FindRootElement(doc.get()); + xml::Element* el = doc->root.get(); ASSERT_NE(nullptr, el); EXPECT_EQ("manifest", el->name); diff --git a/tools/aapt2/link/XmlCompatVersioner.cpp b/tools/aapt2/link/XmlCompatVersioner.cpp index f1f4e3b7f05ff..20ebdc696814d 100644 --- a/tools/aapt2/link/XmlCompatVersioner.cpp +++ b/tools/aapt2/link/XmlCompatVersioner.cpp @@ -107,7 +107,7 @@ std::unique_ptr XmlCompatVersioner::ProcessDoc( std::unique_ptr cloned_doc = util::make_unique(doc->file); cloned_doc->file.config.sdkVersion = static_cast(target_api); - cloned_doc->root = doc->root->Clone([&](const xml::Element& el, xml::Element* out_el) { + cloned_doc->root = doc->root->CloneElement([&](const xml::Element& el, xml::Element* out_el) { for (const auto& attr : el.attributes) { if (!attr.compiled_attribute) { // Just copy if this isn't a compiled attribute. diff --git a/tools/aapt2/link/XmlCompatVersioner_test.cpp b/tools/aapt2/link/XmlCompatVersioner_test.cpp index ce6605c6ad97d..29ad25f79ab98 100644 --- a/tools/aapt2/link/XmlCompatVersioner_test.cpp +++ b/tools/aapt2/link/XmlCompatVersioner_test.cpp @@ -19,6 +19,12 @@ #include "Linkers.h" #include "test/Test.h" +using ::aapt::test::ValueEq; +using ::testing::Eq; +using ::testing::IsNull; +using ::testing::NotNull; +using ::testing::SizeIs; + namespace aapt { constexpr auto TYPE_DIMENSION = android::ResTable_map::TYPE_DIMENSION; @@ -68,12 +74,12 @@ class XmlCompatVersionerTest : public ::testing::Test { }; TEST_F(XmlCompatVersionerTest, NoRulesOnlyStripsAndCopies) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + foo="bar"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -84,35 +90,35 @@ TEST_F(XmlCompatVersionerTest, NoRulesOnlyStripsAndCopies) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; // Source XML file's sdkVersion == 0, so the first one must also have the same sdkVersion. - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(3u, el->attributes.size()); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(3u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), NotNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); } TEST_F(XmlCompatVersionerTest, SingleRule) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + foo="bar"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -129,51 +135,51 @@ TEST_F(XmlCompatVersionerTest, SingleRule) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(4u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(4u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(5u, el->attributes.size()); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_NE(nullptr, el->FindAttribute(xml::kSchemaAuto, "foo")); - EXPECT_NE(nullptr, el->FindAttribute({}, "foo")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(5u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), NotNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAuto, "foo"), NotNull()); + EXPECT_THAT(el->FindAttribute({}, "foo"), NotNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); } TEST_F(XmlCompatVersionerTest, ChainedRule) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:paddingHorizontal="24dp" />)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); @@ -193,71 +199,70 @@ TEST_F(XmlCompatVersionerTest, ChainedRule) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(3u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(3u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_HONEYCOMB), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(1u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingLeft")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingRight")); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_HONEYCOMB)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(1u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingRight"), IsNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "progressBarPadding"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[2]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[2].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingLeft")); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingRight")); + EXPECT_THAT(versioned_docs[2]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[2]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"), IsNull()); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingRight"), IsNull()); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); attr = el->FindAttribute(xml::kSchemaAndroid, "progressBarPadding"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); } TEST_F(XmlCompatVersionerTest, DegradeRuleOverridesExistingAttribute) { - auto doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + auto doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:paddingRight="16dp"/>)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - Item* padding_horizontal_value = xml::FindRootElement(doc.get()) - ->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal") - ->compiled_value.get(); - ASSERT_NE(nullptr, padding_horizontal_value); + Item* padding_horizontal_value = + doc->root->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")->compiled_value.get(); + ASSERT_THAT(padding_horizontal_value, NotNull()); XmlCompatVersioner::Rules rules; rules[R::attr::paddingHorizontal] = @@ -271,50 +276,50 @@ TEST_F(XmlCompatVersionerTest, DegradeRuleOverridesExistingAttribute) { XmlCompatVersioner versioner(&rules); std::vector> versioned_docs = versioner.Process(context_.get(), doc.get(), api_range); - ASSERT_EQ(2u, versioned_docs.size()); + ASSERT_THAT(versioned_docs, SizeIs(2u)); xml::Element* el; - EXPECT_EQ(static_cast(0), versioned_docs[0]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[0].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(2u, el->attributes.size()); - EXPECT_EQ(nullptr, el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal")); + EXPECT_THAT(versioned_docs[0]->file.config.sdkVersion, Eq(0u)); + el = versioned_docs[0]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(2u)); + EXPECT_THAT(el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"), IsNull()); xml::Attribute* attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); - EXPECT_EQ(static_cast(SDK_LOLLIPOP_MR1), versioned_docs[1]->file.config.sdkVersion); - el = xml::FindRootElement(versioned_docs[1].get()); - ASSERT_NE(nullptr, el); - EXPECT_EQ(3u, el->attributes.size()); + EXPECT_THAT(versioned_docs[1]->file.config.sdkVersion, Eq(SDK_LOLLIPOP_MR1)); + el = versioned_docs[1]->root.get(); + ASSERT_THAT(el, NotNull()); + EXPECT_THAT(el->attributes, SizeIs(3u)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingHorizontal"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingLeft"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); attr = el->FindAttribute(xml::kSchemaAndroid, "paddingRight"); - ASSERT_NE(nullptr, attr); - ASSERT_NE(nullptr, attr->compiled_value); - ASSERT_TRUE(padding_horizontal_value->Equals(attr->compiled_value.get())); + ASSERT_THAT(attr, NotNull()); + ASSERT_THAT(attr->compiled_value, NotNull()); ASSERT_TRUE(attr->compiled_attribute); + ASSERT_THAT(*attr->compiled_value, ValueEq(padding_horizontal_value)); } } // namespace aapt diff --git a/tools/aapt2/link/XmlNamespaceRemover.cpp b/tools/aapt2/link/XmlNamespaceRemover.cpp index 24aa5660ae309..b5e2423d58dc1 100644 --- a/tools/aapt2/link/XmlNamespaceRemover.cpp +++ b/tools/aapt2/link/XmlNamespaceRemover.cpp @@ -24,37 +24,19 @@ namespace aapt { namespace { -/** - * Visits each xml Node, removing URI references and nested namespaces. - */ +// Visits each xml Node, removing URI references and nested namespaces. class XmlVisitor : public xml::Visitor { public: explicit XmlVisitor(bool keep_uris) : keep_uris_(keep_uris) {} void Visit(xml::Element* el) override { - // Strip namespaces - for (auto& child : el->children) { - while (child && xml::NodeCast(child.get())) { - if (child->children.empty()) { - child = {}; - } else { - child = std::move(child->children.front()); - child->parent = el; - } - } - } - el->children.erase( - std::remove_if(el->children.begin(), el->children.end(), - [](const std::unique_ptr& child) -> bool { - return child == nullptr; - }), - el->children.end()); + el->namespace_decls.clear(); if (!keep_uris_) { for (xml::Attribute& attr : el->attributes) { - attr.namespace_uri = std::string(); + attr.namespace_uri.clear(); } - el->namespace_uri = std::string(); + el->namespace_uri.clear(); } xml::Visitor::Visit(el); } @@ -67,19 +49,11 @@ class XmlVisitor : public xml::Visitor { } // namespace -bool XmlNamespaceRemover::Consume(IAaptContext* context, - xml::XmlResource* resource) { +bool XmlNamespaceRemover::Consume(IAaptContext* context, xml::XmlResource* resource) { if (!resource->root) { return false; } - // Replace any root namespaces until the root is a non-namespace node - while (xml::NodeCast(resource->root.get())) { - if (resource->root->children.empty()) { - break; - } - resource->root = std::move(resource->root->children.front()); - resource->root->parent = nullptr; - } + XmlVisitor visitor(keep_uris_); resource->root->Accept(&visitor); return true; diff --git a/tools/aapt2/link/XmlNamespaceRemover_test.cpp b/tools/aapt2/link/XmlNamespaceRemover_test.cpp index a176c03a64325..df4920fde37f2 100644 --- a/tools/aapt2/link/XmlNamespaceRemover_test.cpp +++ b/tools/aapt2/link/XmlNamespaceRemover_test.cpp @@ -18,6 +18,10 @@ #include "test/Test.h" +using ::testing::NotNull; +using ::testing::SizeIs; +using ::testing::StrEq; + namespace aapt { class XmlUriTestVisitor : public xml::Visitor { @@ -25,16 +29,14 @@ class XmlUriTestVisitor : public xml::Visitor { XmlUriTestVisitor() = default; void Visit(xml::Element* el) override { - for (const auto& attr : el->attributes) { - EXPECT_EQ(std::string(), attr.namespace_uri); - } - EXPECT_EQ(std::string(), el->namespace_uri); - xml::Visitor::Visit(el); - } + EXPECT_THAT(el->namespace_decls, SizeIs(0u)); - void Visit(xml::Namespace* ns) override { - EXPECT_EQ(std::string(), ns->namespace_uri); - xml::Visitor::Visit(ns); + for (const auto& attr : el->attributes) { + EXPECT_THAT(attr.namespace_uri, StrEq("")); + } + EXPECT_THAT(el->namespace_uri, StrEq("")); + + xml::Visitor::Visit(el); } private: @@ -45,10 +47,9 @@ class XmlNamespaceTestVisitor : public xml::Visitor { public: XmlNamespaceTestVisitor() = default; - void Visit(xml::Namespace* ns) override { - ADD_FAILURE() << "Detected namespace: " << ns->namespace_prefix << "=\"" - << ns->namespace_uri << "\""; - xml::Visitor::Visit(ns); + void Visit(xml::Element* el) override { + EXPECT_THAT(el->namespace_decls, SizeIs(0u)); + xml::Visitor::Visit(el); } private: @@ -58,8 +59,7 @@ class XmlNamespaceTestVisitor : public xml::Visitor { class XmlNamespaceRemoverTest : public ::testing::Test { public: void SetUp() override { - context_ = - test::ContextBuilder().SetCompilationPackage("com.app.test").Build(); + context_ = test::ContextBuilder().SetCompilationPackage("com.app.test").Build(); } protected: @@ -75,8 +75,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveUris) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlUriTestVisitor visitor; root->Accept(&visitor); @@ -93,8 +93,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveNamespaces) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlNamespaceTestVisitor visitor; root->Accept(&visitor); @@ -112,8 +112,8 @@ TEST_F(XmlNamespaceRemoverTest, RemoveNestedNamespaces) { XmlNamespaceRemover remover; ASSERT_TRUE(remover.Consume(context_.get(), doc.get())); - xml::Node* root = doc.get()->root.get(); - ASSERT_NE(root, nullptr); + xml::Node* root = doc->root.get(); + ASSERT_THAT(root, NotNull()); XmlNamespaceTestVisitor visitor; root->Accept(&visitor); diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index 721fc26399bcc..bcecd20038468 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -144,7 +144,9 @@ class XmlVisitor : public xml::PackageAwareVisitor { xml::PackageAwareVisitor::Visit(el); } - bool HasError() { return error_ || reference_visitor_.HasError(); } + bool HasError() { + return error_ || reference_visitor_.HasError(); + } private: DISALLOW_COPY_AND_ASSIGN(XmlVisitor); diff --git a/tools/aapt2/link/XmlReferenceLinker_test.cpp b/tools/aapt2/link/XmlReferenceLinker_test.cpp index 228cfb9e3d663..ef99355e5b5fb 100644 --- a/tools/aapt2/link/XmlReferenceLinker_test.cpp +++ b/tools/aapt2/link/XmlReferenceLinker_test.cpp @@ -79,20 +79,20 @@ class XmlReferenceLinkerTest : public ::testing::Test { }; TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAndroid, "layout_width"); @@ -138,32 +138,32 @@ TEST_F(XmlReferenceLinkerTest, LinkBasicAttributes) { } TEST_F(XmlReferenceLinkerTest, PrivateSymbolsAreNotLinked) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_FALSE(linker.Consume(context_.get(), doc.get())); } TEST_F(XmlReferenceLinkerTest, PrivateSymbolsAreLinkedWhenReferenceHasStarPrefix) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( )EOF"); + android:colorAccent="@*android:color/hidden" />)"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); } TEST_F(XmlReferenceLinkerTest, LinkMangledAttributes) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = @@ -175,14 +175,14 @@ TEST_F(XmlReferenceLinkerTest, LinkMangledAttributes) { } TEST_F(XmlReferenceLinkerTest, LinkAutoResReference) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); xml::Attribute* xml_attr = view_el->FindAttribute(xml::kSchemaAuto, "colorAccent"); @@ -196,17 +196,15 @@ TEST_F(XmlReferenceLinkerTest, LinkAutoResReference) { } TEST_F(XmlReferenceLinkerTest, LinkViewWithShadowedPackageAlias) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - - - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + + + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); // All attributes and references in this element should be referring to @@ -235,14 +233,14 @@ TEST_F(XmlReferenceLinkerTest, LinkViewWithShadowedPackageAlias) { } TEST_F(XmlReferenceLinkerTest, LinkViewWithLocalPackageAndAliasOfTheSameName) { - std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"EOF( - )EOF"); + std::unique_ptr doc = test::BuildXmlDomForPackageName(context_.get(), R"( + )"); XmlReferenceLinker linker; ASSERT_TRUE(linker.Consume(context_.get(), doc.get())); - xml::Element* view_el = xml::FindRootElement(doc.get()); + xml::Element* view_el = doc->root.get(); ASSERT_THAT(view_el, NotNull()); // All attributes and references in this element should be referring to diff --git a/tools/aapt2/proto/ProtoHelpers.h b/tools/aapt2/proto/ProtoHelpers.h index 344e9477ea714..cecec2d06c030 100644 --- a/tools/aapt2/proto/ProtoHelpers.h +++ b/tools/aapt2/proto/ProtoHelpers.h @@ -27,23 +27,18 @@ namespace aapt { -void SerializeStringPoolToPb(const StringPool& pool, - pb::StringPool* out_pb_pool); +void SerializeStringPoolToPb(const StringPool& pool, pb::StringPool* out_pb_pool); -void SerializeSourceToPb(const Source& source, StringPool* src_pool, - pb::Source* out_pb_source); +void SerializeSourceToPb(const Source& source, StringPool* src_pool, pb::Source* out_pb_source); -void DeserializeSourceFromPb(const pb::Source& pb_source, - const android::ResStringPool& src_pool, +void DeserializeSourceFromPb(const pb::Source& pb_source, const android::ResStringPool& src_pool, Source* out_source); pb::SymbolStatus_Visibility SerializeVisibilityToPb(SymbolState state); -SymbolState DeserializeVisibilityFromPb( - pb::SymbolStatus_Visibility pb_visibility); +SymbolState DeserializeVisibilityFromPb(pb::SymbolStatus_Visibility pb_visibility); -void SerializeConfig(const ConfigDescription& config, - pb::ConfigDescription* out_pb_config); +void SerializeConfig(const ConfigDescription& config, pb::ConfigDescription* out_pb_config); bool DeserializeConfigDescriptionFromPb(const pb::ConfigDescription& pb_config, ConfigDescription* out_config); diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index b5795451e7499..8f9788e8a25d5 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -201,7 +201,7 @@ std::unique_ptr BuildXmlDom(const StringPiece& str) { StringInputStream in(input); StdErrDiagnostics diag; std::unique_ptr doc = xml::Inflate(&in, &diag, Source("test.xml")); - CHECK(doc != nullptr) << "failed to parse inline XML string"; + CHECK(doc != nullptr && doc->root != nullptr) << "failed to parse inline XML string"; return doc; } diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index d7b46caf8c941..e6b38c0007b4f 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -142,10 +142,97 @@ MATCHER_P(StrEq, a, return android::StringPiece16(arg) == a; } -MATCHER_P(ValueEq, a, - std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { - return arg.Equals(&a); -} +class ValueEq { + public: + template + class BaseImpl : public ::testing::MatcherInterface { + BaseImpl(const BaseImpl&) = default; + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + protected: + BaseImpl(const Value* expected) : expected_(expected) { + } + + const Value* expected_; + }; + + template + class Impl {}; + + template + class Impl : public ::testing::MatcherInterface { + public: + explicit Impl(const Value* expected) : expected_(expected) { + } + + bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override { + return expected_->Equals(&x); + } + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + private: + DISALLOW_COPY_AND_ASSIGN(Impl); + + const Value* expected_; + }; + + template + class Impl : public ::testing::MatcherInterface { + public: + explicit Impl(const Value* expected) : expected_(expected) { + } + + bool MatchAndExplain(T x, ::testing::MatchResultListener* listener) const override { + return expected_->Equals(x); + } + + void DescribeTo(::std::ostream* os) const override { + *os << "is equal to " << *expected_; + } + + void DescribeNegationTo(::std::ostream* os) const override { + *os << "is not equal to " << *expected_; + } + + private: + DISALLOW_COPY_AND_ASSIGN(Impl); + + const Value* expected_; + }; + + ValueEq(const Value& expected) : expected_(&expected) { + } + ValueEq(const Value* expected) : expected_(expected) { + } + ValueEq(const ValueEq&) = default; + + template + operator ::testing::Matcher() const { + return ::testing::Matcher(new Impl::value>(expected_)); + } + + private: + const Value* expected_; +}; + +// MATCHER_P(ValueEq, a, +// std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { +// return arg.Equals(&a); +//} MATCHER_P(StrValueEq, a, std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { diff --git a/tools/aapt2/xml/XmlActionExecutor.cpp b/tools/aapt2/xml/XmlActionExecutor.cpp index 352a933616333..cc664a5de7224 100644 --- a/tools/aapt2/xml/XmlActionExecutor.cpp +++ b/tools/aapt2/xml/XmlActionExecutor.cpp @@ -78,7 +78,7 @@ bool XmlActionExecutor::Execute(XmlActionExecutorPolicy policy, IDiagnostics* di XmlResource* doc) const { SourcePathDiagnostics source_diag(doc->file.source, diag); - Element* el = FindRootElement(doc); + Element* el = doc->root.get(); if (!el) { if (policy == XmlActionExecutorPolicy::kWhitelist) { source_diag.Error(DiagMessage() << "no root XML tag found"); diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp index d6df7150214bf..cbb652ed9a1a2 100644 --- a/tools/aapt2/xml/XmlDom.cpp +++ b/tools/aapt2/xml/XmlDom.cpp @@ -39,17 +39,15 @@ namespace xml { constexpr char kXmlNamespaceSep = 1; struct Stack { - std::unique_ptr root; - std::stack node_stack; + std::unique_ptr root; + std::stack node_stack; + std::unique_ptr pending_element; std::string pending_comment; std::unique_ptr last_text_node; }; -/** - * Extracts the namespace and name of an expanded element or attribute name. - */ -static void SplitName(const char* name, std::string* out_ns, - std::string* out_name) { +// Extracts the namespace and name of an expanded element or attribute name. +static void SplitName(const char* name, std::string* out_ns, std::string* out_name) { const char* p = name; while (*p != 0 && *p != kXmlNamespaceSep) { p++; @@ -67,6 +65,7 @@ static void SplitName(const char* name, std::string* out_ns, static void FinishPendingText(Stack* stack) { if (stack->last_text_node != nullptr) { if (!stack->last_text_node->text.empty()) { + CHECK(!stack->node_stack.empty()); stack->node_stack.top()->AppendChild(std::move(stack->last_text_node)); } else { // Drop an empty text node. @@ -75,48 +74,27 @@ static void FinishPendingText(Stack* stack) { } } -static void AddToStack(Stack* stack, XML_Parser parser, - std::unique_ptr node) { - node->line_number = XML_GetCurrentLineNumber(parser); - node->column_number = XML_GetCurrentColumnNumber(parser); - - Node* this_node = node.get(); - if (!stack->node_stack.empty()) { - stack->node_stack.top()->AppendChild(std::move(node)); - } else { - stack->root = std::move(node); - } - - if (!NodeCast(this_node)) { - stack->node_stack.push(this_node); - } -} - -static void XMLCALL StartNamespaceHandler(void* user_data, const char* prefix, - const char* uri) { +static void XMLCALL StartNamespaceHandler(void* user_data, const char* prefix, const char* uri) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - std::unique_ptr ns = util::make_unique(); - if (prefix) { - ns->namespace_prefix = prefix; - } + NamespaceDecl decl; + decl.line_number = XML_GetCurrentLineNumber(parser); + decl.column_number = XML_GetCurrentColumnNumber(parser); + decl.prefix = prefix ? prefix : ""; + decl.uri = uri ? uri : ""; - if (uri) { - ns->namespace_uri = uri; + if (stack->pending_element == nullptr) { + stack->pending_element = util::make_unique(); } - - AddToStack(stack, parser, std::move(ns)); + stack->pending_element->namespace_decls.push_back(std::move(decl)); } -static void XMLCALL EndNamespaceHandler(void* user_data, const char* prefix) { +static void XMLCALL EndNamespaceHandler(void* user_data, const char* /*prefix*/) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - - CHECK(!stack->node_stack.empty()); - stack->node_stack.pop(); } static bool less_attribute(const Attribute& lhs, const Attribute& rhs) { @@ -124,28 +102,42 @@ static bool less_attribute(const Attribute& lhs, const Attribute& rhs) { std::tie(rhs.namespace_uri, rhs.name, rhs.value); } -static void XMLCALL StartElementHandler(void* user_data, const char* name, - const char** attrs) { +static void XMLCALL StartElementHandler(void* user_data, const char* name, const char** attrs) { XML_Parser parser = reinterpret_cast(user_data); Stack* stack = reinterpret_cast(XML_GetUserData(parser)); FinishPendingText(stack); - std::unique_ptr el = util::make_unique(); + std::unique_ptr el; + if (stack->pending_element != nullptr) { + el = std::move(stack->pending_element); + } else { + el = util::make_unique(); + } + + el->line_number = XML_GetCurrentLineNumber(parser); + el->column_number = XML_GetCurrentColumnNumber(parser); + el->comment = std::move(stack->pending_comment); + SplitName(name, &el->namespace_uri, &el->name); while (*attrs) { Attribute attribute; SplitName(*attrs++, &attribute.namespace_uri, &attribute.name); attribute.value = *attrs++; - - // Insert in sorted order. - auto iter = std::lower_bound(el->attributes.begin(), el->attributes.end(), attribute, - less_attribute); - el->attributes.insert(iter, std::move(attribute)); + el->attributes.push_back(std::move(attribute)); } - el->comment = std::move(stack->pending_comment); - AddToStack(stack, parser, std::move(el)); + // Sort the attributes. + std::sort(el->attributes.begin(), el->attributes.end(), less_attribute); + + // Add to the stack. + Element* this_el = el.get(); + if (!stack->node_stack.empty()) { + stack->node_stack.top()->AppendChild(std::move(el)); + } else { + stack->root = std::move(el); + } + stack->node_stack.push(this_el); } static void XMLCALL EndElementHandler(void* user_data, const char* name) { @@ -263,13 +255,13 @@ static void CopyAttributes(Element* el, android::ResXMLParser* parser, StringPoo std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnostics* diag, const Source& source) { // We import the android namespace because on Windows NO_ERROR is a macro, not - // an enum, which - // causes errors when qualifying it with android:: + // an enum, which causes errors when qualifying it with android:: using namespace android; StringPool string_pool; - std::unique_ptr root; - std::stack node_stack; + std::unique_ptr root; + std::stack node_stack; + std::unique_ptr pending_element; ResXMLTree tree; if (tree.setTo(data, data_len) != NO_ERROR) { @@ -277,57 +269,76 @@ std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnos } ResXMLParser::event_code_t code; - while ((code = tree.next()) != ResXMLParser::BAD_DOCUMENT && - code != ResXMLParser::END_DOCUMENT) { + while ((code = tree.next()) != ResXMLParser::BAD_DOCUMENT && code != ResXMLParser::END_DOCUMENT) { std::unique_ptr new_node; switch (code) { case ResXMLParser::START_NAMESPACE: { - std::unique_ptr node = util::make_unique(); + NamespaceDecl decl; size_t len; const char16_t* str16 = tree.getNamespacePrefix(&len); if (str16) { - node->namespace_prefix = util::Utf16ToUtf8(StringPiece16(str16, len)); + decl.prefix = util::Utf16ToUtf8(StringPiece16(str16, len)); } str16 = tree.getNamespaceUri(&len); if (str16) { - node->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + decl.uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + } + + if (pending_element == nullptr) { + pending_element = util::make_unique(); } - new_node = std::move(node); break; } case ResXMLParser::START_TAG: { - std::unique_ptr node = util::make_unique(); + std::unique_ptr el; + if (pending_element != nullptr) { + el = std::move(pending_element); + } else { + el = util::make_unique(); + ; + } + size_t len; const char16_t* str16 = tree.getElementNamespace(&len); if (str16) { - node->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); + el->namespace_uri = util::Utf16ToUtf8(StringPiece16(str16, len)); } str16 = tree.getElementName(&len); if (str16) { - node->name = util::Utf16ToUtf8(StringPiece16(str16, len)); + el->name = util::Utf16ToUtf8(StringPiece16(str16, len)); } - CopyAttributes(node.get(), &tree, &string_pool); + Element* this_el = el.get(); + CopyAttributes(el.get(), &tree, &string_pool); - new_node = std::move(node); + if (!node_stack.empty()) { + node_stack.top()->AppendChild(std::move(el)); + } else { + root = std::move(el); + } + node_stack.push(this_el); break; } case ResXMLParser::TEXT: { - std::unique_ptr node = util::make_unique(); + std::unique_ptr text = util::make_unique(); + text->line_number = tree.getLineNumber(); size_t len; const char16_t* str16 = tree.getText(&len); if (str16) { - node->text = util::Utf16ToUtf8(StringPiece16(str16, len)); + text->text = util::Utf16ToUtf8(StringPiece16(str16, len)); } - new_node = std::move(node); + CHECK(!node_stack.empty()); + node_stack.top()->AppendChild(std::move(text)); break; } case ResXMLParser::END_NAMESPACE: + break; + case ResXMLParser::END_TAG: CHECK(!node_stack.empty()); node_stack.pop(); @@ -337,74 +348,32 @@ std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnos LOG(FATAL) << "unhandled XML chunk type"; break; } - - if (new_node) { - new_node->line_number = tree.getLineNumber(); - - Node* this_node = new_node.get(); - if (!root) { - CHECK(node_stack.empty()) << "node stack should be empty"; - root = std::move(new_node); - } else { - CHECK(!node_stack.empty()) << "node stack should not be empty"; - node_stack.top()->AppendChild(std::move(new_node)); - } - - if (!NodeCast(this_node)) { - node_stack.push(this_node); - } - } } return util::make_unique(ResourceFile{}, std::move(string_pool), std::move(root)); } -std::unique_ptr Namespace::Clone(const ElementCloneFunc& el_cloner) { - auto ns = util::make_unique(); - ns->comment = comment; - ns->line_number = line_number; - ns->column_number = column_number; - ns->namespace_prefix = namespace_prefix; - ns->namespace_uri = namespace_uri; - ns->children.reserve(children.size()); - for (const std::unique_ptr& child : children) { - ns->AppendChild(child->Clone(el_cloner)); - } - return std::move(ns); -} - -Element* FindRootElement(XmlResource* doc) { - return FindRootElement(doc->root.get()); -} - Element* FindRootElement(Node* node) { - if (!node) { + if (node == nullptr) { return nullptr; } - Element* el = nullptr; - while ((el = NodeCast(node)) == nullptr) { - if (node->children.empty()) { - return nullptr; - } - // We are looking for the first element, and namespaces can only have one - // child. - node = node->children.front().get(); + while (node->parent != nullptr) { + node = node->parent; } - return el; + return NodeCast(node); } -void Node::AppendChild(std::unique_ptr child) { +void Element::AppendChild(std::unique_ptr child) { child->parent = this; children.push_back(std::move(child)); } -void Node::InsertChild(size_t index, std::unique_ptr child) { +void Element::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) { +Attribute* Element::FindAttribute(const StringPiece& ns, const StringPiece& name) { for (auto& attr : attributes) { if (ns == attr.namespace_uri && name == attr.name) { return &attr; @@ -426,21 +395,11 @@ Element* Element::FindChild(const StringPiece& ns, const StringPiece& name) { return FindChildWithAttribute(ns, name, {}, {}, {}); } -Element* Element::FindChildWithAttribute(const StringPiece& ns, - const StringPiece& name, - const StringPiece& attr_ns, - const StringPiece& attr_name, +Element* Element::FindChildWithAttribute(const StringPiece& ns, const StringPiece& name, + const StringPiece& attr_ns, const StringPiece& attr_name, const StringPiece& attr_value) { - for (auto& child_node : children) { - Node* child = child_node.get(); - while (NodeCast(child)) { - if (child->children.empty()) { - break; - } - child = child->children[0].get(); - } - - if (Element* el = NodeCast(child)) { + for (auto& child : children) { + if (Element* el = NodeCast(child.get())) { if (ns == el->namespace_uri && name == el->name) { if (attr_ns.empty() && attr_name.empty()) { return el; @@ -459,23 +418,16 @@ Element* Element::FindChildWithAttribute(const StringPiece& ns, std::vector Element::GetChildElements() { std::vector elements; for (auto& child_node : children) { - Node* child = child_node.get(); - while (NodeCast(child)) { - if (child->children.empty()) { - break; - } - child = child->children[0].get(); - } - - if (Element* el = NodeCast(child)) { - elements.push_back(el); + if (Element* child = NodeCast(child_node.get())) { + elements.push_back(child); } } return elements; } -std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) { +std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) const { auto el = util::make_unique(); + el->namespace_decls = namespace_decls; el->comment = comment; el->line_number = line_number; el->column_number = column_number; @@ -490,7 +442,17 @@ std::unique_ptr Element::Clone(const ElementCloneFunc& el_cloner) { return std::move(el); } -std::unique_ptr Text::Clone(const ElementCloneFunc&) { +std::unique_ptr Element::CloneElement(const ElementCloneFunc& el_cloner) const { + return std::unique_ptr(static_cast(Clone(el_cloner).release())); +} + +void Element::Accept(Visitor* visitor) { + visitor->BeforeVisitElement(this); + visitor->Visit(this); + visitor->AfterVisitElement(this); +} + +std::unique_ptr Text::Clone(const ElementCloneFunc&) const { auto t = util::make_unique(); t->comment = comment; t->line_number = line_number; @@ -499,21 +461,22 @@ std::unique_ptr Text::Clone(const ElementCloneFunc&) { return std::move(t); } -void PackageAwareVisitor::Visit(Namespace* ns) { - bool added = false; - if (Maybe maybe_package = - ExtractPackageFromNamespace(ns->namespace_uri)) { - ExtractedPackage& package = maybe_package.value(); - package_decls_.push_back( - PackageDecl{ns->namespace_prefix, std::move(package)}); - added = true; - } +void Text::Accept(Visitor* visitor) { + visitor->Visit(this); +} - Visitor::Visit(ns); - - if (added) { - package_decls_.pop_back(); +void PackageAwareVisitor::BeforeVisitElement(Element* el) { + std::vector decls; + for (const NamespaceDecl& decl : el->namespace_decls) { + if (Maybe maybe_package = ExtractPackageFromNamespace(decl.uri)) { + decls.push_back(PackageDecl{decl.prefix, std::move(maybe_package.value())}); + } } + package_decls_.push_back(std::move(decls)); +} + +void PackageAwareVisitor::AfterVisitElement(Element* el) { + package_decls_.pop_back(); } Maybe PackageAwareVisitor::TransformPackageAlias( @@ -524,11 +487,16 @@ Maybe PackageAwareVisitor::TransformPackageAlias( const auto rend = package_decls_.rend(); for (auto iter = package_decls_.rbegin(); iter != rend; ++iter) { - if (alias == iter->prefix) { - if (iter->package.package.empty()) { - return ExtractedPackage{local_package.to_string(), iter->package.private_namespace}; + const std::vector& decls = *iter; + const auto rend2 = decls.rend(); + for (auto iter2 = decls.rbegin(); iter2 != rend2; ++iter2) { + const PackageDecl& decl = *iter2; + if (alias == decl.prefix) { + if (decl.package.package.empty()) { + return ExtractedPackage{local_package.to_string(), decl.package.private_namespace}; + } + return decl.package; } - return iter->package; } } return {}; diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h index 54a70333fad26..1542243816266 100644 --- a/tools/aapt2/xml/XmlDom.h +++ b/tools/aapt2/xml/XmlDom.h @@ -33,45 +33,33 @@ namespace aapt { namespace xml { -class RawVisitor; - class Element; +class Visitor; // Base class for all XML nodes. class Node { public: - Node* parent = nullptr; - size_t line_number = 0; - size_t column_number = 0; - std::string comment; - std::vector> children; - virtual ~Node() = default; - void AppendChild(std::unique_ptr child); - void InsertChild(size_t index, std::unique_ptr child); - virtual void Accept(RawVisitor* visitor) = 0; + Element* parent = nullptr; + size_t line_number = 0u; + size_t column_number = 0u; + std::string comment; + + virtual void Accept(Visitor* visitor) = 0; using ElementCloneFunc = std::function; // Clones the Node subtree, using the given function to decide how to clone an Element. - virtual std::unique_ptr Clone(const ElementCloneFunc& el_cloner) = 0; + virtual std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const = 0; }; -// Base class that implements the visitor methods for a subclass of Node. -template -class BaseNode : public Node { - public: - virtual void Accept(RawVisitor* visitor) override; -}; - -// A Namespace XML node. Can only have one child. -class Namespace : public BaseNode { - public: - std::string namespace_prefix; - std::string namespace_uri; - - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; +// A namespace declaration (xmlns:prefix="uri"). +struct NamespaceDecl { + std::string prefix; + std::string uri; + size_t line_number = 0u; + size_t column_number = 0u; }; struct AaptAttribute { @@ -94,31 +82,46 @@ struct Attribute { }; // An Element XML node. -class Element : public BaseNode { +class Element : public Node { public: + // Ordered namespace prefix declarations. + std::vector namespace_decls; + std::string namespace_uri; std::string name; std::vector attributes; + std::vector> children; + + void AppendChild(std::unique_ptr child); + void InsertChild(size_t index, std::unique_ptr child); Attribute* FindAttribute(const android::StringPiece& ns, const android::StringPiece& name); const Attribute* FindAttribute(const android::StringPiece& ns, const android::StringPiece& name) const; - xml::Element* FindChild(const android::StringPiece& ns, const android::StringPiece& name); - xml::Element* FindChildWithAttribute(const android::StringPiece& ns, - const android::StringPiece& name, - const android::StringPiece& attr_ns, - const android::StringPiece& attr_name, - const android::StringPiece& attr_value); - std::vector GetChildElements(); - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; + Element* FindChild(const android::StringPiece& ns, const android::StringPiece& name); + Element* FindChildWithAttribute(const android::StringPiece& ns, const android::StringPiece& name, + const android::StringPiece& attr_ns, + const android::StringPiece& attr_name, + const android::StringPiece& attr_value); + std::vector GetChildElements(); + + // Due to overriding of subtypes not working with unique_ptr, define a convenience Clone method + // that knows cloning an element returns an element. + std::unique_ptr CloneElement(const ElementCloneFunc& el_cloner) const; + + std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const override; + + void Accept(Visitor* visitor) override; }; // A Text (CDATA) XML node. Can not have any children. -class Text : public BaseNode { +class Text : public Node { public: std::string text; - std::unique_ptr Clone(const ElementCloneFunc& el_cloner) override; + std::unique_ptr Clone(const ElementCloneFunc& el_cloner) const override; + + void Accept(Visitor* visitor) override; }; // An XML resource with a source, name, and XML tree. @@ -131,7 +134,7 @@ class XmlResource { // is destroyed. StringPool string_pool; - std::unique_ptr root; + std::unique_ptr root; }; // Inflates an XML DOM from an InputStream, logging errors to the logger. @@ -143,42 +146,38 @@ std::unique_ptr Inflate(io::InputStream* in, IDiagnostics* diag, co std::unique_ptr Inflate(const void* data, size_t data_len, IDiagnostics* diag, const Source& source); -Element* FindRootElement(XmlResource* doc); Element* FindRootElement(Node* node); -// A visitor interface for the different XML Node subtypes. This will not traverse into children. -// Use Visitor for that. -class RawVisitor { - public: - virtual ~RawVisitor() = default; - - virtual void Visit(Namespace* node) {} - virtual void Visit(Element* node) {} - virtual void Visit(Text* text) {} -}; - // Visitor whose default implementation visits the children nodes of any node. -class Visitor : public RawVisitor { +class Visitor { public: - using RawVisitor::Visit; + virtual ~Visitor() = default; - void Visit(Namespace* node) override { - VisitChildren(node); + virtual void Visit(Element* el) { + VisitChildren(el); } - void Visit(Element* node) override { - VisitChildren(node); + virtual void Visit(Text* text) { } - void Visit(Text* text) override { - VisitChildren(text); - } + protected: + Visitor() = default; - void VisitChildren(Node* node) { - for (auto& child : node->children) { + void VisitChildren(Element* el) { + for (auto& child : el->children) { child->Accept(this); } } + + virtual void BeforeVisitElement(Element* el) { + } + virtual void AfterVisitElement(Element* el) { + } + + private: + DISALLOW_COPY_AND_ASSIGN(Visitor); + + friend class Element; }; // An XML DOM visitor that will record the package name for a namespace prefix. @@ -186,41 +185,70 @@ class PackageAwareVisitor : public Visitor, public IPackageDeclStack { public: using Visitor::Visit; - void Visit(Namespace* ns) override; Maybe TransformPackageAlias( const android::StringPiece& alias, const android::StringPiece& local_package) const override; + protected: + PackageAwareVisitor() = default; + + void BeforeVisitElement(Element* el) override; + void AfterVisitElement(Element* el) override; + private: + DISALLOW_COPY_AND_ASSIGN(PackageAwareVisitor); + struct PackageDecl { std::string prefix; ExtractedPackage package; }; - std::vector package_decls_; + std::vector> package_decls_; }; -// Implementations +namespace internal { -template -void BaseNode::Accept(RawVisitor* visitor) { - visitor->Visit(static_cast(this)); -} +// Base class that overrides the default behaviour and does not descend into child nodes. +class NodeCastBase : public Visitor { + public: + void Visit(Element* el) override { + } + void Visit(Text* el) override { + } + + protected: + NodeCastBase() = default; + + void BeforeVisitElement(Element* el) override { + } + void AfterVisitElement(Element* el) override { + } + + private: + DISALLOW_COPY_AND_ASSIGN(NodeCastBase); +}; template -class NodeCastImpl : public RawVisitor { +class NodeCastImpl : public NodeCastBase { public: - using RawVisitor::Visit; + using NodeCastBase::Visit; + + NodeCastImpl() = default; T* value = nullptr; void Visit(T* v) override { value = v; } + + private: + DISALLOW_COPY_AND_ASSIGN(NodeCastImpl); }; +} // namespace internal + template T* NodeCast(Node* node) { - NodeCastImpl visitor; + internal::NodeCastImpl visitor; node->Accept(&visitor); return visitor.value; } diff --git a/tools/aapt2/xml/XmlDom_test.cpp b/tools/aapt2/xml/XmlDom_test.cpp index 1340ada6d9538..6ed2d616f7828 100644 --- a/tools/aapt2/xml/XmlDom_test.cpp +++ b/tools/aapt2/xml/XmlDom_test.cpp @@ -25,8 +25,10 @@ using ::aapt::io::StringInputStream; using ::testing::Eq; using ::testing::NotNull; using ::testing::SizeIs; +using ::testing::StrEq; namespace aapt { +namespace xml { TEST(XmlDomTest, Inflate) { std::string input = R"( @@ -40,24 +42,23 @@ TEST(XmlDomTest, Inflate) { StdErrDiagnostics diag; StringInputStream in(input); - std::unique_ptr doc = xml::Inflate(&in, &diag, Source("test.xml")); + std::unique_ptr doc = Inflate(&in, &diag, Source("test.xml")); ASSERT_THAT(doc, NotNull()); - xml::Namespace* ns = xml::NodeCast(doc->root.get()); - ASSERT_THAT(ns, NotNull()); - EXPECT_THAT(ns->namespace_uri, Eq(xml::kSchemaAndroid)); - EXPECT_THAT(ns->namespace_prefix, Eq("android")); + Element* el = doc->root.get(); + EXPECT_THAT(el->namespace_decls, SizeIs(1u)); + EXPECT_THAT(el->namespace_decls[0].uri, StrEq(xml::kSchemaAndroid)); + EXPECT_THAT(el->namespace_decls[0].prefix, StrEq("android")); } // Escaping is handled after parsing of the values for resource-specific values. TEST(XmlDomTest, ForwardEscapes) { - std::unique_ptr doc = test::BuildXmlDom(R"( + std::unique_ptr doc = test::BuildXmlDom(R"( \\d{5})"); - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_THAT(el, NotNull()); + Element* el = doc->root.get(); - xml::Attribute* attr = el->FindAttribute({}, "pattern"); + Attribute* attr = el->FindAttribute({}, "pattern"); ASSERT_THAT(attr, NotNull()); EXPECT_THAT(attr->value, Eq("\\\\d{5}")); @@ -67,21 +68,54 @@ TEST(XmlDomTest, ForwardEscapes) { ASSERT_THAT(el->children, SizeIs(1u)); - xml::Text* text = xml::NodeCast(el->children[0].get()); + Text* text = xml::NodeCast(el->children[0].get()); ASSERT_THAT(text, NotNull()); EXPECT_THAT(text->text, Eq("\\\\d{5}")); } TEST(XmlDomTest, XmlEscapeSequencesAreParsed) { - std::unique_ptr doc = test::BuildXmlDom(R"()"); - - xml::Element* el = xml::FindRootElement(doc.get()); - ASSERT_THAT(el, NotNull()); - - xml::Attribute* attr = el->FindAttribute({}, "value"); + std::unique_ptr doc = test::BuildXmlDom(R"()"); + Attribute* attr = doc->root->FindAttribute({}, "value"); ASSERT_THAT(attr, NotNull()); - EXPECT_THAT(attr->value, Eq("\"")); } +class TestVisitor : public PackageAwareVisitor { + public: + using PackageAwareVisitor::Visit; + + void Visit(Element* el) override { + if (el->name == "View1") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + } else if (el->name == "View2") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two", "local"), + Eq(make_value(ExtractedPackage{"com.two", false}))); + } else if (el->name == "View3") { + EXPECT_THAT(TransformPackageAlias("one", "local"), + Eq(make_value(ExtractedPackage{"com.one", false}))); + EXPECT_THAT(TransformPackageAlias("two", "local"), + Eq(make_value(ExtractedPackage{"com.two", false}))); + EXPECT_THAT(TransformPackageAlias("three", "local"), + Eq(make_value(ExtractedPackage{"com.three", false}))); + } + } +}; + +TEST(XmlDomTest, PackageAwareXmlVisitor) { + std::unique_ptr doc = test::BuildXmlDom(R"( + + + + + )"); + + Debug::DumpXml(doc.get()); + TestVisitor visitor; + doc->root->Accept(&visitor); +} + +} // namespace xml } // namespace aapt diff --git a/tools/aapt2/xml/XmlUtil.h b/tools/aapt2/xml/XmlUtil.h index 1650ac2124ac4..866b6dcd7a880 100644 --- a/tools/aapt2/xml/XmlUtil.h +++ b/tools/aapt2/xml/XmlUtil.h @@ -26,79 +26,56 @@ namespace aapt { namespace xml { constexpr const char* kSchemaAuto = "http://schemas.android.com/apk/res-auto"; -constexpr const char* kSchemaPublicPrefix = - "http://schemas.android.com/apk/res/"; -constexpr const char* kSchemaPrivatePrefix = - "http://schemas.android.com/apk/prv/res/"; -constexpr const char* kSchemaAndroid = - "http://schemas.android.com/apk/res/android"; +constexpr const char* kSchemaPublicPrefix = "http://schemas.android.com/apk/res/"; +constexpr const char* kSchemaPrivatePrefix = "http://schemas.android.com/apk/prv/res/"; +constexpr const char* kSchemaAndroid = "http://schemas.android.com/apk/res/android"; constexpr const char* kSchemaTools = "http://schemas.android.com/tools"; constexpr const char* kSchemaAapt = "http://schemas.android.com/aapt"; -/** - * Result of extracting a package name from a namespace URI declaration. - */ +// Result of extracting a package name from a namespace URI declaration. struct ExtractedPackage { - /** - * The name of the package. This can be the empty string, which means that the - * package - * should be assumed to be the package being compiled. - */ + // The name of the package. This can be the empty string, which means that the package + // should be assumed to be the package being compiled. std::string package; - /** - * True if the package's private namespace was declared. This means that - * private resources - * are made visible. - */ + // True if the package's private namespace was declared. This means that private resources + // are made visible. bool private_namespace; + + friend inline bool operator==(const ExtractedPackage& a, const ExtractedPackage& b) { + return a.package == b.package && a.private_namespace == b.private_namespace; + } }; -/** - * Returns an ExtractedPackage struct if the namespace URI is of the form: - * http://schemas.android.com/apk/res/ or - * http://schemas.android.com/apk/prv/res/ - * - * Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, - * returns an empty package name. - */ -Maybe ExtractPackageFromNamespace( - const std::string& namespace_uri); +// Returns an ExtractedPackage struct if the namespace URI is of the form: +// http://schemas.android.com/apk/res/ or +// http://schemas.android.com/apk/prv/res/ +// +// Special case: if namespaceUri is http://schemas.android.com/apk/res-auto, +// returns an empty package name. +Maybe ExtractPackageFromNamespace(const std::string& namespace_uri); -/** - * Returns an XML Android namespace for the given package of the form: - * - * http://schemas.android.com/apk/res/ - * - * If privateReference == true, the package will be of the form: - * - * http://schemas.android.com/apk/prv/res/ - */ +// Returns an XML Android namespace for the given package of the form: +// http://schemas.android.com/apk/res/ +// +// If privateReference == true, the package will be of the form: +// http://schemas.android.com/apk/prv/res/ std::string BuildPackageNamespace(const android::StringPiece& package, bool private_reference = false); -/** - * Interface representing a stack of XML namespace declarations. When looking up - * the package - * for a namespace prefix, the stack is checked from top to bottom. - */ +// Interface representing a stack of XML namespace declarations. When looking up the package +// for a namespace prefix, the stack is checked from top to bottom. struct IPackageDeclStack { virtual ~IPackageDeclStack() = default; - /** - * Returns an ExtractedPackage struct if the alias given corresponds with a - * package declaration. - */ + // Returns an ExtractedPackage struct if the alias given corresponds with a package declaration. virtual Maybe TransformPackageAlias( const android::StringPiece& alias, const android::StringPiece& local_package) const = 0; }; -/** - * Helper function for transforming the original Reference inRef to a fully - * qualified reference - * via the IPackageDeclStack. This will also mark the Reference as private if - * the namespace of the package declaration was private. - */ +// Helper function for transforming the original Reference inRef to a fully qualified reference +// via the IPackageDeclStack. This will also mark the Reference as private if the namespace of the +// package declaration was private. void TransformReferenceFromNamespace(IPackageDeclStack* decl_stack, const android::StringPiece& local_package, Reference* in_ref);