Add ToXML{Instruction,Text,CharData}() checked conversion functions

All usages were previously checked correctly, but this consolidates
some code as well.

Change-Id: I63711748b31b698a3f21f98fdb536db1e9e0b1cf
Reviewed-on: https://pdfium-review.googlesource.com/39010
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

13 files changed, 86 insertions, 76 deletions

diff --git a/core/fxcrt/xml/cfx_xmlchardata.h b/core/fxcrt/xml/cfx_xmlchardata.h
index 013414bc82..9e5669a464 100644
--- a/core/fxcrt/xml/cfx_xmlchardata.h
+++ b/core/fxcrt/xml/cfx_xmlchardata.h
@@ -25,4 +25,10 @@ class CFX_XMLCharData : public CFX_XMLText {
   void Save(const RetainPtr<IFX_SeekableWriteStream>& pXMLStream) override;
 };
 
+inline CFX_XMLCharData* ToXMLCharData(CFX_XMLNode* pNode) {
+  return pNode && pNode->GetType() == FX_XMLNODE_CharData
+             ? static_cast<CFX_XMLCharData*>(pNode)
+             : nullptr;
+}
+
 #endif  // CORE_FXCRT_XML_CFX_XMLCHARDATA_H_
diff --git a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp
index 4f0c49eaac..41906edbfd 100644
--- a/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmlchardata_unittest.cpp
@@ -26,7 +26,7 @@ TEST(CFX_XMLCharDataTest, Clone) {
   EXPECT_TRUE(clone != nullptr);
   EXPECT_NE(&data, clone);
   ASSERT_EQ(FX_XMLNODE_CharData, clone->GetType());
-  EXPECT_EQ(L"My Data", static_cast<CFX_XMLCharData*>(clone)->GetText());
+  EXPECT_EQ(L"My Data", ToXMLCharData(clone)->GetText());
 }
 
 TEST(CFX_XMLCharDataTest, Save) {
diff --git a/core/fxcrt/xml/cfx_xmlelement.cpp b/core/fxcrt/xml/cfx_xmlelement.cpp
index 4bb4eae1bd..bd24091205 100644
--- a/core/fxcrt/xml/cfx_xmlelement.cpp
+++ b/core/fxcrt/xml/cfx_xmlelement.cpp
@@ -76,13 +76,11 @@ WideString CFX_XMLElement::GetNamespaceURI() const {
 
 WideString CFX_XMLElement::GetTextData() const {
   CFX_WideTextBuf buffer;
-
   for (CFX_XMLNode* pChild = GetFirstChild(); pChild;
        pChild = pChild->GetNextSibling()) {
-    if (pChild->GetType() == FX_XMLNODE_Text ||
-        pChild->GetType() == FX_XMLNODE_CharData) {
-      buffer << static_cast<CFX_XMLText*>(pChild)->GetText();
-    }
+    CFX_XMLText* pText = ToXMLText(pChild);
+    if (pText)
+      buffer << pText->GetText();
   }
   return buffer.MakeString();
 }
diff --git a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp
index dfc60a10c3..a1c63589ec 100644
--- a/core/fxcrt/xml/cfx_xmlelement_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmlelement_unittest.cpp
@@ -104,7 +104,7 @@ TEST(CFX_XMLElementTest, Clone) {
   EXPECT_TRUE(inst->GetFirstChild()->GetNextSibling() == nullptr);
 
   ASSERT_EQ(FX_XMLNODE_Text, inst->GetFirstChild()->GetType());
-  auto* text = static_cast<CFX_XMLText*>(inst->GetFirstChild());
+  auto* text = ToXMLText(inst->GetFirstChild());
   EXPECT_EQ(L"Text Child", text->GetText());
 }
 
diff --git a/core/fxcrt/xml/cfx_xmlinstruction.h b/core/fxcrt/xml/cfx_xmlinstruction.h
index 153ef2b625..8962d73ac1 100644
--- a/core/fxcrt/xml/cfx_xmlinstruction.h
+++ b/core/fxcrt/xml/cfx_xmlinstruction.h
@@ -36,4 +36,10 @@ class CFX_XMLInstruction : public CFX_XMLNode {
   std::vector<WideString> m_TargetData;
 };
 
+inline CFX_XMLInstruction* ToXMLInstruction(CFX_XMLNode* pNode) {
+  return pNode && pNode->GetType() == FX_XMLNODE_Instruction
+             ? static_cast<CFX_XMLInstruction*>(pNode)
+             : nullptr;
+}
+
 #endif  // CORE_FXCRT_XML_CFX_XMLINSTRUCTION_H_
diff --git a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp
index b25a18be9c..0109ab2985 100644
--- a/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmlinstruction_unittest.cpp
@@ -53,8 +53,7 @@ TEST(CFX_XMLInstructionTest, Clone) {
   EXPECT_TRUE(clone != nullptr);
 
   ASSERT_EQ(FX_XMLNODE_Instruction, clone->GetType());
-  CFX_XMLInstruction* inst = static_cast<CFX_XMLInstruction*>(clone);
-
+  CFX_XMLInstruction* inst = ToXMLInstruction(clone);
   EXPECT_TRUE(inst->IsAcrobat());
 
   auto& data = inst->GetTargetData();
@@ -98,8 +97,7 @@ TEST(CFX_XMLInstructionTest, ParseAndReSave) {
   ASSERT_TRUE(root->GetFirstChild() != nullptr);
   ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType());
 
-  CFX_XMLInstruction* node =
-      static_cast<CFX_XMLInstruction*>(root->GetFirstChild());
+  CFX_XMLInstruction* node = ToXMLInstruction(root->GetFirstChild());
   ASSERT_TRUE(node != nullptr);
   EXPECT_TRUE(node->IsAcrobat());
 
@@ -136,12 +134,9 @@ TEST(CFX_XMLInstructionTest, ParseAndReSaveInnerInstruction) {
   EXPECT_EQ(L"node", node->GetName());
 
   CFX_XMLInstruction* instruction = nullptr;
-  for (auto* elem = node->GetFirstChild(); elem;
+  for (auto* elem = node->GetFirstChild(); elem && !instruction;
        elem = elem->GetNextSibling()) {
-    if (elem->GetType() == FX_XMLNODE_Instruction) {
-      instruction = static_cast<CFX_XMLInstruction*>(elem);
-      break;
-    }
+    instruction = ToXMLInstruction(elem);
   }
   ASSERT_TRUE(instruction != nullptr);
   EXPECT_TRUE(instruction->IsAcrobat());
diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp
index dd28cf8adc..094daac889 100644
--- a/core/fxcrt/xml/cfx_xmlparser.cpp
+++ b/core/fxcrt/xml/cfx_xmlparser.cpp
@@ -518,12 +518,10 @@ void CFX_XMLParser::ProcessTargetData() {
   WideString target_data = GetTextData();
   if (target_data.IsEmpty())
     return;
-  if (!current_node_)
-    return;
-  if (current_node_->GetType() != FX_XMLNODE_Instruction)
-    return;
 
-  static_cast<CFX_XMLInstruction*>(current_node_)->AppendData(target_data);
+  CFX_XMLInstruction* instruction = ToXMLInstruction(current_node_);
+  if (instruction)
+    instruction->AppendData(target_data);
 }
 
 WideString CFX_XMLParser::GetTextData() {
diff --git a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
index 300131db95..c1236ce182 100644
--- a/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmlparser_unittest.cpp
@@ -325,8 +325,7 @@ TEST_F(CFX_XMLParserTest, ParseInstruction) {
   ASSERT_TRUE(root->GetFirstChild() != nullptr);
   ASSERT_EQ(FX_XMLNODE_Instruction, root->GetFirstChild()->GetType());
 
-  CFX_XMLInstruction* instruction =
-      static_cast<CFX_XMLInstruction*>(root->GetFirstChild());
+  CFX_XMLInstruction* instruction = ToXMLInstruction(root->GetFirstChild());
   EXPECT_TRUE(instruction->IsOriginalXFAVersion());
 }
 
diff --git a/core/fxcrt/xml/cfx_xmltext.h b/core/fxcrt/xml/cfx_xmltext.h
index c5c168059d..f7feae669e 100644
--- a/core/fxcrt/xml/cfx_xmltext.h
+++ b/core/fxcrt/xml/cfx_xmltext.h
@@ -31,4 +31,13 @@ class CFX_XMLText : public CFX_XMLNode {
   WideString m_wsText;
 };
 
+inline bool IsXMLText(CFX_XMLNode* pNode) {
+  FX_XMLNODETYPE type = pNode->GetType();
+  return type == FX_XMLNODE_Text || type == FX_XMLNODE_CharData;
+}
+
+inline CFX_XMLText* ToXMLText(CFX_XMLNode* pNode) {
+  return pNode && IsXMLText(pNode) ? static_cast<CFX_XMLText*>(pNode) : nullptr;
+}
+
 #endif  // CORE_FXCRT_XML_CFX_XMLTEXT_H_
diff --git a/core/fxcrt/xml/cfx_xmltext_unittest.cpp b/core/fxcrt/xml/cfx_xmltext_unittest.cpp
index 6e53f66612..04b1ee8571 100644
--- a/core/fxcrt/xml/cfx_xmltext_unittest.cpp
+++ b/core/fxcrt/xml/cfx_xmltext_unittest.cpp
@@ -25,7 +25,7 @@ TEST(CFX_XMLTextTest, Clone) {
   CFX_XMLNode* clone = data.Clone(&doc);
   EXPECT_TRUE(clone != nullptr);
   ASSERT_EQ(FX_XMLNODE_Text, clone->GetType());
-  EXPECT_EQ(L"My Data", static_cast<CFX_XMLText*>(clone)->GetText());
+  EXPECT_EQ(L"My Data", ToXMLText(clone)->GetText());
 }
 
 TEST(CFX_XMLTextTest, Save) {
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp
index 8bf955bd32..4463a34097 100644
--- a/xfa/fxfa/cxfa_textlayout.cpp
+++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -756,7 +756,7 @@ bool CXFA_TextLayout::LoadRichText(
             bContentNode ? pParentStyle.Get() : pStyle.Get());
         WideString wsText;
         if (bContentNode && iTabCount == 0) {
-          wsText = static_cast<CFX_XMLText*>(pXMLNode)->GetText();
+          wsText = ToXMLText(pXMLNode)->GetText();
         } else if (wsName == L"br") {
           wsText = L'\n';
         } else if (wsName == L"li") {
diff --git a/xfa/fxfa/parser/cxfa_document_parser.cpp b/xfa/fxfa/parser/cxfa_document_parser.cpp
index 34955829e9..96c031079b 100644
--- a/xfa/fxfa/parser/cxfa_document_parser.cpp
+++ b/xfa/fxfa/parser/cxfa_document_parser.cpp
@@ -243,18 +243,16 @@ void ConvertXMLToPlainText(CFX_XMLElement* pRootXMLNode, WideString& wsOutput) {
        pXMLChild = pXMLChild->GetNextSibling()) {
     switch (pXMLChild->GetType()) {
       case FX_XMLNODE_Element: {
-        WideString wsTextData =
-            static_cast<CFX_XMLElement*>(pXMLChild)->GetTextData();
+        WideString wsTextData = ToXMLElement(pXMLChild)->GetTextData();
         wsTextData += L"\n";
         wsOutput += wsTextData;
         break;
       }
       case FX_XMLNODE_Text:
       case FX_XMLNODE_CharData: {
-        WideString wsText = static_cast<CFX_XMLText*>(pXMLChild)->GetText();
+        WideString wsText = ToXMLText(pXMLChild)->GetText();
         if (IsStringAllWhitespace(wsText))
           continue;
-
         wsOutput = std::move(wsText);
         break;
       }
@@ -291,7 +289,7 @@ WideString GetPlainTextFromRichText(CFX_XMLNode* pXMLNode) {
     }
     case FX_XMLNODE_Text:
     case FX_XMLNODE_CharData: {
-      WideString wsContent = static_cast<CFX_XMLText*>(pXMLNode)->GetText();
+      WideString wsContent = ToXMLText(pXMLNode)->GetText();
       wsPlainText += wsContent;
       break;
     }
@@ -837,8 +835,7 @@ CXFA_Node* CXFA_DocumentParser::NormalLoader(CXFA_Node* pXFANode,
         }
       } break;
       case FX_XMLNODE_Instruction:
-        ParseInstruction(pXFANode, static_cast<CFX_XMLInstruction*>(pXMLChild),
-                         ePacketID);
+        ParseInstruction(pXFANode, ToXMLInstruction(pXMLChild), ePacketID);
         break;
       default:
         break;
@@ -882,8 +879,9 @@ void CXFA_DocumentParser::ParseContentNode(CXFA_Node* pXFANode,
     } else {
       if (pElement)
         break;
-      if (eNodeType == FX_XMLNODE_Text || eNodeType == FX_XMLNODE_CharData)
-        wsValue = static_cast<CFX_XMLText*>(pXMLChild)->GetText();
+      CFX_XMLText* pText = ToXMLText(pXMLChild);
+      if (pText)
+        wsValue = pText->GetText();
     }
     break;
   }
@@ -1011,7 +1009,7 @@ void CXFA_DocumentParser::ParseDataGroup(CXFA_Node* pXFANode,
       }
       case FX_XMLNODE_CharData:
       case FX_XMLNODE_Text: {
-        CFX_XMLText* pXMLText = static_cast<CFX_XMLText*>(pXMLChild);
+        CFX_XMLText* pXMLText = ToXMLText(pXMLChild);
         WideString wsText = pXMLText->GetText();
         if (IsStringAllWhitespace(wsText))
           continue;
@@ -1047,57 +1045,58 @@ void CXFA_DocumentParser::ParseDataValue(CXFA_Node* pXFANode,
     if (eNodeType == FX_XMLNODE_Instruction)
       continue;
 
-    if (eNodeType == FX_XMLNODE_Text || eNodeType == FX_XMLNODE_CharData) {
-      WideString wsText = static_cast<CFX_XMLText*>(pXMLChild)->GetText();
+    CFX_XMLText* pText = ToXMLText(pXMLChild);
+    if (pText) {
+      WideString wsText = pText->GetText();
       if (!pXMLCurValueNode)
         pXMLCurValueNode = pXMLChild;
-
       wsCurValueTextBuf << wsText;
-    } else if (XFA_RecognizeRichText(ToXMLElement(pXMLChild))) {
+      continue;
+    }
+    if (XFA_RecognizeRichText(ToXMLElement(pXMLChild))) {
       WideString wsText = GetPlainTextFromRichText(ToXMLElement(pXMLChild));
       if (!pXMLCurValueNode)
         pXMLCurValueNode = pXMLChild;
-
       wsCurValueTextBuf << wsText;
-    } else {
-      bMarkAsCompound = true;
-      if (pXMLCurValueNode) {
-        WideString wsCurValue = wsCurValueTextBuf.MakeString();
-        if (!wsCurValue.IsEmpty()) {
-          CXFA_Node* pXFAChild =
-              m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue);
-          if (!pXFAChild)
-            return;
+      continue;
+    }
+    bMarkAsCompound = true;
+    if (pXMLCurValueNode) {
+      WideString wsCurValue = wsCurValueTextBuf.MakeString();
+      if (!wsCurValue.IsEmpty()) {
+        CXFA_Node* pXFAChild =
+            m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue);
+        if (!pXFAChild)
+          return;
 
-          pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, L"", false,
-                                          false);
-          pXFAChild->JSObject()->SetCData(XFA_Attribute::Value, wsCurValue,
-                                          false, false);
-          pXFANode->InsertChild(pXFAChild, nullptr);
-          pXFAChild->SetXMLMappingNode(pXMLCurValueNode);
-          pXFAChild->SetFlag(XFA_NodeFlag_Initialized);
-          wsValueTextBuf << wsCurValue;
-          wsCurValueTextBuf.Clear();
-        }
-        pXMLCurValueNode = nullptr;
+        pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, L"", false, false);
+        pXFAChild->JSObject()->SetCData(XFA_Attribute::Value, wsCurValue, false,
+                                        false);
+        pXFANode->InsertChild(pXFAChild, nullptr);
+        pXFAChild->SetXMLMappingNode(pXMLCurValueNode);
+        pXFAChild->SetFlag(XFA_NodeFlag_Initialized);
+        wsValueTextBuf << wsCurValue;
+        wsCurValueTextBuf.Clear();
       }
-      CXFA_Node* pXFAChild =
-          m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue);
-      if (!pXFAChild)
-        return;
-
-      WideString wsNodeStr = ToXMLElement(pXMLChild)->GetLocalTagName();
-      pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, wsNodeStr, false,
-                                      false);
-      ParseDataValue(pXFAChild, pXMLChild, ePacketID);
-      pXFANode->InsertChild(pXFAChild, nullptr);
-      pXFAChild->SetXMLMappingNode(pXMLChild);
-      pXFAChild->SetFlag(XFA_NodeFlag_Initialized);
-      WideString wsCurValue =
-          pXFAChild->JSObject()->GetCData(XFA_Attribute::Value);
-      wsValueTextBuf << wsCurValue;
+      pXMLCurValueNode = nullptr;
     }
+    CXFA_Node* pXFAChild =
+        m_pFactory->CreateNode(ePacketID, XFA_Element::DataValue);
+    if (!pXFAChild)
+      return;
+
+    WideString wsNodeStr = ToXMLElement(pXMLChild)->GetLocalTagName();
+    pXFAChild->JSObject()->SetCData(XFA_Attribute::Name, wsNodeStr, false,
+                                    false);
+    ParseDataValue(pXFAChild, pXMLChild, ePacketID);
+    pXFANode->InsertChild(pXFAChild, nullptr);
+    pXFAChild->SetXMLMappingNode(pXMLChild);
+    pXFAChild->SetFlag(XFA_NodeFlag_Initialized);
+    WideString wsCurValue =
+        pXFAChild->JSObject()->GetCData(XFA_Attribute::Value);
+    wsValueTextBuf << wsCurValue;
   }
+
   if (pXMLCurValueNode) {
     WideString wsCurValue = wsCurValueTextBuf.MakeString();
     if (!wsCurValue.IsEmpty()) {
diff --git a/xfa/fxfa/parser/cxfa_node.cpp b/xfa/fxfa/parser/cxfa_node.cpp
index 9fdea14cae..3fe4f915bc 100644
--- a/xfa/fxfa/parser/cxfa_node.cpp
+++ b/xfa/fxfa/parser/cxfa_node.cpp
@@ -4730,7 +4730,7 @@ void CXFA_Node::SetToXML(const WideString& value) {
       break;
     }
     case FX_XMLNODE_Text:
-      static_cast<CFX_XMLText*>(GetXMLMappingNode())->SetText(value);
+      ToXMLText(GetXMLMappingNode())->SetText(value);
       break;
     default:
       NOTREACHED();