Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 467b689a authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

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
parent 67dd91e6
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -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;
+11 −4
Original line number Diff line number Diff line
@@ -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,6 +658,11 @@ 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())) {
      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);
@@ -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
+9 −2
Original line number Diff line number Diff line
@@ -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.
+36 −0
Original line number Diff line number Diff line
@@ -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 =