Move XML attribute handling to a base class.

This CL moves the attribute handling out of CFDE_XMLElement and
CFDE_XMLInstruction into a common CFDE_XMLAttributeNode. The handling is
also converted to a std::map from either a) a vector storing
name,value,name,value or b) two vectors one for names and the other for
values.

The unused Get/Set methods for interger and float are removed and the
iteration is converted to use iterators.

Change-Id: Icda00ae898a595d58b06af0ced337f55f47c317c
Reviewed-on: https://pdfium-review.googlesource.com/3753
Reviewed-by: Nicolás Peña <npm@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

9 files changed, 174 insertions, 378 deletions

diff --git a/xfa/fde/xml/cfde_xmlattributenode.cpp b/xfa/fde/xml/cfde_xmlattributenode.cpp
new file mode 100644
index 0000000000..0bfa949b12
--- /dev/null
+++ b/xfa/fde/xml/cfde_xmlattributenode.cpp
@@ -0,0 +1,35 @@
+// Copyright 2017 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
+
+#include "xfa/fde/xml/cfde_xmlattributenode.h"
+
+#include "core/fxcrt/fx_ext.h"
+
+CFDE_XMLAttributeNode::CFDE_XMLAttributeNode(const CFX_WideString& name)
+    : CFDE_XMLNode(), name_(name) {
+  ASSERT(name_.GetLength() > 0);
+}
+
+CFDE_XMLAttributeNode::~CFDE_XMLAttributeNode() {}
+
+bool CFDE_XMLAttributeNode::HasAttribute(const CFX_WideString& name) const {
+  return attrs_.find(name) != attrs_.end();
+}
+
+CFX_WideString CFDE_XMLAttributeNode::GetString(
+    const CFX_WideString& name) const {
+  auto it = attrs_.find(name);
+  return it != attrs_.end() ? it->second : CFX_WideString();
+}
+
+void CFDE_XMLAttributeNode::SetString(const CFX_WideString& name,
+                                      const CFX_WideString& value) {
+  attrs_[name] = value;
+}
+
+void CFDE_XMLAttributeNode::RemoveAttribute(const CFX_WideString& name) {
+  attrs_.erase(name);
+}
diff --git a/xfa/fde/xml/cfde_xmlattributenode.h b/xfa/fde/xml/cfde_xmlattributenode.h
new file mode 100644
index 0000000000..07a1ef1610
--- /dev/null
+++ b/xfa/fde/xml/cfde_xmlattributenode.h
@@ -0,0 +1,44 @@
+// Copyright 2017 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
+
+#ifndef XFA_FDE_XML_CFDE_XMLATTRIBUTENODE_H_
+#define XFA_FDE_XML_CFDE_XMLATTRIBUTENODE_H_
+
+#include <map>
+#include <memory>
+
+#include "core/fxcrt/fx_string.h"
+#include "xfa/fde/xml/cfde_xmlnode.h"
+
+class CFDE_XMLAttributeNode : public CFDE_XMLNode {
+ public:
+  explicit CFDE_XMLAttributeNode(const CFX_WideString& name);
+  ~CFDE_XMLAttributeNode() override;
+
+  // CFDE_XMLNode
+  FDE_XMLNODETYPE GetType() const override = 0;
+  std::unique_ptr<CFDE_XMLNode> Clone() override = 0;
+
+  CFX_WideString GetName() const { return name_; }
+  const std::map<CFX_WideString, CFX_WideString>& GetAttributes() const {
+    return attrs_;
+  }
+  void SetAttributes(const std::map<CFX_WideString, CFX_WideString>& attrs) {
+    attrs_ = attrs;
+  }
+  bool HasAttribute(const CFX_WideString& name) const;
+
+  void SetString(const CFX_WideString& name, const CFX_WideString& value);
+  CFX_WideString GetString(const CFX_WideString& name) const;
+
+  void RemoveAttribute(const CFX_WideString& name);
+
+ private:
+  CFX_WideString name_;
+  std::map<CFX_WideString, CFX_WideString> attrs_;
+};
+
+#endif  // XFA_FDE_XML_CFDE_XMLATTRIBUTENODE_H_
diff --git a/xfa/fde/xml/cfde_xmldoc.cpp b/xfa/fde/xml/cfde_xmldoc.cpp
index 5d427fb06e..7032700578 100644
--- a/xfa/fde/xml/cfde_xmldoc.cpp
+++ b/xfa/fde/xml/cfde_xmldoc.cpp
@@ -54,7 +54,7 @@ void CFDE_XMLDoc::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream,
     case FDE_XMLNODE_Instruction: {
       CFX_WideString ws;
       CFDE_XMLInstruction* pInstruction = (CFDE_XMLInstruction*)pNode;
-      if (pInstruction->m_wsTarget.CompareNoCase(L"xml") == 0) {
+      if (pInstruction->GetName().CompareNoCase(L"xml") == 0) {
         ws = L"<?xml version=\"1.0\" encoding=\"";
         uint16_t wCodePage = pXMLStream->GetCodePage();
         if (wCodePage == FX_CODEPAGE_UTF16LE) {
@@ -67,31 +67,28 @@ void CFDE_XMLDoc::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream,
         ws += L"\"?>";
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
       } else {
-        ws.Format(L"<?%s", pInstruction->m_wsTarget.c_str());
+        ws.Format(L"<?%s", pInstruction->GetName().c_str());
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
-        std::vector<CFX_WideString>& attributes = pInstruction->m_Attributes;
-        int32_t i;
-        int32_t iCount = pdfium::CollectionSize<int32_t>(attributes);
-        CFX_WideString wsValue;
-        for (i = 0; i < iCount; i += 2) {
-          ws = L" ";
-          ws += attributes[i];
-          ws += L"=\"";
-          wsValue = attributes[i + 1];
+
+        for (auto it : pInstruction->GetAttributes()) {
+          CFX_WideString wsValue = it.second;
           wsValue.Replace(L"&", L"&amp;");
           wsValue.Replace(L"<", L"&lt;");
           wsValue.Replace(L">", L"&gt;");
           wsValue.Replace(L"\'", L"&apos;");
           wsValue.Replace(L"\"", L"&quot;");
+
+          ws = L" ";
+          ws += it.first;
+          ws += L"=\"";
           ws += wsValue;
           ws += L"\"";
           pXMLStream->WriteString(ws.c_str(), ws.GetLength());
         }
-        std::vector<CFX_WideString>& targetdata = pInstruction->m_TargetData;
-        iCount = pdfium::CollectionSize<int32_t>(targetdata);
-        for (i = 0; i < iCount; i++) {
+
+        for (auto target : pInstruction->GetTargetData()) {
           ws = L" \"";
-          ws += targetdata[i];
+          ws += target;
           ws += L"\"";
           pXMLStream->WriteString(ws.c_str(), ws.GetLength());
         }
@@ -103,22 +100,20 @@ void CFDE_XMLDoc::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream,
     case FDE_XMLNODE_Element: {
       CFX_WideString ws;
       ws = L"<";
-      ws += ((CFDE_XMLElement*)pNode)->m_wsTag;
+      ws += static_cast<CFDE_XMLElement*>(pNode)->GetName();
       pXMLStream->WriteString(ws.c_str(), ws.GetLength());
-      std::vector<CFX_WideString>& attributes =
-          static_cast<CFDE_XMLElement*>(pNode)->m_Attributes;
-      int32_t iCount = pdfium::CollectionSize<int32_t>(attributes);
-      CFX_WideString wsValue;
-      for (int32_t i = 0; i < iCount; i += 2) {
-        ws = L" ";
-        ws += attributes[i];
-        ws += L"=\"";
-        wsValue = attributes[i + 1];
+
+      for (auto it : static_cast<CFDE_XMLElement*>(pNode)->GetAttributes()) {
+        CFX_WideString wsValue = it.second;
         wsValue.Replace(L"&", L"&amp;");
         wsValue.Replace(L"<", L"&lt;");
         wsValue.Replace(L">", L"&gt;");
         wsValue.Replace(L"\'", L"&apos;");
         wsValue.Replace(L"\"", L"&quot;");
+
+        ws = L" ";
+        ws += it.first;
+        ws += L"=\"";
         ws += wsValue;
         ws += L"\"";
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
@@ -132,7 +127,7 @@ void CFDE_XMLDoc::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream,
           pChild = pChild->m_pNext;
         }
         ws = L"</";
-        ws += ((CFDE_XMLElement*)pNode)->m_wsTag;
+        ws += static_cast<CFDE_XMLElement*>(pNode)->GetName();
         ws += L"\n>";
       } else {
         ws = L"\n/>";
diff --git a/xfa/fde/xml/cfde_xmlelement.cpp b/xfa/fde/xml/cfde_xmlelement.cpp
index 646eea2ee2..560cf89280 100644
--- a/xfa/fde/xml/cfde_xmlelement.cpp
+++ b/xfa/fde/xml/cfde_xmlelement.cpp
@@ -13,9 +13,7 @@
 #include "xfa/fde/xml/cfde_xmltext.h"
 
 CFDE_XMLElement::CFDE_XMLElement(const CFX_WideString& wsTag)
-    : CFDE_XMLNode(), m_wsTag(wsTag), m_Attributes() {
-  ASSERT(m_wsTag.GetLength() > 0);
-}
+    : CFDE_XMLAttributeNode(wsTag) {}
 
 CFDE_XMLElement::~CFDE_XMLElement() {}
 
@@ -24,8 +22,8 @@ FDE_XMLNODETYPE CFDE_XMLElement::GetType() const {
 }
 
 std::unique_ptr<CFDE_XMLNode> CFDE_XMLElement::Clone() {
-  auto pClone = pdfium::MakeUnique<CFDE_XMLElement>(m_wsTag);
-  pClone->m_Attributes = m_Attributes;
+  auto pClone = pdfium::MakeUnique<CFDE_XMLElement>(GetName());
+  pClone->SetAttributes(GetAttributes());
 
   CFX_WideString wsText;
   CFDE_XMLNode* pChild = m_pChild;
@@ -43,155 +41,44 @@ std::unique_ptr<CFDE_XMLNode> CFDE_XMLElement::Clone() {
   return pClone;
 }
 
-void CFDE_XMLElement::GetTagName(CFX_WideString& wsTag) const {
-  wsTag = m_wsTag;
-}
-
-void CFDE_XMLElement::GetLocalTagName(CFX_WideString& wsTag) const {
-  FX_STRSIZE iFind = m_wsTag.Find(L':', 0);
-  if (iFind < 0) {
-    wsTag = m_wsTag;
-  } else {
-    wsTag = m_wsTag.Right(m_wsTag.GetLength() - iFind - 1);
-  }
+CFX_WideString CFDE_XMLElement::GetLocalTagName() const {
+  FX_STRSIZE iFind = GetName().Find(L':', 0);
+  if (iFind < 0)
+    return GetName();
+  return GetName().Right(GetName().GetLength() - iFind - 1);
 }
 
-void CFDE_XMLElement::GetNamespacePrefix(CFX_WideString& wsPrefix) const {
-  FX_STRSIZE iFind = m_wsTag.Find(L':', 0);
-  if (iFind < 0) {
-    wsPrefix.clear();
-  } else {
-    wsPrefix = m_wsTag.Left(iFind);
-  }
+CFX_WideString CFDE_XMLElement::GetNamespacePrefix() const {
+  FX_STRSIZE iFind = GetName().Find(L':', 0);
+  if (iFind < 0)
+    return CFX_WideString();
+  return GetName().Left(iFind);
 }
 
-void CFDE_XMLElement::GetNamespaceURI(CFX_WideString& wsNamespace) const {
-  CFX_WideString wsAttri(L"xmlns"), wsPrefix;
-  GetNamespacePrefix(wsPrefix);
+CFX_WideString CFDE_XMLElement::GetNamespaceURI() const {
+  CFX_WideString wsAttri(L"xmlns");
+  CFX_WideString wsPrefix = GetNamespacePrefix();
   if (wsPrefix.GetLength() > 0) {
     wsAttri += L":";
     wsAttri += wsPrefix;
   }
-  wsNamespace.clear();
-  CFDE_XMLNode* pNode = (CFDE_XMLNode*)this;
+
+  auto* pNode = static_cast<const CFDE_XMLNode*>(this);
   while (pNode) {
-    if (pNode->GetType() != FDE_XMLNODE_Element) {
+    if (pNode->GetType() != FDE_XMLNODE_Element)
       break;
-    }
-    CFDE_XMLElement* pElement = (CFDE_XMLElement*)pNode;
-    if (!pElement->HasAttribute(wsAttri.c_str())) {
+
+    auto* pElement = static_cast<const CFDE_XMLElement*>(pNode);
+    if (!pElement->HasAttribute(wsAttri)) {
       pNode = pNode->GetNodeItem(CFDE_XMLNode::Parent);
       continue;
     }
-    pElement->GetString(wsAttri.c_str(), wsNamespace);
-    break;
-  }
-}
-
-int32_t CFDE_XMLElement::CountAttributes() const {
-  return pdfium::CollectionSize<int32_t>(m_Attributes) / 2;
-}
-
-bool CFDE_XMLElement::GetAttribute(int32_t index,
-                                   CFX_WideString& wsAttriName,
-                                   CFX_WideString& wsAttriValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  ASSERT(index > -1 && index < iCount / 2);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (index == 0) {
-      wsAttriName = m_Attributes[i];
-      wsAttriValue = m_Attributes[i + 1];
-      return true;
-    }
-    index--;
-  }
-  return false;
-}
-
-bool CFDE_XMLElement::HasAttribute(const wchar_t* pwsAttriName) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0)
-      return true;
-  }
-  return false;
-}
-
-void CFDE_XMLElement::GetString(const wchar_t* pwsAttriName,
-                                CFX_WideString& wsAttriValue,
-                                const wchar_t* pwsDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      wsAttriValue = m_Attributes[i + 1];
-      return;
-    }
+    return pElement->GetString(wsAttri);
   }
-  wsAttriValue = pwsDefValue;
+  return CFX_WideString();
 }
 
-void CFDE_XMLElement::SetString(const CFX_WideString& wsAttriName,
-                                const CFX_WideString& wsAttriValue) {
-  ASSERT(wsAttriName.GetLength() > 0);
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(wsAttriName) == 0) {
-      m_Attributes[i] = wsAttriName;
-      m_Attributes[i + 1] = wsAttriValue;
-      return;
-    }
-  }
-  m_Attributes.push_back(wsAttriName);
-  m_Attributes.push_back(wsAttriValue);
-}
-
-int32_t CFDE_XMLElement::GetInteger(const wchar_t* pwsAttriName,
-                                    int32_t iDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      return FXSYS_wtoi(m_Attributes[i + 1].c_str());
-    }
-  }
-  return iDefValue;
-}
-
-void CFDE_XMLElement::SetInteger(const wchar_t* pwsAttriName,
-                                 int32_t iAttriValue) {
-  CFX_WideString wsValue;
-  wsValue.Format(L"%d", iAttriValue);
-  SetString(pwsAttriName, wsValue);
-}
-
-float CFDE_XMLElement::GetFloat(const wchar_t* pwsAttriName,
-                                float fDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      return FXSYS_wcstof(m_Attributes[i + 1].c_str(), -1, nullptr);
-    }
-  }
-  return fDefValue;
-}
-
-void CFDE_XMLElement::SetFloat(const wchar_t* pwsAttriName, float fAttriValue) {
-  CFX_WideString wsValue;
-  wsValue.Format(L"%f", fAttriValue);
-  SetString(pwsAttriName, wsValue);
-}
-
-void CFDE_XMLElement::RemoveAttribute(const wchar_t* pwsAttriName) {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      m_Attributes.erase(m_Attributes.begin() + i,
-                         m_Attributes.begin() + i + 2);
-      return;
-    }
-  }
-}
-
-void CFDE_XMLElement::GetTextData(CFX_WideString& wsText) const {
+CFX_WideString CFDE_XMLElement::GetTextData() const {
   CFX_WideTextBuf buffer;
   CFDE_XMLNode* pChild = m_pChild;
   while (pChild) {
@@ -205,12 +92,11 @@ void CFDE_XMLElement::GetTextData(CFX_WideString& wsText) const {
     }
     pChild = pChild->m_pNext;
   }
-  wsText = buffer.AsStringC();
+  return buffer.MakeString();
 }
 
 void CFDE_XMLElement::SetTextData(const CFX_WideString& wsText) {
-  if (wsText.GetLength() < 1) {
+  if (wsText.GetLength() < 1)
     return;
-  }
   InsertChildNode(new CFDE_XMLText(wsText));
 }
diff --git a/xfa/fde/xml/cfde_xmlelement.h b/xfa/fde/xml/cfde_xmlelement.h
index c4c522e8a5..a891ce8928 100644
--- a/xfa/fde/xml/cfde_xmlelement.h
+++ b/xfa/fde/xml/cfde_xmlelement.h
@@ -11,9 +11,9 @@
 #include <vector>
 
 #include "core/fxcrt/fx_string.h"
-#include "xfa/fde/xml/cfde_xmlnode.h"
+#include "xfa/fde/xml/cfde_xmlattributenode.h"
 
-class CFDE_XMLElement : public CFDE_XMLNode {
+class CFDE_XMLElement : public CFDE_XMLAttributeNode {
  public:
   explicit CFDE_XMLElement(const CFX_WideString& wsTag);
   ~CFDE_XMLElement() override;
@@ -22,36 +22,12 @@ class CFDE_XMLElement : public CFDE_XMLNode {
   FDE_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFDE_XMLNode> Clone() override;
 
-  void GetTagName(CFX_WideString& wsTag) const;
-  void GetLocalTagName(CFX_WideString& wsTag) const;
+  CFX_WideString GetLocalTagName() const;
+  CFX_WideString GetNamespacePrefix() const;
+  CFX_WideString GetNamespaceURI() const;
 
-  void GetNamespacePrefix(CFX_WideString& wsPrefix) const;
-  void GetNamespaceURI(CFX_WideString& wsNamespace) const;
-
-  int32_t CountAttributes() const;
-  bool GetAttribute(int32_t index,
-                    CFX_WideString& wsAttriName,
-                    CFX_WideString& wsAttriValue) const;
-  bool HasAttribute(const wchar_t* pwsAttriName) const;
-  void RemoveAttribute(const wchar_t* pwsAttriName);
-
-  void GetString(const wchar_t* pwsAttriName,
-                 CFX_WideString& wsAttriValue,
-                 const wchar_t* pwsDefValue = nullptr) const;
-  void SetString(const CFX_WideString& wsAttriName,
-                 const CFX_WideString& wsAttriValue);
-
-  int32_t GetInteger(const wchar_t* pwsAttriName, int32_t iDefValue = 0) const;
-  void SetInteger(const wchar_t* pwsAttriName, int32_t iAttriValue);
-
-  float GetFloat(const wchar_t* pwsAttriName, float fDefValue = 0) const;
-  void SetFloat(const wchar_t* pwsAttriName, float fAttriValue);
-
-  void GetTextData(CFX_WideString& wsText) const;
+  CFX_WideString GetTextData() const;
   void SetTextData(const CFX_WideString& wsText);
-
-  CFX_WideString m_wsTag;
-  std::vector<CFX_WideString> m_Attributes;
 };
 
 #endif  // XFA_FDE_XML_CFDE_XMLELEMENT_H_
diff --git a/xfa/fde/xml/cfde_xmlinstruction.cpp b/xfa/fde/xml/cfde_xmlinstruction.cpp
index 2229b4768a..d289d9e88a 100644
--- a/xfa/fde/xml/cfde_xmlinstruction.cpp
+++ b/xfa/fde/xml/cfde_xmlinstruction.cpp
@@ -11,9 +11,7 @@
 #include "third_party/base/stl_util.h"
 
 CFDE_XMLInstruction::CFDE_XMLInstruction(const CFX_WideString& wsTarget)
-    : m_wsTarget(wsTarget) {
-  ASSERT(m_wsTarget.GetLength() > 0);
-}
+    : CFDE_XMLAttributeNode(wsTarget) {}
 
 CFDE_XMLInstruction::~CFDE_XMLInstruction() {}
 
@@ -22,129 +20,12 @@ FDE_XMLNODETYPE CFDE_XMLInstruction::GetType() const {
 }
 
 std::unique_ptr<CFDE_XMLNode> CFDE_XMLInstruction::Clone() {
-  auto pClone = pdfium::MakeUnique<CFDE_XMLInstruction>(m_wsTarget);
-  pClone->m_Attributes = m_Attributes;
+  auto pClone = pdfium::MakeUnique<CFDE_XMLInstruction>(GetName());
+  pClone->SetAttributes(GetAttributes());
   pClone->m_TargetData = m_TargetData;
   return pClone;
 }
 
-int32_t CFDE_XMLInstruction::CountAttributes() const {
-  return pdfium::CollectionSize<int32_t>(m_Attributes) / 2;
-}
-
-bool CFDE_XMLInstruction::GetAttribute(int32_t index,
-                                       CFX_WideString& wsAttriName,
-                                       CFX_WideString& wsAttriValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  ASSERT(index > -1 && index < iCount / 2);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (index == 0) {
-      wsAttriName = m_Attributes[i];
-      wsAttriValue = m_Attributes[i + 1];
-      return true;
-    }
-    index--;
-  }
-  return false;
-}
-
-bool CFDE_XMLInstruction::HasAttribute(const wchar_t* pwsAttriName) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      return true;
-    }
-  }
-  return false;
-}
-
-void CFDE_XMLInstruction::GetString(const wchar_t* pwsAttriName,
-                                    CFX_WideString& wsAttriValue,
-                                    const wchar_t* pwsDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      wsAttriValue = m_Attributes[i + 1];
-      return;
-    }
-  }
-  wsAttriValue = pwsDefValue;
-}
-
-void CFDE_XMLInstruction::SetString(const CFX_WideString& wsAttriName,
-                                    const CFX_WideString& wsAttriValue) {
-  ASSERT(wsAttriName.GetLength() > 0);
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(wsAttriName) == 0) {
-      m_Attributes[i] = wsAttriName;
-      m_Attributes[i + 1] = wsAttriValue;
-      return;
-    }
-  }
-  m_Attributes.push_back(wsAttriName);
-  m_Attributes.push_back(wsAttriValue);
-}
-
-int32_t CFDE_XMLInstruction::GetInteger(const wchar_t* pwsAttriName,
-                                        int32_t iDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      return FXSYS_wtoi(m_Attributes[i + 1].c_str());
-    }
-  }
-  return iDefValue;
-}
-
-void CFDE_XMLInstruction::SetInteger(const wchar_t* pwsAttriName,
-                                     int32_t iAttriValue) {
-  CFX_WideString wsValue;
-  wsValue.Format(L"%d", iAttriValue);
-  SetString(pwsAttriName, wsValue);
-}
-
-float CFDE_XMLInstruction::GetFloat(const wchar_t* pwsAttriName,
-                                    float fDefValue) const {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      return FXSYS_wcstof(m_Attributes[i + 1].c_str(), -1, nullptr);
-    }
-  }
-  return fDefValue;
-}
-
-void CFDE_XMLInstruction::SetFloat(const wchar_t* pwsAttriName,
-                                   float fAttriValue) {
-  CFX_WideString wsValue;
-  wsValue.Format(L"%f", fAttriValue);
-  SetString(pwsAttriName, wsValue);
-}
-
-void CFDE_XMLInstruction::RemoveAttribute(const wchar_t* pwsAttriName) {
-  int32_t iCount = pdfium::CollectionSize<int32_t>(m_Attributes);
-  for (int32_t i = 0; i < iCount; i += 2) {
-    if (m_Attributes[i].Compare(pwsAttriName) == 0) {
-      m_Attributes.erase(m_Attributes.begin() + i,
-                         m_Attributes.begin() + i + 2);
-      return;
-    }
-  }
-}
-
-int32_t CFDE_XMLInstruction::CountData() const {
-  return pdfium::CollectionSize<int32_t>(m_TargetData);
-}
-
-bool CFDE_XMLInstruction::GetData(int32_t index, CFX_WideString& wsData) const {
-  if (!pdfium::IndexInBounds(m_TargetData, index))
-    return false;
-
-  wsData = m_TargetData[index];
-  return true;
-}
-
 void CFDE_XMLInstruction::AppendData(const CFX_WideString& wsData) {
   m_TargetData.push_back(wsData);
 }
