Improve idmap2 xml parsing

To parse the <overlay> tags for internal overlay references, we require
the ability to parse XML. The current implementation only allows idmap2
to find a tag somewhere in an xml file.

This implementation allows for the retrieval of an iterator that
iterates over direct children xml elements. Now we can enforce that the
<overlay> tag in the manifest is nested within the <manifest> tag and we
can ensure <item> tags are within the <overlay> tags of the overlay
resource configuration xml.

Bug: 135051420
Bug: 135943783
Test: idmap2_tests
Change-Id: I12f965b55c18960af6cf9dc6ca46429f53124c9a

13 files changed, 543 insertions, 263 deletions

diff --git a/cmds/idmap2/Android.bp b/cmds/idmap2/Android.bp
index 4c77ba402595..60d64149510c 100644
--- a/cmds/idmap2/Android.bp
+++ b/cmds/idmap2/Android.bp
@@ -45,7 +45,7 @@ cc_library {
         "libidmap2/RawPrintVisitor.cpp",
         "libidmap2/ResourceUtils.cpp",
         "libidmap2/Result.cpp",
-        "libidmap2/Xml.cpp",
+        "libidmap2/XmlParser.cpp",
         "libidmap2/ZipFile.cpp",
     ],
     export_include_dirs: ["include"],
