Cleanup CFX_XMLNode and friendschromium/3347

This CL removes unused methods from CFX_XMLNode, adds an AppendChild to
handle the case of a -1 index to InsertChildNode, removes the
InsertChildNode return value which is unused and cleans up various other
things.

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

9 files changed, 29 insertions, 260 deletions

diff --git a/core/fxcrt/xml/cfx_xmlchardata.h b/core/fxcrt/xml/cfx_xmlchardata.h
index 9a4710fa51..b7e691495f 100644
--- a/core/fxcrt/xml/cfx_xmlchardata.h
+++ b/core/fxcrt/xml/cfx_xmlchardata.h
@@ -17,6 +17,7 @@ class CFX_XMLCharData : public CFX_XMLText {
   explicit CFX_XMLCharData(const WideString& wsCData);
   ~CFX_XMLCharData() override;
 
+  // CFX_XMLNode
   FX_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFX_XMLNode> Clone() override;
 };
diff --git a/core/fxcrt/xml/cfx_xmldoc.cpp b/core/fxcrt/xml/cfx_xmldoc.cpp
index 36c3606017..1c3785d313 100644
--- a/core/fxcrt/xml/cfx_xmldoc.cpp
+++ b/core/fxcrt/xml/cfx_xmldoc.cpp
@@ -20,7 +20,7 @@
 
 CFX_XMLDoc::CFX_XMLDoc()
     : m_iStatus(0), m_pRoot(pdfium::MakeUnique<CFX_XMLNode>()) {
-  m_pRoot->InsertChildNode(new CFX_XMLInstruction(L"xml"));
+  m_pRoot->AppendChild(new CFX_XMLInstruction(L"xml"));
 }
 
 CFX_XMLDoc::~CFX_XMLDoc() {}
