Tidy fpdfsave.cpp

Remove CFX_PtrArray filelist.
Promote ScopedFileStream to .h file and use it.
Fix _CAPS names, bool returns, and put in anonymous namespace.
FX_CreateMemoryStream() can't return null, so remove checks.

R=thestig@chromium.org

Review URL: https://codereview.chromium.org/1672153002 .

6 files changed, 103 insertions, 133 deletions

diff --git a/core/include/fpdfapi/fpdf_parser.h b/core/include/fpdfapi/fpdf_parser.h
index 6620a71e2e..18a942e4e8 100644
--- a/core/include/fpdfapi/fpdf_parser.h
+++ b/core/include/fpdfapi/fpdf_parser.h
@@ -12,7 +12,7 @@
 #include <set>
 
 #include "core/include/fpdfapi/fpdf_objects.h"
-#include "core/include/fxcrt/fx_system.h"
+#include "core/include/fxcrt/fx_basic.h"
 
 class CFX_Font;
 class CFX_Matrix;
@@ -28,7 +28,6 @@ class CPDF_Object;
 class CPDF_Parser;
 class CPDF_Pattern;
 class CPDF_StandardSecurityHandler;
-class IFX_FileRead;
 class IPDF_SecurityHandler;
 
 #define FPDFPERM_PRINT 0x0004
@@ -41,6 +40,11 @@ class IPDF_SecurityHandler;
 #define FPDFPERM_PRINT_HIGH 0x0800
 #define FPDF_PAGE_MAX_NUM 0xFFFFF
 
+// TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal.
+// Come up or wait for something better.
+using ScopedFileStream =
+    std::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>>;
+
 // Use the accessors below instead of directly accessing PDF_CharType.
 extern const char PDF_CharType[256];
 
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index 6489b1ed98..f170b20d98 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -81,11 +81,6 @@ bool CanReadFromBitStream(const CFX_BitStream* hStream,
 
 }  // namespace
 
-// TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal.
-// Come up or wait for something better.
-using ScopedFileStream =
-    std::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>>;
-
 bool IsSignatureDict(const CPDF_Dictionary* pDict) {
   CPDF_Object* pType = pDict->GetElementValue("Type");
   if (!pType)
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
index 77780b9dcf..e71b19011e 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
@@ -68,11 +68,6 @@ class CPDF_TestParser : public CPDF_Parser {
   FRIEND_TEST(fpdf_parser_parser, LoadCrossRefV4);
 };
 
-// TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal.
-// Come up or wait for something better.
-using ScopedFileStream =
-    std::unique_ptr<IFX_FileStream, ReleaseDeleter<IFX_FileStream>>;
-
 TEST(fpdf_parser_parser, ReadHexString) {
   {
     // Empty string.
diff --git a/fpdfsdk/src/fpdfsave.cpp b/fpdfsdk/src/fpdfsave.cpp
index b8e9c6e25c..ca0fa5364d 100644
--- a/fpdfsdk/src/fpdfsave.cpp
+++ b/fpdfsdk/src/fpdfsave.cpp
@@ -6,6 +6,7 @@
 
 #include "public/fpdf_save.h"
 
+#include "core/include/fpdfapi/fpdf_parser.h"
 #include "core/include/fpdfapi/fpdf_serial.h"
 #include "fpdfsdk/include/fsdk_define.h"
 #include "public/fpdf_edit.h"
@@ -60,41 +61,50 @@ void CFX_IFileWrite::Release() {
   delete this;
 }
 
+namespace {
+
 #ifdef PDF_ENABLE_XFA
-FX_BOOL _SaveXFADocumentData(CPDFXFA_Document* pDocument,
-                             CFX_PtrArray& fileList) {
+bool SaveXFADocumentData(CPDFXFA_Document* pDocument,
+                         std::vector<ScopedFileStream>* fileList) {
   if (!pDocument)
-    return FALSE;
+    return false;
+
   if (pDocument->GetDocType() != DOCTYPE_DYNAMIC_XFA &&
       pDocument->GetDocType() != DOCTYPE_STATIC_XFA)
-    return TRUE;
+    return true;
+
   if (!CPDFXFA_App::GetInstance()->GetXFAApp())
-    return TRUE;
+    return true;
 
   IXFA_DocView* pXFADocView = pDocument->GetXFADocView();
-  if (NULL == pXFADocView)
-    return TRUE;
+  if (!pXFADocView)
+    return true;
 
   IXFA_DocHandler* pXFADocHandler =
       CPDFXFA_App::GetInstance()->GetXFAApp()->GetDocHandler();
   CPDF_Document* pPDFDocument = pDocument->GetPDFDoc();
-  if (pDocument == NULL)
-    return FALSE;
+  if (!pDocument)
+    return false;
 
   CPDF_Dictionary* pRoot = pPDFDocument->GetRoot();
-  if (pRoot == NULL)
-    return FALSE;
+  if (!pRoot)
+    return false;
+
   CPDF_Dictionary* pAcroForm = pRoot->GetDictBy("AcroForm");
-  if (NULL == pAcroForm)
-    return FALSE;
+  if (!pAcroForm)
+    return false;
+
   CPDF_Object* pXFA = pAcroForm->GetElement("XFA");
-  if (pXFA == NULL)
-    return TRUE;
+  if (!pXFA)
+    return true;
+
   if (!pXFA->IsArray())
-    return FALSE;
+    return false;
+
   CPDF_Array* pArray = pXFA->GetArray();
-  if (NULL == pArray)
-    return FALSE;
+  if (!pArray)
+    return false;
+
   int size = pArray->GetCount();
   int iFormIndex = -1;
   int iDataSetsIndex = -1;
@@ -111,10 +121,8 @@ FX_BOOL _SaveXFADocumentData(CPDFXFA_Document* pDocument,
     else if (pPDFObj->GetString() == "template")
       iTemplate = i + 1;
   }
-  IXFA_ChecksumContext* pContext = NULL;
-  // Checksum
-  pContext = XFA_Checksum_Create();
-  FXSYS_assert(pContext);
+  std::unique_ptr<IXFA_ChecksumContext, ReleaseDeleter<IXFA_ChecksumContext>>
+      pContext(XFA_Checksum_Create());
   pContext->StartChecksum();
 
   // template
@@ -124,9 +132,8 @@ FX_BOOL _SaveXFADocumentData(CPDFXFA_Document* pDocument,
     streamAcc.LoadAllData(pTemplateStream);
     uint8_t* pData = (uint8_t*)streamAcc.GetData();
     FX_DWORD dwSize2 = streamAcc.GetSize();
-    IFX_FileStream* pTemplate = FX_CreateMemoryStream(pData, dwSize2);
-    pContext->UpdateChecksum((IFX_FileRead*)pTemplate);
-    pTemplate->Release();
+    ScopedFileStream pTemplate(FX_CreateMemoryStream(pData, dwSize2));
+    pContext->UpdateChecksum(pTemplate.get());
   }
   CPDF_Stream* pFormStream = NULL;
   CPDF_Stream* pDataSetsStream = NULL;
@@ -156,81 +163,69 @@ FX_BOOL _SaveXFADocumentData(CPDFXFA_Document* pDocument,
       pDataSetsStream = (CPDF_Stream*)pDataSetsPDFObj;
     }
   }
-  // end
   // L"datasets"
   {
-    IFX_FileStream* pDsfileWrite = FX_CreateMemoryStream();
-    if (NULL == pDsfileWrite) {
-      pContext->Release();
-      pDsfileWrite->Release();
-      return FALSE;
-    }
+    ScopedFileStream pDsfileWrite(FX_CreateMemoryStream());
     if (pXFADocHandler->SavePackage(pXFADocView->GetDoc(),
                                     CFX_WideStringC(L"datasets"),
-                                    pDsfileWrite) &&
+                                    pDsfileWrite.get()) &&
         pDsfileWrite->GetSize() > 0) {
       // Datasets
-      pContext->UpdateChecksum((IFX_FileRead*)pDsfileWrite);
+      pContext->UpdateChecksum(pDsfileWrite.get());
       pContext->FinishChecksum();
       CPDF_Dictionary* pDataDict = new CPDF_Dictionary;
       if (iDataSetsIndex != -1) {
         if (pDataSetsStream)
-          pDataSetsStream->InitStreamFromFile(pDsfileWrite, pDataDict);
+          pDataSetsStream->InitStreamFromFile(pDsfileWrite.get(), pDataDict);
       } else {
         CPDF_Stream* pData = new CPDF_Stream(NULL, 0, NULL);
-        pData->InitStreamFromFile(pDsfileWrite, pDataDict);
+        pData->InitStreamFromFile(pDsfileWrite.get(), pDataDict);
         pPDFDocument->AddIndirectObject(pData);
         iLast = pArray->GetCount() - 2;
         pArray->InsertAt(iLast, new CPDF_String("datasets", FALSE));
         pArray->InsertAt(iLast + 1, pData, pPDFDocument);
       }
-      fileList.Add(pDsfileWrite);
+      fileList->push_back(std::move(pDsfileWrite));
     }
   }
-
   // L"form"
   {
-    IFX_FileStream* pfileWrite = FX_CreateMemoryStream();
-    if (NULL == pfileWrite) {
-      pContext->Release();
-      return FALSE;
-    }
+    ScopedFileStream pfileWrite(FX_CreateMemoryStream());
     if (pXFADocHandler->SavePackage(pXFADocView->GetDoc(),
-                                    CFX_WideStringC(L"form"), pfileWrite,
-                                    pContext) &&
-        pfileWrite > 0) {
+                                    CFX_WideStringC(L"form"), pfileWrite.get(),
+                                    pContext.get()) &&
+        pfileWrite->GetSize() > 0) {
       CPDF_Dictionary* pDataDict = new CPDF_Dictionary;
       if (iFormIndex != -1) {
         if (pFormStream)
-          pFormStream->InitStreamFromFile(pfileWrite, pDataDict);
+          pFormStream->InitStreamFromFile(pfileWrite.get(), pDataDict);
       } else {
         CPDF_Stream* pData = new CPDF_Stream(NULL, 0, NULL);
-        pData->InitStreamFromFile(pfileWrite, pDataDict);
+        pData->InitStreamFromFile(pfileWrite.get(), pDataDict);
         pPDFDocument->AddIndirectObject(pData);
         iLast = pArray->GetCount() - 2;
         pArray->InsertAt(iLast, new CPDF_String("form", FALSE));
         pArray->InsertAt(iLast + 1, pData, pPDFDocument);
       }
-      fileList.Add(pfileWrite);
+      fileList->push_back(std::move(pfileWrite));
     }
   }
-  pContext->Release();
-  return TRUE;
+  return true;
 }
 
-FX_BOOL _SendPostSaveToXFADoc(CPDFXFA_Document* pDocument) {
+bool SendPostSaveToXFADoc(CPDFXFA_Document* pDocument) {
   if (!pDocument)
-    return FALSE;
+    return false;
 
   if (pDocument->GetDocType() != DOCTYPE_DYNAMIC_XFA &&
       pDocument->GetDocType() != DOCTYPE_STATIC_XFA)
-    return TRUE;
+    return true;
 
   IXFA_DocView* pXFADocView = pDocument->GetXFADocView();
-  if (NULL == pXFADocView)
-    return FALSE;
-  IXFA_WidgetHandler* pWidgetHander = pXFADocView->GetWidgetHandler();
+  if (!pXFADocView)
+    return false;
 
+  IXFA_WidgetHandler* pWidgetHander = pXFADocView->GetWidgetHandler();
   CXFA_WidgetAcc* pWidgetAcc = NULL;
   IXFA_WidgetAccIterator* pWidgetAccIterator =
       pXFADocView->CreateWidgetAccIterator();
@@ -244,17 +239,19 @@ FX_BOOL _SendPostSaveToXFADoc(CPDFXFA_Document* pDocument) {
   pWidgetAccIterator->Release();
   pXFADocView->UpdateDocView();
   pDocument->_ClearChangeMark();
-  return TRUE;
+  return true;
 }
 
-FX_BOOL _SendPreSaveToXFADoc(CPDFXFA_Document* pDocument,
-                             CFX_PtrArray& fileList) {
+bool SendPreSaveToXFADoc(CPDFXFA_Document* pDocument,
+                         std::vector<ScopedFileStream>* fileList) {
   if (pDocument->GetDocType() != DOCTYPE_DYNAMIC_XFA &&
       pDocument->GetDocType() != DOCTYPE_STATIC_XFA)
-    return TRUE;
+    return true;
+
   IXFA_DocView* pXFADocView = pDocument->GetXFADocView();
-  if (NULL == pXFADocView)
-    return TRUE;
+  if (!pXFADocView)
+    return true;
+
   IXFA_WidgetHandler* pWidgetHander = pXFADocView->GetWidgetHandler();
   CXFA_WidgetAcc* pWidgetAcc = NULL;
   IXFA_WidgetAccIterator* pWidgetAccIterator =
@@ -268,28 +265,27 @@ FX_BOOL _SendPreSaveToXFADoc(CPDFXFA_Document* pDocument,
   }
   pWidgetAccIterator->Release();
   pXFADocView->UpdateDocView();
-  return _SaveXFADocumentData(pDocument, fileList);
+  return SaveXFADocumentData(pDocument, fileList);
 }
 #endif  // PDF_ENABLE_XFA
 
-FPDF_BOOL _FPDF_Doc_Save(FPDF_DOCUMENT document,
-                         FPDF_FILEWRITE* pFileWrite,
-                         FPDF_DWORD flags,
-                         FPDF_BOOL bSetVersion,
-                         int fileVerion) {
+bool FPDF_Doc_Save(FPDF_DOCUMENT document,
+                   FPDF_FILEWRITE* pFileWrite,
+                   FPDF_DWORD flags,
+                   FPDF_BOOL bSetVersion,
+                   int fileVerion) {
   CPDF_Document* pPDFDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pPDFDoc)
     return 0;
 
 #ifdef PDF_ENABLE_XFA
-  CPDFXFA_Document* pDoc = (CPDFXFA_Document*)document;
-  CFX_PtrArray fileList;
-  _SendPreSaveToXFADoc(pDoc, fileList);
+  CPDFXFA_Document* pDoc = static_cast<CPDFXFA_Document*>(document);
+  std::vector<ScopedFileStream> fileList;
+  SendPreSaveToXFADoc(pDoc, &fileList);
 #endif  // PDF_ENABLE_XFA
 
-  if (flags < FPDF_INCREMENTAL || flags > FPDF_REMOVE_SECURITY) {
+  if (flags < FPDF_INCREMENTAL || flags > FPDF_REMOVE_SECURITY)
     flags = 0;
-  }
 
   CPDF_Creator FileMaker(pPDFDoc);
   if (bSetVersion)
@@ -305,26 +301,23 @@ FPDF_BOOL _FPDF_Doc_Save(FPDF_DOCUMENT document,
   pStreamWrite->Init(pFileWrite);
   bRet = FileMaker.Create(pStreamWrite, flags);
 #ifdef PDF_ENABLE_XFA
-  _SendPostSaveToXFADoc(pDoc);
-  for (int i = 0; i < fileList.GetSize(); i++) {
-    IFX_FileStream* pFile = (IFX_FileStream*)fileList.GetAt(i);
-    pFile->Release();
-  }
-  fileList.RemoveAll();
+  SendPostSaveToXFADoc(pDoc);
 #endif  // PDF_ENABLE_XFA
   pStreamWrite->Release();
   return bRet;
 }
 
+}  // namespace
+
 DLLEXPORT FPDF_BOOL STDCALL FPDF_SaveAsCopy(FPDF_DOCUMENT document,
                                             FPDF_FILEWRITE* pFileWrite,
                                             FPDF_DWORD flags) {
-  return _FPDF_Doc_Save(document, pFileWrite, flags, FALSE, 0);
+  return FPDF_Doc_Save(document, pFileWrite, flags, FALSE, 0);
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FPDF_SaveWithVersion(FPDF_DOCUMENT document,
                                                  FPDF_FILEWRITE* pFileWrite,
                                                  FPDF_DWORD flags,
                                                  int fileVersion) {
-  return _FPDF_Doc_Save(document, pFileWrite, flags, TRUE, fileVersion);
+  return FPDF_Doc_Save(document, pFileWrite, flags, TRUE, fileVersion);
 }
diff --git a/xfa/src/fxfa/src/app/xfa_ffdoc.cpp b/xfa/src/fxfa/src/app/xfa_ffdoc.cpp
index 87745616e0..513098c7a9 100644
--- a/xfa/src/fxfa/src/app/xfa_ffdoc.cpp
+++ b/xfa/src/fxfa/src/app/xfa_ffdoc.cpp
@@ -113,13 +113,6 @@ int32_t CXFA_FFDoc::DoLoad(IFX_Pause* pPause) {
     IFX_FileRead* pXFAReader = NULL;
     if (XFA_GetPDFContentsFromPDFXML(pPDFXML, pByteBuffer, iBufferSize)) {
       pXFAReader = FX_CreateMemoryStream(pByteBuffer, iBufferSize, TRUE);
-      if (!pXFAReader) {
-        if (pByteBuffer) {
-          FX_Free(pByteBuffer);
-          pByteBuffer = NULL;
-        }
-        return XFA_PARSESTATUS_SyntaxErr;
-      }
     } else {
       CFX_WideString wsHref;
       ((IFDE_XMLElement*)pPDFXML)->GetString(L"href", wsHref);
diff --git a/xfa/src/fxfa/src/parser/xfa_object_imp.cpp b/xfa/src/fxfa/src/parser/xfa_object_imp.cpp
index 10d5e24abf..278ad85324 100644
--- a/xfa/src/fxfa/src/parser/xfa_object_imp.cpp
+++ b/xfa/src/fxfa/src/parser/xfa_object_imp.cpp
@@ -1056,6 +1056,7 @@ void CXFA_Node::Script_NodeClass_LoadXML(CFXJSE_Arguments* pArguments) {
 }
 void CXFA_Node::Script_NodeClass_SaveFilteredXML(CFXJSE_Arguments* pArguments) {
 }
+
 void CXFA_Node::Script_NodeClass_SaveXML(CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength < 0 || iLength > 1) {
@@ -1065,20 +1066,15 @@ void CXFA_Node::Script_NodeClass_SaveXML(CFXJSE_Arguments* pArguments) {
   FX_BOOL bPrettyMode = FALSE;
   if (iLength == 1) {
     CFX_ByteString bsPretty = pArguments->GetUTF8String(0);
-    if (bsPretty.Equal("pretty")) {
-      bPrettyMode = TRUE;
-    } else {
+    if (!bsPretty.Equal("pretty")) {
       ThrowScriptErrorMessage(XFA_IDS_ARGUMENT_MISMATCH);
       return;
     }
+    bPrettyMode = TRUE;
   }
   CFX_ByteStringC bsXMLHeader = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n";
   if (GetPacketID() == XFA_XDPPACKET_Form) {
     IFX_MemoryStream* pMemoryStream = FX_CreateMemoryStream(TRUE);
-    if (!pMemoryStream) {
-      FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
-      return;
-    }
     IFX_Stream* pStream = IFX_Stream::CreateStream(
         (IFX_FileWrite*)pMemoryStream,
         FX_STREAMACCESS_Text | FX_STREAMACCESS_Write | FX_STREAMACCESS_Append);
@@ -1101,7 +1097,8 @@ void CXFA_Node::Script_NodeClass_SaveXML(CFXJSE_Arguments* pArguments) {
       pMemoryStream = NULL;
     }
     return;
-  } else if (GetPacketID() == XFA_XDPPACKET_Datasets) {
+  }
+  if (GetPacketID() == XFA_XDPPACKET_Datasets) {
     IFDE_XMLNode* pElement = this->GetXMLMappingNode();
     if (!pElement || pElement->GetType() != FDE_XMLNODE_Element) {
       FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
@@ -1109,35 +1106,28 @@ void CXFA_Node::Script_NodeClass_SaveXML(CFXJSE_Arguments* pArguments) {
     }
     XFA_DataExporter_DealWithDataGroupNode(this);
     IFX_MemoryStream* pMemoryStream = FX_CreateMemoryStream(TRUE);
-    if (!pMemoryStream) {
-      FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), bsXMLHeader);
-      return;
+    IFX_Stream* pStream = IFX_Stream::CreateStream(
+        (IFX_FileWrite*)pMemoryStream,
+        FX_STREAMACCESS_Text | FX_STREAMACCESS_Write | FX_STREAMACCESS_Append);
+    if (pStream) {
+      pStream->SetCodePage(FX_CODEPAGE_UTF8);
+      pStream->WriteData(bsXMLHeader.GetPtr(), bsXMLHeader.GetLength());
+      pElement->SaveXMLNode(pStream);
+      FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(),
+                                CFX_ByteStringC(pMemoryStream->GetBuffer(),
+                                                pMemoryStream->GetSize()));
+      pStream->Release();
+      pStream = NULL;
     }
     if (pMemoryStream) {
-      IFX_Stream* pStream = IFX_Stream::CreateStream(
-          (IFX_FileWrite*)pMemoryStream, FX_STREAMACCESS_Text |
-                                             FX_STREAMACCESS_Write |
-                                             FX_STREAMACCESS_Append);
-      if (pStream) {
-        pStream->SetCodePage(FX_CODEPAGE_UTF8);
-        pStream->WriteData(bsXMLHeader.GetPtr(), bsXMLHeader.GetLength());
-        pElement->SaveXMLNode(pStream);
-        FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(),
-                                  CFX_ByteStringC(pMemoryStream->GetBuffer(),
-                                                  pMemoryStream->GetSize()));
-        pStream->Release();
-        pStream = NULL;
-      }
-      if (pMemoryStream) {
-        pMemoryStream->Release();
-        pMemoryStream = NULL;
-      }
-      return;
+      pMemoryStream->Release();
+      pMemoryStream = NULL;
     }
-  } else {
-    FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), "");
+    return;
   }
+  FXJSE_Value_SetUTF8String(pArguments->GetReturnValue(), "");
 }
+
 void CXFA_Node::Script_NodeClass_SetAttribute(CFXJSE_Arguments* pArguments) {
   int32_t iLength = pArguments->GetLength();
   if (iLength != 2) {