Cleanup ownership of parser members

xfa/fxfa/parser/cxfa_simple_parser.cpp

Index: xfa/fxfa/parser/cxfa_simple_parser.cpp
diff --git a/xfa/fxfa/parser/cxfa_simple_parser.cpp b/xfa/fxfa/parser/cxfa_simple_parser.cpp
index c9edf2daff8293d40884038329bee3f6436d3ae4..a983d960564a518d429a8bdd3176b1808cd95304 100644
--- a/xfa/fxfa/parser/cxfa_simple_parser.cpp
+++ b/xfa/fxfa/parser/cxfa_simple_parser.cpp
@@ -24,7 +24,6 @@ CXFA_SimpleParser::CXFA_SimpleParser(CXFA_Document* pFactory,
       m_bDocumentParser(bDocumentParser) {}
 
 CXFA_SimpleParser::~CXFA_SimpleParser() {
-  CloseParser();
 }
 
 void CXFA_SimpleParser::SetFactory(CXFA_Document* pFactory) {
@@ -61,45 +60,41 @@ int32_t CXFA_SimpleParser::StartParse(IFX_FileRead* pStream,
                                       XFA_XDPPACKET ePacketID) {
   CloseParser();
   m_pFileRead = pStream;
-  m_pStream = IFX_Stream::CreateStream(
-      pStream, FX_STREAMACCESS_Read | FX_STREAMACCESS_Text);
-  if (!m_pStream) {
+  m_pStream.reset(IFX_Stream::CreateStream(
+      pStream, FX_STREAMACCESS_Read | FX_STREAMACCESS_Text));
+  if (!m_pStream)
     return XFA_PARSESTATUS_StreamErr;
-  }
+
   uint16_t wCodePage = m_pStream->GetCodePage();
   if (wCodePage != FX_CODEPAGE_UTF16LE && wCodePage != FX_CODEPAGE_UTF16BE &&
       wCodePage != FX_CODEPAGE_UTF8) {
     m_pStream->SetCodePage(FX_CODEPAGE_UTF8);
   }
-  m_pXMLDoc = new CFDE_XMLDoc;
-  m_pXMLParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream);
-  if (!m_pXMLDoc->LoadXML(m_pXMLParser)) {
+  m_pXMLDoc.reset(new CFDE_XMLDoc);
+  m_pXMLParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream.get());
+  if (!m_pXMLDoc->LoadXML(m_pXMLParser))
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   m_ePacketID = ePacketID;
   return XFA_PARSESTATUS_Ready;
 }

[Wei Li, 2016/07/07 22:22:11]

Blank line here? Ditto.

[dsinclair, 2016/07/11 13:41:29]

Done.

 int32_t CXFA_SimpleParser::DoParse(IFX_Pause* pPause) {
-  if (!m_pXMLDoc || m_ePacketID == XFA_XDPPACKET_UNKNOWN) {
+  if (!m_pXMLDoc || m_ePacketID == XFA_XDPPACKET_UNKNOWN)
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   int32_t iRet = m_pXMLDoc->DoLoad(pPause);
-  if (iRet < 0) {
+  if (iRet < 0)
     return XFA_PARSESTATUS_SyntaxErr;
-  }
-  if (iRet < 100) {
+  if (iRet < 100)
     return iRet / 2;
-  }
-  m_pRootNode = ParseAsXDPPacket(XFA_FDEExtension_GetDocumentNode(m_pXMLDoc),
-                                 m_ePacketID);
+
+  m_pRootNode = ParseAsXDPPacket(
+      XFA_FDEExtension_GetDocumentNode(m_pXMLDoc.get()), m_ePacketID);
   m_pXMLDoc->CloseXML();
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
-  if (!m_pRootNode) {
+  m_pStream.reset();
+
+  if (!m_pRootNode)
     return XFA_PARSESTATUS_StatusErr;
-  }
   return XFA_PARSESTATUS_Done;
 }
 int32_t CXFA_SimpleParser::ParseXMLData(const CFX_WideString& wsXML,
@@ -108,31 +103,28 @@ int32_t CXFA_SimpleParser::ParseXMLData(const CFX_WideString& wsXML,
   CloseParser();
   pXMLNode = nullptr;
   IFX_Stream* pStream = XFA_CreateWideTextRead(wsXML);
-  if (!pStream) {
+  if (!pStream)
     return XFA_PARSESTATUS_StreamErr;
-  }
-  m_pStream = pStream;
-  m_pXMLDoc = new CFDE_XMLDoc;
-  CXFA_XMLParser* pParser = new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream);
+
+  m_pStream.reset(pStream);

[Wei Li, 2016/07/07 22:22:11]

Is |m_pStream| needed here? Having a unique_ptr of |pStream| may be simpler?

[dsinclair, 2016/07/11 13:41:29]

Done.

+  m_pXMLDoc.reset(new CFDE_XMLDoc);
+  CXFA_XMLParser* pParser =
+      new CXFA_XMLParser(m_pXMLDoc->GetRoot(), m_pStream.get());
   pParser->m_dwCheckStatus = 0x03;
-  if (!m_pXMLDoc->LoadXML(pParser)) {
+  if (!m_pXMLDoc->LoadXML(pParser))
     return XFA_PARSESTATUS_StatusErr;
-  }
+
   int32_t iRet = m_pXMLDoc->DoLoad(pPause);
-  if (iRet < 0 || iRet >= 100) {
+  if (iRet < 0 || iRet >= 100)
     m_pXMLDoc->CloseXML();
-  }
-  if (iRet < 0) {
+  if (iRet < 0)
     return XFA_PARSESTATUS_SyntaxErr;
-  }
-  if (iRet < 100) {
+  if (iRet < 100)
     return iRet / 2;
-  }
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
-  pXMLNode = XFA_FDEExtension_GetDocumentNode(m_pXMLDoc);
+
+  m_pStream.reset();
+
+  pXMLNode = XFA_FDEExtension_GetDocumentNode(m_pXMLDoc.get());
   return XFA_PARSESTATUS_Done;
 }
 
@@ -187,7 +179,7 @@ CXFA_Node* CXFA_SimpleParser::GetRootNode() const {
 }
 
 CFDE_XMLDoc* CXFA_SimpleParser::GetXMLDoc() const {
-  return m_pXMLDoc;
+  return m_pXMLDoc.get();
 }
 
 FX_BOOL XFA_FDEExtension_ResolveNamespaceQualifier(
@@ -1307,12 +1299,6 @@ void CXFA_SimpleParser::ParseInstruction(CXFA_Node* pXFANode,
   }
 }
 void CXFA_SimpleParser::CloseParser() {
-  if (m_pXMLDoc) {
-    m_pXMLDoc->Release();
-    m_pXMLDoc = nullptr;
-  }
-  if (m_pStream) {
-    m_pStream->Release();
-    m_pStream = nullptr;
-  }
+  m_pXMLDoc.reset();
+  m_pStream.reset();
 }