diff --git a/core/fxcrt/xml/cfx_xmlelement.cpp b/core/fxcrt/xml/cfx_xmlelement.cpp
index 1317e9a72b..bba6fe6187 100644
--- a/core/fxcrt/xml/cfx_xmlelement.cpp
+++ b/core/fxcrt/xml/cfx_xmlelement.cpp
@@ -99,5 +99,5 @@ WideString CFX_XMLElement::GetTextData() const {
 void CFX_XMLElement::SetTextData(const WideString& wsText) {
   if (wsText.GetLength() < 1)
     return;
-  InsertChildNode(new CFX_XMLText(wsText));
+  AppendChild(new CFX_XMLText(wsText));
 }
diff --git a/core/fxcrt/xml/cfx_xmlnode.cpp b/core/fxcrt/xml/cfx_xmlnode.cpp
index 41889e2d1c..19f397deb2 100644
--- a/core/fxcrt/xml/cfx_xmlnode.cpp
+++ b/core/fxcrt/xml/cfx_xmlnode.cpp
@@ -39,134 +39,51 @@ void CFX_XMLNode::DeleteChildren() {
   m_pChild = nullptr;
 }
 
-int32_t CFX_XMLNode::CountChildNodes() const {
-  int32_t iCount = 0;
-  CFX_XMLNode* pChild = m_pChild;
-  while (pChild) {
-    iCount++;
-    pChild = pChild->m_pNext;
-  }
-  return iCount;
-}
-
-CFX_XMLNode* CFX_XMLNode::GetChildNode(int32_t index) const {
-  CFX_XMLNode* pChild = m_pChild;
-  while (pChild) {
-    if (index == 0) {
-      return pChild;
-    }
-    index--;
-    pChild = pChild->m_pNext;
-  }
-  return nullptr;
-}
-
-int32_t CFX_XMLNode::GetChildNodeIndex(CFX_XMLNode* pNode) const {
-  int32_t index = 0;
-  CFX_XMLNode* pChild = m_pChild;
-  while (pChild) {
-    if (pChild == pNode) {
-      return index;
-    }
-    index++;
-    pChild = pChild->m_pNext;
-  }
-  return -1;
+void CFX_XMLNode::AppendChild(CFX_XMLNode* pNode) {
+  InsertChildNode(pNode, -1);
 }
 
-CFX_XMLNode* CFX_XMLNode::GetPath(const wchar_t* pPath,
-                                  int32_t iLength,
-                                  bool bQualifiedName) const {
-  ASSERT(pPath);
-  if (iLength < 0) {
-    iLength = wcslen(pPath);
-  }
-  if (iLength == 0) {
-    return nullptr;
-  }
-  WideString csPath;
-  const wchar_t* pStart = pPath;
-  const wchar_t* pEnd = pPath + iLength;
-  wchar_t ch;
-  while (pStart < pEnd) {
-    ch = *pStart++;
-    if (ch == L'/')
-      break;
-    csPath += ch;
-  }
-  iLength -= pStart - pPath;
-  CFX_XMLNode* pFind = nullptr;
-  if (csPath.GetLength() < 1) {
-    pFind = GetNodeItem(CFX_XMLNode::Root);
-  } else if (csPath.Compare(L"..") == 0) {
-    pFind = m_pParent;
-  } else if (csPath.Compare(L".") == 0) {
-    pFind = (CFX_XMLNode*)this;
-  } else {
-    WideString wsTag;
-    CFX_XMLNode* pNode = m_pChild;
-    while (pNode) {
-      if (pNode->GetType() == FX_XMLNODE_Element) {
-        if (bQualifiedName)
-          wsTag = static_cast<CFX_XMLElement*>(pNode)->GetName();
-        else
-          wsTag = static_cast<CFX_XMLElement*>(pNode)->GetLocalTagName();
-
-        if (wsTag.Compare(csPath) == 0) {
-          if (iLength < 1)
-            pFind = pNode;
-          else
-            pFind = pNode->GetPath(pStart, iLength, bQualifiedName);
-
-          if (pFind)
-            return pFind;
-        }
-      }
-      pNode = pNode->m_pNext;
-    }
-  }
-  if (!pFind || iLength < 1)
-    return pFind;
-  return pFind->GetPath(pStart, iLength, bQualifiedName);
-}
+void CFX_XMLNode::InsertChildNode(CFX_XMLNode* pNode, int32_t index) {
+  ASSERT(!pNode->m_pParent);
 
-int32_t CFX_XMLNode::InsertChildNode(CFX_XMLNode* pNode, int32_t index) {
   pNode->m_pParent = this;
   if (!m_pChild) {
     m_pChild = pNode;
     pNode->m_pPrior = nullptr;
     pNode->m_pNext = nullptr;
-    return 0;
+    return;
   }
   if (index == 0) {
     pNode->m_pNext = m_pChild;
     pNode->m_pPrior = nullptr;
     m_pChild->m_pPrior = pNode;
     m_pChild = pNode;
-    return 0;
+    return;
   }
+
   int32_t iCount = 0;
   CFX_XMLNode* pFind = m_pChild;
-  while (++iCount != index && pFind->m_pNext) {
+  while (++iCount != index && pFind->m_pNext)
     pFind = pFind->m_pNext;
-  }
+
   pNode->m_pPrior = pFind;
   pNode->m_pNext = pFind->m_pNext;
   if (pFind->m_pNext)
     pFind->m_pNext->m_pPrior = pNode;
   pFind->m_pNext = pNode;
-  return iCount;
 }
 
 void CFX_XMLNode::RemoveChildNode(CFX_XMLNode* pNode) {
   ASSERT(m_pChild && pNode);
-  if (m_pChild == pNode) {
+
+  if (m_pChild == pNode)
     m_pChild = pNode->m_pNext;
-  } else {
+  else
     pNode->m_pPrior->m_pNext = pNode->m_pNext;
-  }
+
   if (pNode->m_pNext)
     pNode->m_pNext->m_pPrior = pNode->m_pPrior;
+
   pNode->m_pParent = nullptr;
   pNode->m_pNext = nullptr;
   pNode->m_pPrior = nullptr;
@@ -176,153 +93,21 @@ CFX_XMLNode* CFX_XMLNode::GetNodeItem(CFX_XMLNode::NodeItem eItem) const {
   switch (eItem) {
     case CFX_XMLNode::Root: {
       CFX_XMLNode* pParent = (CFX_XMLNode*)this;
-      while (pParent->m_pParent) {
+      while (pParent->m_pParent)
         pParent = pParent->m_pParent;
-      }
+
       return pParent;
     }
     case CFX_XMLNode::Parent:
       return m_pParent;
-    case CFX_XMLNode::FirstSibling: {
-      CFX_XMLNode* pItem = (CFX_XMLNode*)this;
-      while (pItem->m_pPrior) {
-        pItem = pItem->m_pPrior;
-      }
-      return pItem == (CFX_XMLNode*)this ? nullptr : pItem;
-    }
-    case CFX_XMLNode::PriorSibling:
-      return m_pPrior;
     case CFX_XMLNode::NextSibling:
       return m_pNext;
-    case CFX_XMLNode::LastSibling: {
-      CFX_XMLNode* pItem = (CFX_XMLNode*)this;
-      while (pItem->m_pNext)
-        pItem = pItem->m_pNext;
-      return pItem == (CFX_XMLNode*)this ? nullptr : pItem;
-    }
-    case CFX_XMLNode::FirstNeighbor: {
-      CFX_XMLNode* pParent = (CFX_XMLNode*)this;
-      while (pParent->m_pParent)
-        pParent = pParent->m_pParent;
-      return pParent == (CFX_XMLNode*)this ? nullptr : pParent;
-    }
-    case CFX_XMLNode::PriorNeighbor: {
-      if (!m_pPrior)
-        return m_pParent;
-
-      CFX_XMLNode* pItem = m_pPrior;
-      while (pItem->m_pChild) {
-        pItem = pItem->m_pChild;
-        while (pItem->m_pNext)
-          pItem = pItem->m_pNext;
-      }
-      return pItem;
-    }
-    case CFX_XMLNode::NextNeighbor: {
-      if (m_pChild)
-        return m_pChild;
-      if (m_pNext)
-        return m_pNext;
-      CFX_XMLNode* pItem = m_pParent;
-      while (pItem) {
-        if (pItem->m_pNext)
-          return pItem->m_pNext;
-        pItem = pItem->m_pParent;
-      }
-      return nullptr;
-    }
-    case CFX_XMLNode::LastNeighbor: {
-      CFX_XMLNode* pItem = (CFX_XMLNode*)this;
-      while (pItem->m_pParent) {
-        pItem = pItem->m_pParent;
-      }
-      while (true) {
-        while (pItem->m_pNext)
-          pItem = pItem->m_pNext;
-        if (!pItem->m_pChild)
-          break;
-        pItem = pItem->m_pChild;
-      }
-      return pItem == (CFX_XMLNode*)this ? nullptr : pItem;
-    }
     case CFX_XMLNode::FirstChild:
       return m_pChild;
-    case CFX_XMLNode::LastChild: {
-      if (!m_pChild)
-        return nullptr;
-
-      CFX_XMLNode* pChild = m_pChild;
-      while (pChild->m_pNext)
-        pChild = pChild->m_pNext;
-      return pChild;
-    }
-    default:
-      break;
   }
   return nullptr;
 }
 
-int32_t CFX_XMLNode::GetNodeLevel() const {
-  int32_t iLevel = 0;
-  const CFX_XMLNode* pItem = m_pParent;
-  while (pItem) {
-    iLevel++;
-    pItem = pItem->m_pParent;
-  }
-  return iLevel;
-}
-
-bool CFX_XMLNode::InsertNodeItem(CFX_XMLNode::NodeItem eItem,
-                                 CFX_XMLNode* pNode) {
-  switch (eItem) {
-    case CFX_XMLNode::NextSibling: {
-      pNode->m_pParent = m_pParent;
-      pNode->m_pNext = m_pNext;
-      pNode->m_pPrior = this;
-      if (m_pNext) {
-        m_pNext->m_pPrior = pNode;
-      }
-      m_pNext = pNode;
-      return true;
-    }
-    case CFX_XMLNode::PriorSibling: {
-      pNode->m_pParent = m_pParent;
-      pNode->m_pNext = this;
-      pNode->m_pPrior = m_pPrior;
-      if (m_pPrior) {
-        m_pPrior->m_pNext = pNode;
-      } else if (m_pParent) {
-        m_pParent->m_pChild = pNode;
-      }
-      m_pPrior = pNode;
-      return true;
-    }
-    default:
-      return false;
-  }
-}
-
-CFX_XMLNode* CFX_XMLNode::RemoveNodeItem(CFX_XMLNode::NodeItem eItem) {
-  CFX_XMLNode* pNode = nullptr;
-  switch (eItem) {
-    case CFX_XMLNode::NextSibling:
-      if (m_pNext) {
-        pNode = m_pNext;
-        m_pNext = pNode->m_pNext;
-        if (m_pNext) {
-          m_pNext->m_pPrior = this;
-        }
-        pNode->m_pParent = nullptr;
-        pNode->m_pNext = nullptr;
-        pNode->m_pPrior = nullptr;
-      }
-      break;
-    default:
-      break;
-  }
-  return pNode;
-}
-
 std::unique_ptr<CFX_XMLNode> CFX_XMLNode::Clone() {
   return nullptr;
 }
diff --git a/core/fxcrt/xml/cfx_xmlnode.h b/core/fxcrt/xml/cfx_xmlnode.h
index 278b3bf15a..dc3152f19a 100644
--- a/core/fxcrt/xml/cfx_xmlnode.h
+++ b/core/fxcrt/xml/cfx_xmlnode.h
@@ -30,16 +30,8 @@ class CFX_XMLNode {
   enum NodeItem {
     Root = 0,
     Parent,
-    FirstSibling,
-    PriorSibling,
     NextSibling,
-    LastSibling,
-    FirstNeighbor,
-    PriorNeighbor,
-    NextNeighbor,
-    LastNeighbor,
     FirstChild,
-    LastChild
   };
 
   CFX_XMLNode();
@@ -48,21 +40,12 @@ class CFX_XMLNode {
   virtual FX_XMLNODETYPE GetType() const;
   virtual std::unique_ptr<CFX_XMLNode> Clone();
 
-  int32_t CountChildNodes() const;
-  CFX_XMLNode* GetChildNode(int32_t index) const;
-  int32_t GetChildNodeIndex(CFX_XMLNode* pNode) const;
-  int32_t InsertChildNode(CFX_XMLNode* pNode, int32_t index = -1);
+  void AppendChild(CFX_XMLNode* pNode);
+  void InsertChildNode(CFX_XMLNode* pNode, int32_t index);
   void RemoveChildNode(CFX_XMLNode* pNode);
   void DeleteChildren();
 
-  CFX_XMLNode* GetPath(const wchar_t* pPath,
-                       int32_t iLength = -1,
-                       bool bQualifiedName = true) const;
-
-  int32_t GetNodeLevel() const;
   CFX_XMLNode* GetNodeItem(CFX_XMLNode::NodeItem eItem) const;
-  bool InsertNodeItem(CFX_XMLNode::NodeItem eItem, CFX_XMLNode* pNode);
-  CFX_XMLNode* RemoveNodeItem(CFX_XMLNode::NodeItem eItem);
 
   void SaveXMLNode(const RetainPtr<CFX_SeekableStreamProxy>& pXMLStream);
 
diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp
index 0f08b06255..c81c4082eb 100644
--- a/core/fxcrt/xml/cfx_xmlparser.cpp
+++ b/core/fxcrt/xml/cfx_xmlparser.cpp
@@ -87,7 +87,7 @@ int32_t CFX_XMLParser::DoParser() {
         m_ws1 = m_pParser->GetTargetName();
         if (m_ws1 == L"originalXFAVersion" || m_ws1 == L"acrobat") {
           m_pChild = new CFX_XMLInstruction(m_ws1);
-          m_pParent->InsertChildNode(m_pChild);
+          m_pParent->AppendChild(m_pChild);
         } else {
           m_pChild = nullptr;
         }
@@ -96,7 +96,7 @@ int32_t CFX_XMLParser::DoParser() {
       case FX_XmlSyntaxResult::TagName:
         m_ws1 = m_pParser->GetTagName();
         m_pChild = new CFX_XMLElement(m_ws1);
-        m_pParent->InsertChildNode(m_pChild);
+        m_pParent->AppendChild(m_pChild);
         m_NodeStack.push(m_pChild);
         m_pParent = m_pChild;
 
@@ -130,13 +130,13 @@ int32_t CFX_XMLParser::DoParser() {
       case FX_XmlSyntaxResult::Text:
         m_ws1 = m_pParser->GetTextData();
         m_pChild = new CFX_XMLText(m_ws1);
-        m_pParent->InsertChildNode(m_pChild);
+        m_pParent->AppendChild(m_pChild);
         m_pChild = m_pParent;
         break;
       case FX_XmlSyntaxResult::CData:
         m_ws1 = m_pParser->GetTextData();
         m_pChild = new CFX_XMLCharData(m_ws1);
-        m_pParent->InsertChildNode(m_pChild);
+        m_pParent->AppendChild(m_pChild);
         m_pChild = m_pParent;
         break;
       case FX_XmlSyntaxResult::TargetData:
diff --git a/fxjs/xfa/cjx_node.cpp b/fxjs/xfa/cjx_node.cpp
index b1749d6025..e5396b7c90 100644
--- a/fxjs/xfa/cjx_node.cpp
+++ b/fxjs/xfa/cjx_node.cpp
@@ -251,7 +251,7 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime,
       CFX_XMLNode* pXMLSibling =
           pXMLChild->GetNodeItem(CFX_XMLNode::NextSibling);
       pXMLNode->RemoveChildNode(pXMLChild);
-      pFakeXMLRoot->InsertChildNode(pXMLChild);
+      pFakeXMLRoot->AppendChild(pXMLChild);
       pXMLChild = pXMLSibling;
     }
   } else {
@@ -259,7 +259,7 @@ CJS_Return CJX_Node::loadXML(CFX_V8* runtime,
     if (pXMLParent)
       pXMLParent->RemoveChildNode(pXMLNode);
 
-    pFakeXMLRoot->InsertChildNode(pXMLNode);
+    pFakeXMLRoot->AppendChild(pXMLNode);
   }
 
   pParser->ConstructXFANode(pFakeRoot, pFakeXMLRoot.get());
diff --git a/xfa/fxfa/parser/cxfa_simple_parser.cpp b/xfa/fxfa/parser/cxfa_simple_parser.cpp
index 1864532f28..0bce147223 100644
--- a/xfa/fxfa/parser/cxfa_simple_parser.cpp
+++ b/xfa/fxfa/parser/cxfa_simple_parser.cpp
@@ -768,7 +768,7 @@ CXFA_Node* CXFA_SimpleParser::ParseAsXDPPacket_Data(
       static_cast<CFX_XMLElement*>(pXMLDocumentNode)
           ->RemoveAttribute(L"xmlns:xfa");
     }
-    pDataElement->InsertChildNode(pXMLDocumentNode);
+    pDataElement->AppendChild(pXMLDocumentNode);
     pDataXMLNode = pDataElement;
   }
 
diff --git a/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp b/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp
index 5b098cce42..fed59158aa 100644
--- a/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp
+++ b/xfa/fxfa/parser/xfa_document_datamerger_imp.cpp
@@ -1375,7 +1375,7 @@ void CXFA_Document::DoDataMerge() {
         CreateNode(XFA_PacketType::Datasets, XFA_Element::DataModel);
     pDatasetsRoot->JSObject()->SetCData(XFA_Attribute::Name, L"datasets", false,
                                         false);
-    m_pRootNode->GetXMLMappingNode()->InsertChildNode(pDatasetsXMLNode);
+    m_pRootNode->GetXMLMappingNode()->AppendChild(pDatasetsXMLNode);
     m_pRootNode->InsertChild(pDatasetsRoot, nullptr);
     pDatasetsRoot->SetXMLMappingNode(pDatasetsXMLNode);
   }