Change CFX_XML Save to take a write stream

This CL changes CFX_XML to use an IFX_SeekableWriteStream instead of the more
generic IFX_SeekableStream.

Change-Id: I6e4def380c43eca755d91ad5cb6146c2dfdaee10
Reviewed-on: https://pdfium-review.googlesource.com/30877
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

11 files changed, 63 insertions, 92 deletions

diff --git a/core/fpdfdoc/cpdf_metadata.cpp b/core/fpdfdoc/cpdf_metadata.cpp
index 161fc93edd..c928952dca 100644
--- a/core/fpdfdoc/cpdf_metadata.cpp
+++ b/core/fpdfdoc/cpdf_metadata.cpp
@@ -19,12 +19,11 @@ void CheckForSharedFormInternal(CFX_XMLElement* element,
                                 std::vector<UnsupportedFeature>* unsupported) {
   WideString attr = element->GetAttribute(L"xmlns:adhocwf");
   if (attr == L"http://ns.adobe.com/AcrobatAdhocWorkflow/1.0/") {
-    for (const auto* child = element->GetFirstChild(); child;
-         child = child->GetNextSibling()) {
+    for (const auto& child : *element) {
       if (child->GetType() != FX_XMLNODE_Element)
         continue;
 
-      const auto* child_elem = static_cast<const CFX_XMLElement*>(child);
+      const auto* child_elem = static_cast<const CFX_XMLElement*>(child.get());
       if (child_elem->GetName() != L"adhocwf:workflowType")
         continue;
 
@@ -46,12 +45,11 @@ void CheckForSharedFormInternal(CFX_XMLElement* element,
     }
   }
 
-  for (auto* child = element->GetFirstChild(); child;
-       child = child->GetNextSibling()) {
+  for (const auto& child : *element) {
     if (child->GetType() != FX_XMLNODE_Element)
       continue;
 
-    CheckForSharedFormInternal(static_cast<CFX_XMLElement*>(child),
+    CheckForSharedFormInternal(static_cast<CFX_XMLElement*>(child.get()),
                                unsupported);
   }
 }
diff --git a/core/fxcrt/xml/cfx_xmlchardata.cpp b/core/fxcrt/xml/cfx_xmlchardata.cpp
index dec2e4618f..e62a43ac53 100644
--- a/core/fxcrt/xml/cfx_xmlchardata.cpp
+++ b/core/fxcrt/xml/cfx_xmlchardata.cpp
@@ -21,7 +21,8 @@ std::unique_ptr<CFX_XMLNode> CFX_XMLCharData::Clone() {
   return pdfium::MakeUnique<CFX_XMLCharData>(GetText());
 }
 
-void CFX_XMLCharData::Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) {
+void CFX_XMLCharData::Save(
+    const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) {
   pXMLStream->WriteString("<![CDATA[");
   pXMLStream->WriteString(GetText().UTF8Encode().AsStringView());
   pXMLStream->WriteString("]]>");
diff --git a/core/fxcrt/xml/cfx_xmlchardata.h b/core/fxcrt/xml/cfx_xmlchardata.h
index 5b00597955..0cdc348f26 100644
--- a/core/fxcrt/xml/cfx_xmlchardata.h
+++ b/core/fxcrt/xml/cfx_xmlchardata.h
@@ -20,7 +20,7 @@ class CFX_XMLCharData : public CFX_XMLText {
   // CFX_XMLNode
   FX_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFX_XMLNode> Clone() override;
-  void Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) override;
+  void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override;
 };
 
 #endif  // CORE_FXCRT_XML_CFX_XMLCHARDATA_H_
diff --git a/core/fxcrt/xml/cfx_xmlelement.cpp b/core/fxcrt/xml/cfx_xmlelement.cpp
index 5e79da63cf..8c8922e0c6 100644
--- a/core/fxcrt/xml/cfx_xmlelement.cpp
+++ b/core/fxcrt/xml/cfx_xmlelement.cpp
@@ -33,10 +33,9 @@ std::unique_ptr<CFX_XMLNode> CFX_XMLElement::Clone() {
   // TODO(dsinclair): This clone is wrong. It doesn't clone child nodes just
   // copies text nodes?
   WideString wsText;
-  for (CFX_XMLNode* pChild = GetFirstChild(); pChild;
-       pChild = pChild->GetNextSibling()) {
+  for (const auto& pChild : *this) {
     if (pChild->GetType() == FX_XMLNODE_Text)
-      wsText += static_cast<CFX_XMLText*>(pChild)->GetText();
+      wsText += static_cast<CFX_XMLText*>(pChild.get())->GetText();
   }
   pClone->SetTextData(wsText);
   return std::move(pClone);
@@ -79,11 +78,10 @@ WideString CFX_XMLElement::GetNamespaceURI() const {
 WideString CFX_XMLElement::GetTextData() const {
   CFX_WideTextBuf buffer;
 
-  for (CFX_XMLNode* pChild = GetFirstChild(); pChild;
-       pChild = pChild->GetNextSibling()) {
+  for (const auto& pChild : *this) {
     if (pChild->GetType() == FX_XMLNODE_Text ||
         pChild->GetType() == FX_XMLNODE_CharData) {
-      buffer << static_cast<CFX_XMLText*>(pChild)->GetText();
+      buffer << static_cast<CFX_XMLText*>(pChild.get())->GetText();
     }
   }
   return buffer.MakeString();
@@ -96,7 +94,8 @@ void CFX_XMLElement::SetTextData(const WideString& wsText) {
   AppendChild(pdfium::MakeUnique<CFX_XMLText>(wsText));
 }
 
-void CFX_XMLElement::Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) {
+void CFX_XMLElement::Save(
+    const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) {
   ByteString bsNameEncoded = name_.UTF8Encode();
 
   pXMLStream->WriteString("<");
@@ -109,17 +108,16 @@ void CFX_XMLElement::Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) {
         AttributeToString(it.first, it.second).UTF8Encode().AsStringView());
   }
 
-  if (!GetFirstChild()) {
+  if (HasChildren()) {
     pXMLStream->WriteString(" />\n");
     return;
   }
 
   pXMLStream->WriteString(">\n");
 
-  for (CFX_XMLNode* pChild = GetFirstChild(); pChild;
-       pChild = pChild->GetNextSibling()) {
+  for (const auto& pChild : *this)
     pChild->Save(pXMLStream);
-  }
+
   pXMLStream->WriteString("</");
   pXMLStream->WriteString(bsNameEncoded.AsStringView());
   pXMLStream->WriteString(">\n");
@@ -132,11 +130,11 @@ CFX_XMLElement* CFX_XMLElement::GetFirstChildNamed(
 
 CFX_XMLElement* CFX_XMLElement::GetNthChildNamed(const WideStringView& name,
                                                  size_t idx) const {
-  for (auto* child = GetFirstChild(); child; child = child->GetNextSibling()) {
+  for (const auto& child : *this) {
     if (child->GetType() != FX_XMLNODE_Element)
       continue;
 
-    CFX_XMLElement* elem = static_cast<CFX_XMLElement*>(child);
+    CFX_XMLElement* elem = static_cast<CFX_XMLElement*>(child.get());
     if (elem->name_ != name)
       continue;
     if (idx == 0)
diff --git a/core/fxcrt/xml/cfx_xmlelement.h b/core/fxcrt/xml/cfx_xmlelement.h
index c1d9fea3f0..678ba6d996 100644
--- a/core/fxcrt/xml/cfx_xmlelement.h
+++ b/core/fxcrt/xml/cfx_xmlelement.h
@@ -22,7 +22,7 @@ class CFX_XMLElement : public CFX_XMLNode {
   // CFX_XMLNode
   FX_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFX_XMLNode> Clone() override;
-  void Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) override;
+  void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override;
 
   WideString GetName() const { return name_; }
 
diff --git a/core/fxcrt/xml/cfx_xmlinstruction.cpp b/core/fxcrt/xml/cfx_xmlinstruction.cpp
index 7b844e6808..d0f5cbb68b 100644
--- a/core/fxcrt/xml/cfx_xmlinstruction.cpp
+++ b/core/fxcrt/xml/cfx_xmlinstruction.cpp
@@ -40,7 +40,8 @@ bool CFX_XMLInstruction::IsAcrobat() const {
   return name_ == L"acrobat";
 }
 
-void CFX_XMLInstruction::Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) {
+void CFX_XMLInstruction::Save(
+    const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) {
   if (name_.CompareNoCase(L"xml") == 0) {
     pXMLStream->WriteString("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
     return;
diff --git a/core/fxcrt/xml/cfx_xmlinstruction.h b/core/fxcrt/xml/cfx_xmlinstruction.h
index 045610bd8a..3be9d48573 100644
--- a/core/fxcrt/xml/cfx_xmlinstruction.h
+++ b/core/fxcrt/xml/cfx_xmlinstruction.h
@@ -21,7 +21,7 @@ class CFX_XMLInstruction : public CFX_XMLNode {
   // CFX_XMLNode
   FX_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFX_XMLNode> Clone() override;
-  void Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) override;
+  void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override;
 
   bool IsOriginalXFAVersion() const;
   bool IsAcrobat() const;
diff --git a/core/fxcrt/xml/cfx_xmlnode.cpp b/core/fxcrt/xml/cfx_xmlnode.cpp
index 4f811e608e..de0a4acd2f 100644
--- a/core/fxcrt/xml/cfx_xmlnode.cpp
+++ b/core/fxcrt/xml/cfx_xmlnode.cpp
@@ -6,8 +6,8 @@
 
 #include "core/fxcrt/xml/cfx_xmlnode.h"
 
+#include <algorithm>
 #include <utility>
-#include <vector>
 
 #include "core/fxcrt/xml/cfx_xmlchardata.h"
 #include "core/fxcrt/xml/cfx_xmlelement.h"
@@ -22,15 +22,7 @@ CFX_XMLNode::~CFX_XMLNode() {
 }
 
 void CFX_XMLNode::DeleteChildren() {
-  CFX_XMLNode* child = last_child_.Get();
-  // Clear last child early as it will have been deleted already.
-  last_child_ = nullptr;
-  while (child) {
-    child = child->prev_sibling_.Get();
-    if (child)
-      child->next_sibling_ = nullptr;
-  }
-  first_child_ = nullptr;
+  children_.clear();
 }
 
 void CFX_XMLNode::AppendChild(std::unique_ptr<CFX_XMLNode> pNode) {
@@ -42,63 +34,38 @@ void CFX_XMLNode::InsertChildNode(std::unique_ptr<CFX_XMLNode> pNode,
   ASSERT(!pNode->parent_);
 
   pNode->parent_ = this;
-  // No existing children, add node as first child.
-  if (!first_child_) {
-    ASSERT(!last_child_);
-
-    first_child_ = std::move(pNode);
-    last_child_ = first_child_.get();
-    first_child_->prev_sibling_ = nullptr;
-    first_child_->next_sibling_ = nullptr;
-    return;
-  }
-
-  if (index == 0) {
-    first_child_->prev_sibling_ = pNode.get();
-    pNode->next_sibling_ = std::move(first_child_);
-    pNode->prev_sibling_ = nullptr;
-    first_child_ = std::move(pNode);
+  if (static_cast<size_t>(index) >= children_.size()) {
+    if (!children_.empty())
+      children_.back()->next_sibling_ = pNode.get();
+    children_.push_back(std::move(pNode));
     return;
   }
+  index = std::max(index, 0);
 
-  int32_t iCount = 0;
-  CFX_XMLNode* pFind = first_child_.get();
-  // Note, negative indexes, and indexes after the end of the list will result
-  // in appending to the list.
-  while (++iCount != index && pFind->next_sibling_)
-    pFind = pFind->next_sibling_.get();
-
-  pNode->prev_sibling_ = pFind;
-  if (pFind->next_sibling_)
-    pFind->next_sibling_->prev_sibling_ = pNode.get();
-  pNode->next_sibling_ = std::move(pFind->next_sibling_);
-
-  pFind->next_sibling_ = std::move(pNode);
-  if (pFind == last_child_.Get())
-    last_child_ = pFind->next_sibling_.get();
+  pNode->next_sibling_ = children_[index].get();
+  children_.insert(children_.begin() + index, std::move(pNode));
+  if (index > 0)
+    children_[index - 1]->next_sibling_ = children_[index].get();
 }
 
 void CFX_XMLNode::RemoveChildNode(CFX_XMLNode* pNode) {
-  ASSERT(first_child_);
   ASSERT(pNode);
 
-  if (first_child_.get() == pNode) {
-    first_child_.release();
-    first_child_ = std::move(pNode->next_sibling_);
-  } else {
-    CFX_XMLNode* prev = pNode->prev_sibling_.Get();
-    prev->next_sibling_.release();  // Release pNode
-    prev->next_sibling_ = std::move(pNode->next_sibling_);
-  }
-
-  if (last_child_.Get() == pNode)
-    last_child_ = pNode->prev_sibling_.Get();
+  auto it = std::find(children_.begin(), children_.end(),
+                      pdfium::FakeUniquePtr<CFX_XMLNode>(pNode));
+  if (it != children_.end()) {
+    if (it != children_.begin()) {
+      CFX_XMLNode* prev = (*(it - 1)).get();
+      prev->next_sibling_ = (*it)->next_sibling_;
+    }
 
-  if (pNode->next_sibling_)
-    pNode->next_sibling_->prev_sibling_ = pNode->prev_sibling_;
+    children_.erase(it);
+  }
+}
 
-  pNode->parent_ = nullptr;
-  pNode->prev_sibling_ = nullptr;
+void CFX_XMLNode::MoveChildrenTo(CFX_XMLNode* root) {
+  ASSERT(root->children_.empty());
+  std::swap(root->children_, children_);
 }
 
 CFX_XMLNode* CFX_XMLNode::GetRoot() {
diff --git a/core/fxcrt/xml/cfx_xmlnode.h b/core/fxcrt/xml/cfx_xmlnode.h
index 288095403c..cbf89adb5e 100644
--- a/core/fxcrt/xml/cfx_xmlnode.h
+++ b/core/fxcrt/xml/cfx_xmlnode.h
@@ -8,6 +8,7 @@
 #define CORE_FXCRT_XML_CFX_XMLNODE_H_
 
 #include <memory>
+#include <vector>
 
 #include "core/fxcrt/fx_stream.h"
 #include "core/fxcrt/retain_ptr.h"
@@ -22,34 +23,39 @@ enum FX_XMLNODETYPE {
 
 class CFX_XMLNode {
  public:
+  using const_iterator =
+      std::vector<std::unique_ptr<CFX_XMLNode>>::const_iterator;
+
   CFX_XMLNode();
   virtual ~CFX_XMLNode();
 
   virtual FX_XMLNODETYPE GetType() const = 0;
   virtual std::unique_ptr<CFX_XMLNode> Clone() = 0;
-  virtual void Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) = 0;
+  virtual void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) = 0;
 
   CFX_XMLNode* GetRoot();
   CFX_XMLNode* GetParent() const { return parent_.Get(); }
-  CFX_XMLNode* GetFirstChild() const { return first_child_.get(); }
-  CFX_XMLNode* GetNextSibling() const { return next_sibling_.get(); }
+  CFX_XMLNode* GetNextSibling() const { return next_sibling_; }
+  bool HasChildren() const { return !children_.empty(); }
+  const_iterator begin() const { return children_.begin(); }
+  const_iterator end() const { return children_.end(); }
 
   void AppendChild(std::unique_ptr<CFX_XMLNode> pNode);
   void InsertChildNode(std::unique_ptr<CFX_XMLNode> pNode, int32_t index);
   void RemoveChildNode(CFX_XMLNode* pNode);
   void DeleteChildren();
+  // Note |root| must not have any children.
+  void MoveChildrenTo(CFX_XMLNode* root);
 
  protected:
   WideString EncodeEntities(const WideString& value);
 
  private:
-  // A node owns its first child and it owns its next sibling. The rest
-  // are unowned pointers.
   UnownedPtr<CFX_XMLNode> parent_;
-  UnownedPtr<CFX_XMLNode> last_child_;
-  UnownedPtr<CFX_XMLNode> prev_sibling_;
-  std::unique_ptr<CFX_XMLNode> first_child_;
-  std::unique_ptr<CFX_XMLNode> next_sibling_;
+  // The next_sibling is owned by the vector. We don't use an UnownedPtr
+  // because we don't know the destruction order of the vector.
+  CFX_XMLNode* next_sibling_;
+  std::vector<std::unique_ptr<CFX_XMLNode>> children_;
 };
 
 #endif  // CORE_FXCRT_XML_CFX_XMLNODE_H_
diff --git a/core/fxcrt/xml/cfx_xmltext.cpp b/core/fxcrt/xml/cfx_xmltext.cpp
index 9bed941dd4..2c324be1c9 100644
--- a/core/fxcrt/xml/cfx_xmltext.cpp
+++ b/core/fxcrt/xml/cfx_xmltext.cpp
@@ -21,7 +21,7 @@ std::unique_ptr<CFX_XMLNode> CFX_XMLText::Clone() {
   return pdfium::MakeUnique<CFX_XMLText>(m_wsText);
 }
 
-void CFX_XMLText::Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) {
+void CFX_XMLText::Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) {
   pXMLStream->WriteString(
       EncodeEntities(GetText()).UTF8Encode().AsStringView());
 }
diff --git a/core/fxcrt/xml/cfx_xmltext.h b/core/fxcrt/xml/cfx_xmltext.h
index bbf14be257..cc9c71d01c 100644
--- a/core/fxcrt/xml/cfx_xmltext.h
+++ b/core/fxcrt/xml/cfx_xmltext.h
@@ -20,7 +20,7 @@ class CFX_XMLText : public CFX_XMLNode {
   // CFX_XMLNode
   FX_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFX_XMLNode> Clone() override;
-  void Save(const RetainPtr<IFX_SeekableStream>& pXMLStream) override;
+  void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override;
 
   WideString GetText() const { return m_wsText; }
   void SetText(const WideString& wsText) { m_wsText = wsText; }