@@ -99,7 +99,7 @@ cc_test {
         "tests/RawPrintVisitorTests.cpp",
         "tests/ResourceUtilsTests.cpp",
         "tests/ResultTests.cpp",
-        "tests/XmlTests.cpp",
+        "tests/XmlParserTests.cpp",
         "tests/ZipFileTests.cpp",
     ],
     required: [
diff --git a/cmds/idmap2/idmap2/Lookup.cpp b/cmds/idmap2/idmap2/Lookup.cpp
index b7ae9d090cee..c5cf9807b689 100644
--- a/cmds/idmap2/idmap2/Lookup.cpp
+++ b/cmds/idmap2/idmap2/Lookup.cpp
@@ -33,9 +33,10 @@
 #include "androidfw/Util.h"
 #include "idmap2/CommandLineOptions.h"
 #include "idmap2/Idmap.h"
+#include "idmap2/ResourceUtils.h"
 #include "idmap2/Result.h"
 #include "idmap2/SysTrace.h"
-#include "idmap2/Xml.h"
+#include "idmap2/XmlParser.h"
 #include "idmap2/ZipFile.h"
 #include "utils/String16.h"
 #include "utils/String8.h"
@@ -57,8 +58,7 @@ using android::idmap2::IdmapHeader;
 using android::idmap2::ResourceId;
 using android::idmap2::Result;
 using android::idmap2::Unit;
-using android::idmap2::Xml;
-using android::idmap2::ZipFile;
+using android::idmap2::utils::ExtractOverlayManifestInfo;
 using android::util::Utf16ToUtf8;
 
 namespace {
@@ -132,29 +132,6 @@ Result<std::string> WARN_UNUSED GetValue(const AssetManager2& am, ResourceId res
   return out;
 }
 
-Result<std::string> GetTargetPackageNameFromManifest(const std::string& apk_path) {
-  const auto zip = ZipFile::Open(apk_path);
-  if (!zip) {
-    return Error("failed to open %s as zip", apk_path.c_str());
-  }
-  const auto entry = zip->Uncompress("AndroidManifest.xml");
-  if (!entry) {
-    return Error("failed to uncompress AndroidManifest.xml in %s", apk_path.c_str());
-  }
-  const auto xml = Xml::Create(entry->buf, entry->size);
-  if (!xml) {
-    return Error("failed to create XML buffer");
-  }
-  const auto tag = xml->FindTag("overlay");
-  if (!tag) {
-    return Error("failed to find <overlay> tag");
-  }
-  const auto iter = tag->find("targetPackage");
-  if (iter == tag->end()) {
-    return Error("failed to find targetPackage attribute");
-  }
-  return iter->second;
-}
 }  // namespace
 
 Result<Unit> Lookup(const std::vector<std::string>& args) {
@@ -202,12 +179,12 @@ Result<Unit> Lookup(const std::vector<std::string>& args) {
       }
       apk_assets.push_back(std::move(target_apk));
 
-      const Result<std::string> package_name =
-          GetTargetPackageNameFromManifest(idmap_header->GetOverlayPath().to_string());
-      if (!package_name) {
-        return Error("failed to parse android:targetPackage from overlay manifest");
+      auto manifest_info = ExtractOverlayManifestInfo(idmap_header->GetOverlayPath().to_string(),
+                                                      true /* assert_overlay */);
+      if (!manifest_info) {
+        return manifest_info.GetError();
       }
-      target_package_name = *package_name;
+      target_package_name = (*manifest_info).target_package;
     } else if (target_path != idmap_header->GetTargetPath()) {
       return Error("different target APKs (expected target APK %s but %s has target APK %s)",
                    target_path.c_str(), idmap_path.c_str(),
diff --git a/cmds/idmap2/idmap2/Scan.cpp b/cmds/idmap2/idmap2/Scan.cpp
index d0530f2d344e..e643ab567644 100644
--- a/cmds/idmap2/idmap2/Scan.cpp
+++ b/cmds/idmap2/idmap2/Scan.cpp
@@ -33,7 +33,7 @@
 #include "idmap2/ResourceUtils.h"
 #include "idmap2/Result.h"
 #include "idmap2/SysTrace.h"
-#include "idmap2/Xml.h"
+#include "idmap2/XmlParser.h"
 #include "idmap2/ZipFile.h"
 
 using android::idmap2::CommandLineOptions;
diff --git a/cmds/idmap2/include/idmap2/Xml.h b/cmds/idmap2/include/idmap2/Xml.h
deleted file mode 100644
index dd89dee0f64f..000000000000
--- a/cmds/idmap2/include/idmap2/Xml.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef IDMAP2_INCLUDE_IDMAP2_XML_H_
-#define IDMAP2_INCLUDE_IDMAP2_XML_H_
-
-#include <map>
-#include <memory>
-#include <string>
-
-#include "android-base/macros.h"
-#include "androidfw/ResourceTypes.h"
-#include "utils/String16.h"
-
-namespace android::idmap2 {
-
-class Xml {
- public:
-  static std::unique_ptr<const Xml> Create(const uint8_t* data, size_t size, bool copyData = false);
-
-  std::unique_ptr<std::map<std::string, std::string>> FindTag(const std::string& name) const;
-
-  ~Xml();
-
- private:
-  Xml() {
-  }
-
-  mutable ResXMLTree xml_;
-
-  DISALLOW_COPY_AND_ASSIGN(Xml);
-};
-
-}  // namespace android::idmap2
-
-#endif  // IDMAP2_INCLUDE_IDMAP2_XML_H_
diff --git a/cmds/idmap2/include/idmap2/XmlParser.h b/cmds/idmap2/include/idmap2/XmlParser.h
new file mode 100644
index 000000000000..972a6d7e3427
--- /dev/null
+++ b/cmds/idmap2/include/idmap2/XmlParser.h
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef IDMAP2_INCLUDE_IDMAP2_XMLPARSER_H_
+#define IDMAP2_INCLUDE_IDMAP2_XMLPARSER_H_
+
+#include <iostream>
+#include <map>
+#include <memory>
+#include <string>
+
+#include "Result.h"
+#include "android-base/macros.h"
+#include "androidfw/ResourceTypes.h"
+#include "utils/String16.h"
+
+namespace android::idmap2 {
+
+class XmlParser {
+ public:
+  using Event = ResXMLParser::event_code_t;
+  class iterator;
+
+  class Node {
+   public:
+    Event event() const;
+    std::string name() const;
+
+    Result<std::string> GetAttributeStringValue(const std::string& name) const;
+    Result<Res_value> GetAttributeValue(const std::string& name) const;
+
+    bool operator==(const Node& rhs) const;
+    bool operator!=(const Node& rhs) const;
+
+   private:
+    explicit Node(const ResXMLTree& tree);
+    Node(const ResXMLTree& tree, const ResXMLParser::ResXMLPosition& pos);
+
+    // Retrieves/Sets the position of the position of the xml parser in the xml tree.
+    ResXMLParser::ResXMLPosition get_position() const;
+    void set_position(const ResXMLParser::ResXMLPosition& pos);
+
+    // If `inner_child` is true, seek advances the parser to the first inner child of the current
+    // node. Otherwise, seek advances the parser to the following node. Returns false if there is
+    // no node to seek to.
+    bool Seek(bool inner_child);
+
+    ResXMLParser parser_;
+    friend iterator;
+  };
+
+  class iterator {
+   public:
+    iterator(const iterator& other) : iterator(other.tree_, other.iter_) {
+    }
+
+    inline iterator& operator=(const iterator& rhs) {
+      iter_.set_position(rhs.iter_.get_position());
+      return *this;
+    }
+
+    inline bool operator==(const iterator& rhs) const {
+      return iter_ == rhs.iter_;
+    }
+
+    inline bool operator!=(const iterator& rhs) const {
+      return !(*this == rhs);
+    }
+
+    inline iterator operator++() {
+      // Seek to the following xml node.
+      iter_.Seek(false /* inner_child */);
+      return *this;
+    }
+
+    iterator begin() const {
+      iterator child_it(*this);
+      // Seek to the first inner child of the current node.
+      child_it.iter_.Seek(true /* inner_child */);
+      return child_it;
+    }
+
+    iterator end() const {
+      iterator child_it = begin();
+      while (child_it.iter_.Seek(false /* inner_child */)) {
+        // Continue iterating until the end tag is found.
+      }
+
+      return child_it;
+    }
+
+    inline const Node operator*() {
+      return Node(tree_, iter_.get_position());
+    }
+
+    inline const Node* operator->() {
+      return &iter_;
+    }
+
+   private:
+    explicit iterator(const ResXMLTree& tree) : tree_(tree), iter_(Node(tree)) {
+    }
+    iterator(const ResXMLTree& tree, const Node& node)
+        : tree_(tree), iter_(Node(tree, node.get_position())) {
+    }
+
+    const ResXMLTree& tree_;
+    Node iter_;
+    friend XmlParser;
+  };
+
+  // Creates a new xml parser beginning at the first tag.
+  static Result<std::unique_ptr<const XmlParser>> Create(const void* data, size_t size,
+                                                         bool copy_data = false);
+  ~XmlParser();
+
+  inline iterator tree_iterator() const {
+    return iterator(tree_);
+  }
+
+  inline const ResStringPool& get_strings() const {
+    return tree_.getStrings();
+  }
+
+ private:
+  XmlParser() = default;
+  mutable ResXMLTree tree_;
+
+  DISALLOW_COPY_AND_ASSIGN(XmlParser);
+};
+
+}  // namespace android::idmap2
+
+#endif  // IDMAP2_INCLUDE_IDMAP2_XMLPARSER_H_
diff --git a/cmds/idmap2/libidmap2/ResourceUtils.cpp b/cmds/idmap2/libidmap2/ResourceUtils.cpp
index dce83e35978d..d9c97e19399b 100644
--- a/cmds/idmap2/libidmap2/ResourceUtils.cpp
+++ b/cmds/idmap2/libidmap2/ResourceUtils.cpp
@@ -22,18 +22,18 @@
 #include "androidfw/StringPiece.h"
 #include "androidfw/Util.h"
 #include "idmap2/Result.h"
-#include "idmap2/Xml.h"
+#include "idmap2/XmlParser.h"
 #include "idmap2/ZipFile.h"
 
 using android::StringPiece16;
 using android::idmap2::Result;
-using android::idmap2::Xml;
+using android::idmap2::XmlParser;
 using android::idmap2::ZipFile;
 using android::util::Utf16ToUtf8;
 
 namespace android::idmap2::utils {
 
-Result<std::string> ResToTypeEntryName(const AssetManager2& am, ResourceId resid) {
+Result<std::string> ResToTypeEntryName(const AssetManager2& am, uint32_t resid) {
   AssetManager2::ResourceName name;
   if (!am.GetResourceName(resid, &name)) {
     return Error("no resource 0x%08x in asset manager", resid);
@@ -65,42 +65,55 @@ Result<OverlayManifestInfo> ExtractOverlayManifestInfo(const std::string& path,
     return Error("failed to uncompress AndroidManifest.xml from %s", path.c_str());
   }
 
-  std::unique_ptr<const Xml> xml = Xml::Create(entry->buf, entry->size);
+  Result<std::unique_ptr<const XmlParser>> xml = XmlParser::Create(entry->buf, entry->size);
   if (!xml) {
     return Error("failed to parse AndroidManifest.xml from %s", path.c_str());
   }
 
+  auto manifest_it = (*xml)->tree_iterator();
+  if (manifest_it->event() != XmlParser::Event::START_TAG || manifest_it->name() != "manifest") {
+    return Error("root element tag is not <manifest> in AndroidManifest.xml of %s", path.c_str());
+  }
+
+  auto overlay_it = std::find_if(manifest_it.begin(), manifest_it.end(), [](const auto& it) {
+    return it.event() == XmlParser::Event::START_TAG && it.name() == "overlay";
+  });
+
   OverlayManifestInfo info{};
-  const auto tag = xml->FindTag("overlay");
-  if (!tag) {
-    if (assert_overlay) {
-      return Error("<overlay> missing from AndroidManifest.xml of %s", path.c_str());
+  if (overlay_it == manifest_it.end()) {
+    if (!assert_overlay) {
+      return info;
     }
-    return info;
+    return Error("<overlay> missing from AndroidManifest.xml of %s", path.c_str());
   }
 
-  auto iter = tag->find("targetPackage");
-  if (iter == tag->end()) {
-    if (assert_overlay) {
-      return Error("android:targetPackage missing from <overlay> of %s", path.c_str());
-    }
+  if (auto result_str = overlay_it->GetAttributeStringValue("targetPackage")) {
+    info.target_package = *result_str;
   } else {
-    info.target_package = iter->second;
+    return Error("android:targetPackage missing from <overlay> of %s: %s", path.c_str(),
+                 result_str.GetErrorMessage().c_str());
   }
 
-  iter = tag->find("targetName");
-  if (iter != tag->end()) {
-    info.target_name = iter->second;
+  if (auto result_str = overlay_it->GetAttributeStringValue("targetName")) {
+    info.target_name = *result_str;
   }
 
-  iter = tag->find("isStatic");
-  if (iter != tag->end()) {
-    info.is_static = std::stoul(iter->second) != 0U;
+  if (auto result_value = overlay_it->GetAttributeValue("isStatic")) {
+    if ((*result_value).dataType >= Res_value::TYPE_FIRST_INT &&
+        (*result_value).dataType <= Res_value::TYPE_LAST_INT) {
+      info.is_static = (*result_value).data != 0U;
+    } else {
+      return Error("android:isStatic is not a boolean in AndroidManifest.xml of %s", path.c_str());
+    }
   }
 
-  iter = tag->find("priority");
-  if (iter != tag->end()) {
-    info.priority = std::stoi(iter->second);
+  if (auto result_value = overlay_it->GetAttributeValue("priority")) {
+    if ((*result_value).dataType >= Res_value::TYPE_FIRST_INT &&
+        (*result_value).dataType <= Res_value::TYPE_LAST_INT) {
+      info.priority = (*result_value).data;
+    } else {
+      return Error("android:priority is not an integer in AndroidManifest.xml of %s", path.c_str());
+    }
   }
 
   iter = tag->find("requiredSystemPropertyName");
diff --git a/cmds/idmap2/libidmap2/Xml.cpp b/cmds/idmap2/libidmap2/Xml.cpp
deleted file mode 100644
index 264586829c47..000000000000
--- a/cmds/idmap2/libidmap2/Xml.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "idmap2/Xml.h"
-
-#include <map>
-#include <memory>
-#include <string>
-#include <utility>
-
-namespace android::idmap2 {
-
-std::unique_ptr<const Xml> Xml::Create(const uint8_t* data, size_t size, bool copyData) {
-  std::unique_ptr<Xml> xml(new Xml());
-  if (xml->xml_.setTo(data, size, copyData) != NO_ERROR) {
-    return nullptr;
-  }
-  return xml;
-}
-
-std::unique_ptr<std::map<std::string, std::string>> Xml::FindTag(const std::string& name) const {
-  const String16 tag_to_find(name.c_str(), name.size());
-  xml_.restart();
-  ResXMLParser::event_code_t type;
-  do {
-    type = xml_.next();
-    if (type == ResXMLParser::START_TAG) {
-      size_t len;
-      const String16 tag(xml_.getElementName(&len));
-      if (tag == tag_to_find) {
-        std::unique_ptr<std::map<std::string, std::string>> map(
-            new std::map<std::string, std::string>());
-        for (size_t i = 0; i < xml_.getAttributeCount(); i++) {
-          const String16 key16(xml_.getAttributeName(i, &len));
-          std::string key = String8(key16).c_str();
-
-          std::string value;
-          switch (xml_.getAttributeDataType(i)) {
-            case Res_value::TYPE_STRING: {
-              const String16 value16(xml_.getAttributeStringValue(i, &len));
-              value = String8(value16).c_str();
-            } break;
-            case Res_value::TYPE_INT_DEC:
-            case Res_value::TYPE_INT_HEX:
-            case Res_value::TYPE_INT_BOOLEAN: {
-              Res_value resValue;
-              xml_.getAttributeValue(i, &resValue);
-              value = std::to_string(resValue.data);
-            } break;
-            default:
-              return nullptr;
-          }
-
-          map->emplace(std::make_pair(key, value));
-        }
-        return map;
-      }
-    }
-  } while (type != ResXMLParser::BAD_DOCUMENT && type != ResXMLParser::END_DOCUMENT);
-  return nullptr;
-}
-
-Xml::~Xml() {
-  xml_.uninit();
-}
-
-}  // namespace android::idmap2
diff --git a/cmds/idmap2/libidmap2/XmlParser.cpp b/cmds/idmap2/libidmap2/XmlParser.cpp
new file mode 100644
index 000000000000..526a560907aa
--- /dev/null
+++ b/cmds/idmap2/libidmap2/XmlParser.cpp
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "idmap2/XmlParser.h"
+
+#include <iostream>
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>
+
+namespace android::idmap2 {
+
+template <typename T>
+ResXMLParser::ResXMLPosition get_tree_position(const T& tree) {
+  ResXMLParser::ResXMLPosition pos{};
+  tree.getPosition(&pos);
+  return pos;
+}
+
+XmlParser::Node::Node(const ResXMLTree& tree) : Node(tree, get_tree_position(tree)) {
+}
+XmlParser::Node::Node(const ResXMLTree& tree, const ResXMLParser::ResXMLPosition& pos)
+    : parser_(tree) {
+  set_position(pos);
+}
+
+bool XmlParser::Node::operator==(const XmlParser::Node& rhs) const {
+  ResXMLParser::ResXMLPosition pos = get_position();
+  ResXMLParser::ResXMLPosition rhs_pos = rhs.get_position();
+  return pos.curExt == rhs_pos.curExt && pos.curNode == rhs_pos.curNode &&
+         pos.eventCode == rhs_pos.eventCode;
+}
+
+bool XmlParser::Node::operator!=(const XmlParser::Node& rhs) const {
+  return !(*this == rhs);
+}
+
+ResXMLParser::ResXMLPosition XmlParser::Node::get_position() const {
+  return get_tree_position(parser_);
+}
+
+void XmlParser::Node::set_position(const ResXMLParser::ResXMLPosition& pos) {
+  parser_.setPosition(pos);
+}
+
+bool XmlParser::Node::Seek(bool inner_child) {
+  if (parser_.getEventType() == XmlParser::Event::END_TAG) {
+    return false;
+  }
+
+  ssize_t depth = 0;
+  XmlParser::Event code;
+  while ((code = parser_.next()) != XmlParser::Event::BAD_DOCUMENT &&
+         code != XmlParser::Event::END_DOCUMENT) {
+    if (code == XmlParser::Event::START_TAG) {
+      if (++depth == (inner_child ? 1 : 0)) {
+        return true;
+      }
+    } else if (code == XmlParser::Event::END_TAG) {
+      if (--depth == (inner_child ? -1 : -2)) {
+        return false;
+      }
+    }
+  }
+
+  return false;
+}
+
+XmlParser::Event XmlParser::Node::event() const {
+  return parser_.getEventType();
+}
+
+std::string XmlParser::Node::name() const {
+  size_t len;
+  const String16 key16(parser_.getElementName(&len));
+  return String8(key16).c_str();
+}
+
+Result<std::string> XmlParser::Node::GetAttributeStringValue(const std::string& name) const {
+  auto value = GetAttributeValue(name);
+  if (!value) {
+    return value.GetError();
+  }
+
+  switch ((*value).dataType) {
+    case Res_value::TYPE_STRING: {
+      size_t len;
+      const String16 value16(parser_.getStrings().stringAt((*value).data, &len));
+      return std::string(String8(value16).c_str());
+    }
+    case Res_value::TYPE_INT_DEC:
+    case Res_value::TYPE_INT_HEX:
+    case Res_value::TYPE_INT_BOOLEAN: {
+      return std::to_string((*value).data);
+    }
+    default:
+      return Error(R"(Failed to convert attribute "%s" value to a string)", name.c_str());
+  }
+}
+
+Result<Res_value> XmlParser::Node::GetAttributeValue(const std::string& name) const {
+  size_t len;
+  for (size_t i = 0; i < parser_.getAttributeCount(); i++) {
+    const String16 key16(parser_.getAttributeName(i, &len));
+    std::string key = String8(key16).c_str();
+    if (key != name) {
+      continue;
+    }
+
+    Res_value res_value{};
+    if (parser_.getAttributeValue(i, &res_value) == BAD_TYPE) {
+      return Error(R"(Bad value for attribute "%s")", name.c_str());
+    }
+
+    return res_value;
+  }
+
+  return Error(R"(Failed to find attribute "%s")", name.c_str());
+}
+
+Result<std::unique_ptr<const XmlParser>> XmlParser::Create(const void* data, size_t size,
+                                                           bool copy_data) {
+  auto parser = std::unique_ptr<const XmlParser>(new XmlParser());
+  if (parser->tree_.setTo(data, size, copy_data) != NO_ERROR) {
+    return Error("Malformed xml block");
+  }
+
+  // Find the beginning of the first tag.
+  XmlParser::Event event;
+  while ((event = parser->tree_.next()) != XmlParser::Event::BAD_DOCUMENT &&
+         event != XmlParser::Event::END_DOCUMENT && event != XmlParser::Event::START_TAG) {
+  }
+
+  if (event == XmlParser::Event::END_DOCUMENT) {
+    return Error("Root tag was not be found");
+  }
+
+  if (event == XmlParser::Event::BAD_DOCUMENT) {
+    return Error("Bad xml document");
+  }
+
+  return parser;
+}
+
+XmlParser::~XmlParser() {
+  tree_.uninit();
+}
+
+}  // namespace android::idmap2
diff --git a/cmds/idmap2/tests/XmlParserTests.cpp b/cmds/idmap2/tests/XmlParserTests.cpp
new file mode 100644
index 000000000000..1a7eaca4d67b
--- /dev/null
+++ b/cmds/idmap2/tests/XmlParserTests.cpp
@@ -0,0 +1,174 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cstdio>  // fclose
+#include <memory>
+#include <string>
+
+#include "TestHelpers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "idmap2/XmlParser.h"
+#include "idmap2/ZipFile.h"
+
+namespace android::idmap2 {
+
+Result<std::unique_ptr<const XmlParser>> CreateTestParser(const std::string& test_file) {
+  auto zip = ZipFile::Open(GetTestDataPath() + "/target/target.apk");
+  if (zip == nullptr) {
+    return Error("Failed to open zip file");
+  }
+
+  auto data = zip->Uncompress(test_file);
+  if (data == nullptr) {
+    return Error("Failed to open xml file");
+  }
+
+  return XmlParser::Create(data->buf, data->size, /* copy_data */ true);
+}
+
+TEST(XmlParserTests, Create) {
+  auto xml = CreateTestParser("AndroidManifest.xml");
+  ASSERT_TRUE(xml) << xml.GetErrorMessage();
+
+  fclose(stderr);  // silence expected warnings from libandroidfw
+  const char* not_xml = "foo";
+  auto fail = XmlParser::Create(reinterpret_cast<const uint8_t*>(not_xml), strlen(not_xml));
+  ASSERT_FALSE(fail);
+}
+
+TEST(XmlParserTests, NextChild) {
+  auto xml = CreateTestParser("res/xml/test.xml");
+  ASSERT_TRUE(xml) << xml.GetErrorMessage();
+
+  auto root_iter = (*xml)->tree_iterator();
+  ASSERT_EQ(root_iter->event(), XmlParser::Event::START_TAG);
+  ASSERT_EQ(root_iter->name(), "a");
+
+  auto a_iter = root_iter.begin();
+  ASSERT_EQ(a_iter->event(), XmlParser::Event::START_TAG);
+  ASSERT_EQ(a_iter->name(), "b");
+
+  auto c_iter = a_iter.begin();
+  ASSERT_EQ(c_iter->event(), XmlParser::Event::START_TAG);
+  ASSERT_EQ(c_iter->name(), "c");
+
+  ++c_iter;
+  ASSERT_EQ(c_iter->event(), XmlParser::Event::END_TAG);
+  ASSERT_EQ(c_iter, a_iter.end());
+
+  ++a_iter;
+  ASSERT_EQ(a_iter->event(), XmlParser::Event::START_TAG);
+  ASSERT_EQ(a_iter->name(), "d");
+
+  // Skip the <e> tag.
+  ++a_iter;
+  ASSERT_EQ(a_iter->event(), XmlParser::Event::END_TAG);
+  ASSERT_EQ(a_iter, root_iter.end());
+}
+
+TEST(XmlParserTests, AttributeValues) {
+  auto xml = CreateTestParser("res/xml/test.xml");
+  ASSERT_TRUE(xml) << xml.GetErrorMessage();
+
+  // Start at the <a> tag.
+  auto root_iter = (*xml)->tree_iterator();
+
+  // Start at the <b> tag.
+  auto a_iter = root_iter.begin();
+  auto attribute_str = a_iter->GetAttributeStringValue("type_string");
+  ASSERT_TRUE(attribute_str);
+  ASSERT_EQ(*attribute_str, "fortytwo");
+
+  auto attribute_value = a_iter->GetAttributeValue("type_int_dec");
+  ASSERT_TRUE(attribute_value);
+  ASSERT_EQ(attribute_value->data, 42);
+
+  attribute_value = a_iter->GetAttributeValue("type_int_hex");
+  ASSERT_TRUE(attribute_value);
+  ASSERT_EQ(attribute_value->data, 42);
+
+  attribute_value = a_iter->GetAttributeValue("type_int_boolean");
+  ASSERT_TRUE(attribute_value);
+  ASSERT_EQ(attribute_value->data, 0xffffffff);
+}
+
+TEST(XmlParserTests, IteratorEquality) {
+  auto xml = CreateTestParser("res/xml/test.xml");
+  ASSERT_TRUE(xml) << xml.GetErrorMessage();
+
+  // Start at the <a> tag.
+  auto root_iter_1 = (*xml)->tree_iterator();
+  auto root_iter_2 = (*xml)->tree_iterator();
+  ASSERT_EQ(root_iter_1, root_iter_2);
+  ASSERT_EQ(*root_iter_1, *root_iter_2);
+
+  // Start at the <b> tag.
+  auto a_iter_1 = root_iter_1.begin();
+  auto a_iter_2 = root_iter_2.begin();
+  ASSERT_NE(a_iter_1, root_iter_1.end());
+  ASSERT_NE(a_iter_2, root_iter_2.end());
+  ASSERT_EQ(a_iter_1, a_iter_2);
+  ASSERT_EQ(*a_iter_1, *a_iter_2);
+
+  // Move to the <d> tag.
+  ++a_iter_1;
+  ++a_iter_2;
+  ASSERT_NE(a_iter_1, root_iter_1.end());
+  ASSERT_NE(a_iter_2, root_iter_2.end());
+  ASSERT_EQ(a_iter_1, a_iter_2);
+  ASSERT_EQ(*a_iter_1, *a_iter_2);
+
+  // Move to the end of the <a> tag.
+  ++a_iter_1;
+  ++a_iter_2;
+  ASSERT_EQ(a_iter_1, root_iter_1.end());
+  ASSERT_EQ(a_iter_2, root_iter_2.end());
+  ASSERT_EQ(a_iter_1, a_iter_2);
+  ASSERT_EQ(*a_iter_1, *a_iter_2);
+}
+
+TEST(XmlParserTests, Backtracking) {
+  auto xml = CreateTestParser("res/xml/test.xml");
+  ASSERT_TRUE(xml) << xml.GetErrorMessage();
+
+  // Start at the <a> tag.
+  auto root_iter_1 = (*xml)->tree_iterator();
+
+  // Start at the <b> tag.
+  auto a_iter_1 = root_iter_1.begin();
+
+  // Start a second iterator at the <a> tag.
+  auto root_iter_2 = root_iter_1;
+  ASSERT_EQ(root_iter_1, root_iter_2);
+  ASSERT_EQ(*root_iter_1, *root_iter_2);
+
+  // Move the first iterator to the end of the <a> tag.
+  auto root_iter_end_1 = root_iter_1.end();
+  ++root_iter_1;
+  ASSERT_NE(root_iter_1, root_iter_2);
+  ASSERT_NE(*root_iter_1, *root_iter_2);
+
+  // Move to the <d> tag.
+  ++a_iter_1;
+  ASSERT_NE(a_iter_1, root_iter_end_1);
+
+  // Move to the end of the <a> tag.
+  ++a_iter_1;
+  ASSERT_EQ(a_iter_1, root_iter_end_1);
+}
+
+}  // namespace android::idmap2
diff --git a/cmds/idmap2/tests/XmlTests.cpp b/cmds/idmap2/tests/XmlTests.cpp
deleted file mode 100644
index df63211a9209..000000000000
--- a/cmds/idmap2/tests/XmlTests.cpp
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <cstdio>  // fclose
-
-#include "TestHelpers.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-#include "idmap2/Xml.h"
-#include "idmap2/ZipFile.h"
-
-using ::testing::IsNull;
-using ::testing::NotNull;
-
-namespace android::idmap2 {
-
-TEST(XmlTests, Create) {
-  auto zip = ZipFile::Open(GetTestDataPath() + "/target/target.apk");
-  ASSERT_THAT(zip, NotNull());
-
-  auto data = zip->Uncompress("AndroidManifest.xml");
-  ASSERT_THAT(data, NotNull());
-
-  auto xml = Xml::Create(data->buf, data->size);
-  ASSERT_THAT(xml, NotNull());
-
-  fclose(stderr);  // silence expected warnings from libandroidfw
-  const char* not_xml = "foo";
-  auto fail = Xml::Create(reinterpret_cast<const uint8_t*>(not_xml), strlen(not_xml));
-  ASSERT_THAT(fail, IsNull());
-}
-
-TEST(XmlTests, FindTag) {
-  auto zip = ZipFile::Open(GetTestDataPath() + "/target/target.apk");
-  ASSERT_THAT(zip, NotNull());
-
-  auto data = zip->Uncompress("res/xml/test.xml");
-  ASSERT_THAT(data, NotNull());
-
-  auto xml = Xml::Create(data->buf, data->size);
-  ASSERT_THAT(xml, NotNull());
-
-  auto attrs = xml->FindTag("c");
-  ASSERT_THAT(attrs, NotNull());
-  ASSERT_EQ(attrs->size(), 4U);
-  ASSERT_EQ(attrs->at("type_string"), "fortytwo");
-  ASSERT_EQ(std::stoi(attrs->at("type_int_dec")), 42);
-  ASSERT_EQ(std::stoi(attrs->at("type_int_hex")), 42);
-  ASSERT_NE(std::stoul(attrs->at("type_int_boolean")), 0U);
-
-  auto fail = xml->FindTag("does-not-exist");
-  ASSERT_THAT(fail, IsNull());
-}
-
-}  // namespace android::idmap2
diff --git a/cmds/idmap2/tests/data/target/res/xml/test.xml b/cmds/idmap2/tests/data/target/res/xml/test.xml
index 0fe21c6b6d0a..56a3f7f0b13a 100644
--- a/cmds/idmap2/tests/data/target/res/xml/test.xml
+++ b/cmds/idmap2/tests/data/target/res/xml/test.xml
@@ -14,12 +14,15 @@
      limitations under the License.
 -->
 <a>
-    <b>
-        <c
-            type_string="fortytwo"
-            type_int_dec="42"
-            type_int_hex="0x2a"
-            type_int_boolean="true"
-            />
+    <b type_string="fortytwo"
+       type_int_dec="42"
+       type_int_hex="0x2a"
+       type_int_boolean="true">
+
+        <c />
     </b>
-</a>
+
+    <d>
+        <e />
+    </d>
+</a>
\ No newline at end of file
diff --git a/cmds/idmap2/tests/data/target/target-no-overlayable.apk b/cmds/idmap2/tests/data/target/target-no-overlayable.apk
index 033305aaed4f..2eb7c477c3b4 100644
--- a/cmds/idmap2/tests/data/target/target-no-overlayable.apk
+++ b/cmds/idmap2/tests/data/target/target-no-overlayable.apk
diff --git a/cmds/idmap2/tests/data/target/target.apk b/cmds/idmap2/tests/data/target/target.apk
index 9bcd6dcabcde..251cf46f969d 100644
--- a/cmds/idmap2/tests/data/target/target.apk
+++ b/cmds/idmap2/tests/data/target/target.apk