diff --git a/xfa/fde/xml/cfde_xmlinstruction.h b/xfa/fde/xml/cfde_xmlinstruction.h
index 4e891e1fac..e9c4ad3dfe 100644
--- a/xfa/fde/xml/cfde_xmlinstruction.h
+++ b/xfa/fde/xml/cfde_xmlinstruction.h
@@ -11,9 +11,9 @@
 #include <vector>
 
 #include "core/fxcrt/fx_string.h"
-#include "xfa/fde/xml/cfde_xmlnode.h"
+#include "xfa/fde/xml/cfde_xmlattributenode.h"
 
-class CFDE_XMLInstruction : public CFDE_XMLNode {
+class CFDE_XMLInstruction : public CFDE_XMLAttributeNode {
  public:
   explicit CFDE_XMLInstruction(const CFX_WideString& wsTarget);
   ~CFDE_XMLInstruction() override;
@@ -22,29 +22,13 @@ class CFDE_XMLInstruction : public CFDE_XMLNode {
   FDE_XMLNODETYPE GetType() const override;
   std::unique_ptr<CFDE_XMLNode> Clone() override;
 
-  void GetTargetName(CFX_WideString& wsTarget) const { wsTarget = m_wsTarget; }
-  int32_t CountAttributes() const;
-  bool GetAttribute(int32_t index,
-                    CFX_WideString& wsAttriName,
-                    CFX_WideString& wsAttriValue) const;
-  bool HasAttribute(const wchar_t* pwsAttriName) const;
-  void GetString(const wchar_t* pwsAttriName,
-                 CFX_WideString& wsAttriValue,
-                 const wchar_t* pwsDefValue = nullptr) const;
-  void SetString(const CFX_WideString& wsAttriName,
-                 const CFX_WideString& wsAttriValue);
-  int32_t GetInteger(const wchar_t* pwsAttriName, int32_t iDefValue = 0) const;
-  void SetInteger(const wchar_t* pwsAttriName, int32_t iAttriValue);
-  float GetFloat(const wchar_t* pwsAttriName, float fDefValue = 0) const;
-  void SetFloat(const wchar_t* pwsAttriName, float fAttriValue);
-  void RemoveAttribute(const wchar_t* pwsAttriName);
-  int32_t CountData() const;
-  bool GetData(int32_t index, CFX_WideString& wsData) const;
+  const std::vector<CFX_WideString>& GetTargetData() const {
+    return m_TargetData;
+  }
   void AppendData(const CFX_WideString& wsData);
   void RemoveData(int32_t index);
 
-  CFX_WideString m_wsTarget;
-  std::vector<CFX_WideString> m_Attributes;
+ private:
   std::vector<CFX_WideString> m_TargetData;
 };
 
diff --git a/xfa/fde/xml/cfde_xmlnode.cpp b/xfa/fde/xml/cfde_xmlnode.cpp
index 22bd262bf2..82db939a23 100644
--- a/xfa/fde/xml/cfde_xmlnode.cpp
+++ b/xfa/fde/xml/cfde_xmlnode.cpp
@@ -109,17 +109,17 @@ CFDE_XMLNode* CFDE_XMLNode::GetPath(const wchar_t* pPath,
     CFDE_XMLNode* pNode = m_pChild;
     while (pNode) {
       if (pNode->GetType() == FDE_XMLNODE_Element) {
-        if (bQualifiedName) {
-          ((CFDE_XMLElement*)pNode)->GetTagName(wsTag);
-        } else {
-          ((CFDE_XMLElement*)pNode)->GetLocalTagName(wsTag);
-        }
+        if (bQualifiedName)
+          wsTag = static_cast<CFDE_XMLElement*>(pNode)->GetName();
+        else
+          wsTag = static_cast<CFDE_XMLElement*>(pNode)->GetLocalTagName();
+
         if (wsTag.Compare(csPath) == 0) {
-          if (iLength < 1) {
+          if (iLength < 1)
             pFind = pNode;
-          } else {
+          else
             pFind = pNode->GetPath(pStart, iLength, bQualifiedName);
-          }
+
           if (pFind)
             return pFind;
         }
@@ -335,7 +335,7 @@ void CFDE_XMLNode::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream) {
     case FDE_XMLNODE_Instruction: {
       CFX_WideString ws;
       CFDE_XMLInstruction* pInstruction = (CFDE_XMLInstruction*)pNode;
-      if (pInstruction->m_wsTarget.CompareNoCase(L"xml") == 0) {
+      if (pInstruction->GetName().CompareNoCase(L"xml") == 0) {
         ws = L"<?xml version=\"1.0\" encoding=\"";
         uint16_t wCodePage = pXMLStream->GetCodePage();
         if (wCodePage == FX_CODEPAGE_UTF16LE) {
@@ -348,31 +348,28 @@ void CFDE_XMLNode::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream) {
         ws += L"\"?>";
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
       } else {
-        ws.Format(L"<?%s", pInstruction->m_wsTarget.c_str());
+        ws.Format(L"<?%s", pInstruction->GetName().c_str());
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
-        std::vector<CFX_WideString>& attributes = pInstruction->m_Attributes;
-        int32_t i;
-        int32_t iCount = pdfium::CollectionSize<int32_t>(attributes);
-        CFX_WideString wsValue;
-        for (i = 0; i < iCount; i += 2) {
-          ws = L" ";
-          ws += attributes[i];
-          ws += L"=\"";
-          wsValue = attributes[i + 1];
+
+        for (auto it : pInstruction->GetAttributes()) {
+          CFX_WideString wsValue = it.second;
           wsValue.Replace(L"&", L"&amp;");
           wsValue.Replace(L"<", L"&lt;");
           wsValue.Replace(L">", L"&gt;");
           wsValue.Replace(L"\'", L"&apos;");
           wsValue.Replace(L"\"", L"&quot;");
+
+          ws = L" ";
+          ws += it.first;
+          ws += L"=\"";
           ws += wsValue;
           ws += L"\"";
           pXMLStream->WriteString(ws.c_str(), ws.GetLength());
         }
-        std::vector<CFX_WideString>& targetdata = pInstruction->m_TargetData;
-        iCount = pdfium::CollectionSize<int32_t>(targetdata);
-        for (i = 0; i < iCount; i++) {
+
+        for (auto target : pInstruction->GetTargetData()) {
           ws = L" \"";
-          ws += targetdata[i];
+          ws += target;
           ws += L"\"";
           pXMLStream->WriteString(ws.c_str(), ws.GetLength());
         }
@@ -384,22 +381,20 @@ void CFDE_XMLNode::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream) {
     case FDE_XMLNODE_Element: {
       CFX_WideString ws;
       ws = L"<";
-      ws += ((CFDE_XMLElement*)pNode)->m_wsTag;
+      ws += static_cast<CFDE_XMLElement*>(pNode)->GetName();
       pXMLStream->WriteString(ws.c_str(), ws.GetLength());
-      std::vector<CFX_WideString>& attributes =
-          static_cast<CFDE_XMLElement*>(pNode)->m_Attributes;
-      int32_t iCount = pdfium::CollectionSize<int32_t>(attributes);
-      CFX_WideString wsValue;
-      for (int32_t i = 0; i < iCount; i += 2) {
-        ws = L" ";
-        ws += attributes[i];
-        ws += L"=\"";
-        wsValue = attributes[i + 1];
+
+      for (auto it : static_cast<CFDE_XMLElement*>(pNode)->GetAttributes()) {
+        CFX_WideString wsValue = it.second;
         wsValue.Replace(L"&", L"&amp;");
         wsValue.Replace(L"<", L"&lt;");
         wsValue.Replace(L">", L"&gt;");
         wsValue.Replace(L"\'", L"&apos;");
         wsValue.Replace(L"\"", L"&quot;");
+
+        ws = L" ";
+        ws += it.first;
+        ws += L"=\"";
         ws += wsValue;
         ws += L"\"";
         pXMLStream->WriteString(ws.c_str(), ws.GetLength());
@@ -413,7 +408,7 @@ void CFDE_XMLNode::SaveXMLNode(const CFX_RetainPtr<IFGAS_Stream>& pXMLStream) {
           pChild = pChild->m_pNext;
         }
         ws = L"</";
-        ws += ((CFDE_XMLElement*)pNode)->m_wsTag;
+        ws += static_cast<CFDE_XMLElement*>(pNode)->GetName();
         ws += L"\n>";
       } else {
         ws = L"\n/>";
diff --git a/xfa/fde/xml/cfde_xmlparser.cpp b/xfa/fde/xml/cfde_xmlparser.cpp
index db85021693..ad7b9e1a6f 100644
--- a/xfa/fde/xml/cfde_xmlparser.cpp
+++ b/xfa/fde/xml/cfde_xmlparser.cpp
@@ -64,7 +64,7 @@ int32_t CFDE_XMLParser::DoParser(IFX_Pause* pPause) {
           break;
         }
         m_pParser->GetTagName(m_ws1);
-        static_cast<CFDE_XMLElement*>(m_pChild)->GetTagName(m_ws2);
+        m_ws2 = static_cast<CFDE_XMLElement*>(m_pChild)->GetName();
         if (m_ws1.GetLength() > 0 && m_ws1 != m_ws2) {
           m_syntaxParserResult = FDE_XmlSyntaxResult::Error;
           break;
@@ -102,8 +102,8 @@ int32_t CFDE_XMLParser::DoParser(IFX_Pause* pPause) {
         m_pParent = m_pChild;
 
         if (m_dwCheckStatus != 0x03 && m_NodeStack.size() == 3) {
-          CFX_WideString wsTag;
-          static_cast<CFDE_XMLElement*>(m_pChild)->GetLocalTagName(wsTag);
+          CFX_WideString wsTag =
+              static_cast<CFDE_XMLElement*>(m_pChild)->GetLocalTagName();
           if (wsTag == L"template") {
             m_dwCheckStatus |= 0x01;
             m_dwCurrentCheckStatus = 0x01;