From 467b689a4c0869b98b4c8c67c8accd1422cd1956 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Fri, 9 Nov 2018 15:52:07 -0800 Subject: Do not serialize empty text in manifest proto When linking an APK in the proto format, the manifest is currently serializes text nodes that only contain whitespace: child: { text: "\n " source: { line_number : 0x0000000f column_number: 0x0000002f } } These whitespace bloat the proto size unnecessarily. Do not write these text nodes for proto apks. Bug: 118800653 Test: make aapt2_tests Change-Id: Icfaaf88976f81450bbf51610a316b336deeae60c --- tools/aapt2/cmd/Link.cpp | 5 +++- tools/aapt2/format/proto/ProtoSerialize.cpp | 15 +++++++--- tools/aapt2/format/proto/ProtoSerialize.h | 11 ++++++-- tools/aapt2/format/proto/ProtoSerialize_test.cpp | 36 ++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 6a7da0c57df3..1b5601d451a7 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -236,6 +236,9 @@ static bool FlattenXml(IAaptContext* context, const xml::XmlResource& xml_res, case OutputFormat::kProto: { pb::XmlNode pb_node; + // Strip whitespace text nodes from tha AndroidManifest.xml + SerializeXmlOptions options; + options.remove_empty_text_nodes = (path == kAndroidManifestPath); SerializeXmlResourceToPb(xml_res, &pb_node); return io::CopyProtoToArchive(context, &pb_node, path.to_string(), ArchiveEntry::kCompress, writer); @@ -1543,7 +1546,7 @@ class Linker { bool WriteApk(IArchiveWriter* writer, proguard::KeepSet* keep_set, xml::XmlResource* manifest, ResourceTable* table) { const bool keep_raw_values = context_->GetPackageType() == PackageType::kStaticLib; - bool result = FlattenXml(context_, *manifest, "AndroidManifest.xml", keep_raw_values, + bool result = FlattenXml(context_, *manifest, kAndroidManifestPath, keep_raw_values, true /*utf16*/, options_.output_format, writer); if (!result) { return false; diff --git a/tools/aapt2/format/proto/ProtoSerialize.cpp b/tools/aapt2/format/proto/ProtoSerialize.cpp index f1e96d61191b..ecf34d13e1b3 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize.cpp @@ -621,7 +621,8 @@ static void SerializeXmlCommon(const xml::Node& node, pb::XmlNode* out_node) { pb_src->set_column_number(node.column_number); } -void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node) { +void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node, + const SerializeXmlOptions options) { SerializeXmlCommon(el, out_node); pb::XmlElement* pb_element = out_node->mutable_element(); @@ -657,7 +658,12 @@ void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node) { if (const xml::Element* child_el = xml::NodeCast(child.get())) { SerializeXmlToPb(*child_el, pb_element->add_child()); } else if (const xml::Text* text_el = xml::NodeCast(child.get())) { - pb::XmlNode* pb_child_node = pb_element->add_child(); + if (options.remove_empty_text_nodes && util::TrimWhitespace(text_el->text).empty()) { + // Do not serialize whitespace text nodes if told not to + continue; + } + + pb::XmlNode *pb_child_node = pb_element->add_child(); SerializeXmlCommon(*text_el, pb_child_node); pb_child_node->set_text(text_el->text); } else { @@ -666,8 +672,9 @@ void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node) { } } -void SerializeXmlResourceToPb(const xml::XmlResource& resource, pb::XmlNode* out_node) { - SerializeXmlToPb(*resource.root, out_node); +void SerializeXmlResourceToPb(const xml::XmlResource& resource, pb::XmlNode* out_node, + const SerializeXmlOptions options) { + SerializeXmlToPb(*resource.root, out_node, options); } } // namespace aapt diff --git a/tools/aapt2/format/proto/ProtoSerialize.h b/tools/aapt2/format/proto/ProtoSerialize.h index c40e5dd51da6..33ffd182435b 100644 --- a/tools/aapt2/format/proto/ProtoSerialize.h +++ b/tools/aapt2/format/proto/ProtoSerialize.h @@ -30,6 +30,11 @@ namespace aapt { +struct SerializeXmlOptions { + /** Remove text nodes that only contain whitespace. */ + bool remove_empty_text_nodes = false; +}; + // Serializes a Value to its protobuf representation. An optional StringPool will hold the // source path string. void SerializeValueToPb(const Value& value, pb::Value* out_value, StringPool* src_pool = nullptr); @@ -39,10 +44,12 @@ void SerializeValueToPb(const Value& value, pb::Value* out_value, StringPool* sr void SerializeItemToPb(const Item& item, pb::Item* out_item); // Serializes an XML element into its protobuf representation. -void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node); +void SerializeXmlToPb(const xml::Element& el, pb::XmlNode* out_node, + const SerializeXmlOptions options = {}); // Serializes an XmlResource into its protobuf representation. The ResourceFile is NOT serialized. -void SerializeXmlResourceToPb(const xml::XmlResource& resource, pb::XmlNode* out_node); +void SerializeXmlResourceToPb(const xml::XmlResource& resource, pb::XmlNode* out_node, + const SerializeXmlOptions options = {}); // Serializes a StringPool into its protobuf representation, which is really just the binary // ResStringPool representation stuffed into a bytes field. diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index 95dbbeb58a5d..1cd2f0b9a961 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -257,6 +257,42 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeXml) { EXPECT_THAT(child_text->text, StrEq("woah there")); } +TEST(ProtoSerializeTest, SerializeAndDeserializeXmlTrimEmptyWhitepsace) { + xml::Element element; + element.line_number = 22; + element.column_number = 23; + element.name = "element"; + + std::unique_ptr trim_text = util::make_unique(); + trim_text->line_number = 25; + trim_text->column_number = 3; + trim_text->text = " \n "; + element.AppendChild(std::move(trim_text)); + + std::unique_ptr keep_text = util::make_unique(); + keep_text->line_number = 26; + keep_text->column_number = 3; + keep_text->text = " hello "; + element.AppendChild(std::move(keep_text)); + + pb::XmlNode pb_xml; + SerializeXmlOptions options; + options.remove_empty_text_nodes = true; + SerializeXmlToPb(element, &pb_xml, options); + + StringPool pool; + xml::Element actual_el; + std::string error; + ASSERT_TRUE(DeserializeXmlFromPb(pb_xml, &actual_el, &pool, &error)); + ASSERT_THAT(error, IsEmpty()); + + // Only the child that does not consist of only whitespace should remain + ASSERT_THAT(actual_el.children, SizeIs(1u)); + const xml::Text* child_text_keep = xml::NodeCast(actual_el.children[0].get()); + ASSERT_THAT(child_text_keep, NotNull()); + EXPECT_THAT(child_text_keep->text, StrEq( " hello ")); +} + TEST(ProtoSerializeTest, SerializeAndDeserializePrimitives) { std::unique_ptr context = test::ContextBuilder().Build(); std::unique_ptr table = -- cgit v1.2.3-59-g8ed1b