Merge "Do not serialize empty text in manifest proto"

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;

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<xml::Element>(child.get())) {
       SerializeXmlToPb(*child_el, pb_element->add_child());
     } else if (const xml::Text* text_el = xml::NodeCast<xml::Text>(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

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.

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<xml::Text> trim_text = util::make_unique<xml::Text>();
+  trim_text->line_number = 25;
+  trim_text->column_number = 3;
+  trim_text->text = "  \n   ";
+  element.AppendChild(std::move(trim_text));
+
+  std::unique_ptr<xml::Text> keep_text = util::make_unique<xml::Text>();
+  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<xml::Text>(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<IAaptContext> context = test::ContextBuilder().Build();
   std::unique_ptr<ResourceTable> table =