Flip document and parser ownership

This Cl switches the ownership between the parser and the document. Previously
the parser owned the document and we'd jump through hoops during cleanup to
delete the right object. This Cl flips the ownership so the document owns
the parser and simplifies the cleanup logic where needed.

BUG=pdfium:565

Review-Url: https://codereview.chromium.org/2275773003

6 files changed, 41 insertions, 57 deletions

diff --git a/fpdfsdk/fpdf_dataavail.cpp b/fpdfsdk/fpdf_dataavail.cpp
index 5dd42402ed..7b9ba32fb0 100644
--- a/fpdfsdk/fpdf_dataavail.cpp
+++ b/fpdfsdk/fpdf_dataavail.cpp
@@ -139,16 +139,17 @@ FPDFAvail_GetDocument(FPDF_AVAIL avail, FPDF_BYTESTRING password) {
   std::unique_ptr<CPDF_Parser> pParser(new CPDF_Parser);
   pParser->SetPassword(password);
 
-  std::unique_ptr<CPDF_Document> pDocument(new CPDF_Document(pParser.get()));
-  CPDF_Parser::Error error = pParser->StartLinearizedParse(
-      pDataAvail->m_pDataAvail->GetFileRead(), std::move(pDocument));
+  std::unique_ptr<CPDF_Document> pDocument(
+      new CPDF_Document(std::move(pParser)));
+  CPDF_Parser::Error error = pDocument->GetParser()->StartLinearizedParse(
+      pDataAvail->m_pDataAvail->GetFileRead(), pDocument.get());
   if (error != CPDF_Parser::SUCCESS) {
     ProcessParseError(error);
     return nullptr;
   }
-  pDataAvail->m_pDataAvail->SetDocument(pParser->GetDocument());
-  CheckUnSupportError(pParser->GetDocument(), FPDF_ERR_SUCCESS);
-  return FPDFDocumentFromCPDFDocument(pParser.release()->GetDocument());
+  pDataAvail->m_pDataAvail->SetDocument(pDocument.get());
+  CheckUnSupportError(pDocument.get(), FPDF_ERR_SUCCESS);
+  return FPDFDocumentFromCPDFDocument(pDocument.release());
 }
 
 DLLEXPORT int STDCALL FPDFAvail_GetFirstPageNum(FPDF_DOCUMENT doc) {
diff --git a/fpdfsdk/fpdfdoc_unittest.cpp b/fpdfsdk/fpdfdoc_unittest.cpp
index 896c7d0c72..a555e961f3 100644
--- a/fpdfsdk/fpdfdoc_unittest.cpp
+++ b/fpdfsdk/fpdfdoc_unittest.cpp
@@ -10,6 +10,7 @@
 #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_name.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_number.h"
+#include "core/fpdfapi/fpdf_parser/include/cpdf_parser.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_reference.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_string.h"
 #include "core/fpdfapi/include/cpdf_modulemgr.h"
@@ -23,7 +24,7 @@
 
 class CPDF_TestDocument : public CPDF_Document {
  public:
-  CPDF_TestDocument() : CPDF_Document(nullptr) {}
+  CPDF_TestDocument() : CPDF_Document(std::unique_ptr<CPDF_Parser>()) {}
 
   void SetRoot(CPDF_Dictionary* root) { m_pRootDict = root; }
   CPDF_IndirectObjectHolder* GetHolder() { return this; }
@@ -33,7 +34,8 @@ class CPDF_TestDocument : public CPDF_Document {
 class CPDF_TestXFADocument : public CPDFXFA_Document {
  public:
   CPDF_TestXFADocument()
-      : CPDFXFA_Document(new CPDF_TestDocument(), CPDFXFA_App::GetInstance()) {}
+      : CPDFXFA_Document(WrapUnique(new CPDF_TestDocument()),
+                         CPDFXFA_App::GetInstance()) {}
 
   void SetRoot(CPDF_Dictionary* root) {
     reinterpret_cast<CPDF_TestDocument*>(GetPDFDoc())->SetRoot(root);
diff --git a/fpdfsdk/fpdfview.cpp b/fpdfsdk/fpdfview.cpp
index 29fe88d943..0c3a95d094 100644
--- a/fpdfsdk/fpdfview.cpp
+++ b/fpdfsdk/fpdfview.cpp
@@ -66,8 +66,8 @@ CPDF_Document* CPDFDocumentFromFPDFDocument(FPDF_DOCUMENT doc) {
 
 FPDF_DOCUMENT FPDFDocumentFromCPDFDocument(CPDF_Document* doc) {
 #ifdef PDF_ENABLE_XFA
-  return doc ? FPDFDocumentFromUnderlying(
-                   new CPDFXFA_Document(doc, CPDFXFA_App::GetInstance()))
+  return doc ? FPDFDocumentFromUnderlying(new CPDFXFA_Document(
+                   WrapUnique(doc), CPDFXFA_App::GetInstance()))
              : nullptr;
 #else   // PDF_ENABLE_XFA
   return FPDFDocumentFromUnderlying(doc);
@@ -365,22 +365,17 @@ DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_LoadDocument(FPDF_STRING file_path,
   std::unique_ptr<CPDF_Parser> pParser(new CPDF_Parser);
   pParser->SetPassword(password);
 
-  std::unique_ptr<CPDF_Document> pDocument(new CPDF_Document(pParser.get()));
-  CPDF_Parser::Error error =
-      pParser->StartParse(pFileAccess, std::move(pDocument));
+  std::unique_ptr<CPDF_Document> pDocument(
+      new CPDF_Document(std::move(pParser)));
+  CPDF_Parser::Error error = pParser->StartParse(pFileAccess, pDocument.get());
   if (error != CPDF_Parser::SUCCESS) {
     ProcessParseError(error);
     return nullptr;
   }
 #ifdef PDF_ENABLE_XFA
-  CPDF_Document* pPDFDoc = pParser.release()->GetDocument();
-  if (!pPDFDoc)
-    return nullptr;
-
-  CPDFXFA_App* pProvider = CPDFXFA_App::GetInstance();
-  return new CPDFXFA_Document(pPDFDoc, pProvider);
+  return new CPDFXFA_Document(std::move(pDocument), CPDFXFA_App::GetInstance());
 #else   // PDF_ENABLE_XFA
-  return pParser.release()->GetDocument();
+  return pDocument.release();
 #endif  // PDF_ENABLE_XFA
 }
 
@@ -451,15 +446,16 @@ DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_LoadMemDocument(const void* data_buf,
   std::unique_ptr<CPDF_Parser> pParser(new CPDF_Parser);
   pParser->SetPassword(password);
 
-  std::unique_ptr<CPDF_Document> pDocument(new CPDF_Document(pParser.get()));
+  std::unique_ptr<CPDF_Document> pDocument(
+      new CPDF_Document(std::move(pParser)));
   CPDF_Parser::Error error =
-      pParser->StartParse(pMemFile, std::move(pDocument));
+      pDocument->GetParser()->StartParse(pMemFile, pDocument.get());
   if (error != CPDF_Parser::SUCCESS) {
     ProcessParseError(error);
     return nullptr;
   }
-  CheckUnSupportError(pParser->GetDocument(), error);
-  return FPDFDocumentFromCPDFDocument(pParser.release()->GetDocument());
+  CheckUnSupportError(pDocument.get(), error);
+  return FPDFDocumentFromCPDFDocument(pDocument.release());
 }
 
 DLLEXPORT FPDF_DOCUMENT STDCALL
@@ -469,14 +465,16 @@ FPDF_LoadCustomDocument(FPDF_FILEACCESS* pFileAccess,
   std::unique_ptr<CPDF_Parser> pParser(new CPDF_Parser);
   pParser->SetPassword(password);
 
-  std::unique_ptr<CPDF_Document> pDocument(new CPDF_Document(pParser.get()));
-  CPDF_Parser::Error error = pParser->StartParse(pFile, std::move(pDocument));
+  std::unique_ptr<CPDF_Document> pDocument(
+      new CPDF_Document(std::move(pParser)));
+  CPDF_Parser::Error error =
+      pDocument->GetParser()->StartParse(pFile, pDocument.get());
   if (error != CPDF_Parser::SUCCESS) {
     ProcessParseError(error);
     return nullptr;
   }
-  CheckUnSupportError(pParser->GetDocument(), error);
-  return FPDFDocumentFromCPDFDocument(pParser.release()->GetDocument());
+  CheckUnSupportError(pDocument.get(), error);
+  return FPDFDocumentFromCPDFDocument(pDocument.release());
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FPDF_GetFileVersion(FPDF_DOCUMENT doc,
@@ -683,19 +681,7 @@ DLLEXPORT void STDCALL FPDF_ClosePage(FPDF_PAGE page) {
 }
 
 DLLEXPORT void STDCALL FPDF_CloseDocument(FPDF_DOCUMENT document) {
-#ifdef PDF_ENABLE_XFA
   delete UnderlyingFromFPDFDocument(document);
-#else   // PDF_ENABLE_XFA
-  CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document);
-  if (!pDoc)
-    return;
-  CPDF_Parser* pParser = pDoc->GetParser();
-  if (!pParser) {
-    delete pDoc;
-    return;
-  }
-  delete pParser;
-#endif  // PDF_ENABLE_XFA
 }
 
 DLLEXPORT unsigned long STDCALL FPDF_GetLastError() {
diff --git a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
index 605e53c32a..d4c43c5706 100644
--- a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
+++ b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
@@ -45,10 +45,10 @@ extern void SetLastError(int err);
 extern int GetLastError();
 #endif
 
-CPDFXFA_Document::CPDFXFA_Document(CPDF_Document* pPDFDoc,
+CPDFXFA_Document::CPDFXFA_Document(std::unique_ptr<CPDF_Document> pPDFDoc,
                                    CPDFXFA_App* pProvider)
     : m_iDocType(DOCTYPE_PDF),
-      m_pPDFDoc(pPDFDoc),
+      m_pPDFDoc(std::move(pPDFDoc)),
       m_pXFADocView(nullptr),
       m_pApp(pProvider),
       m_pJSContext(nullptr),
@@ -70,16 +70,6 @@ CPDFXFA_Document::~CPDFXFA_Document() {
   }
   if (m_pJSContext && m_pSDKDoc && m_pSDKDoc->GetEnv())
     m_pSDKDoc->GetEnv()->GetJSRuntime()->ReleaseContext(m_pJSContext);
-  // |m_pSDKDoc| has to be released before |pParser| and |m_pPDFDoc| since it
-  // needs to access them to kill focused annotations.
-  m_pSDKDoc.reset();
-  if (m_pPDFDoc) {
-    CPDF_Parser* pParser = m_pPDFDoc->GetParser();
-    if (pParser)
-      delete pParser;
-    else
-      delete m_pPDFDoc;
-  }
 
   m_nLoadStatus = FXFA_LOADSTATUS_CLOSED;
 }
@@ -96,7 +86,7 @@ FX_BOOL CPDFXFA_Document::LoadXFADoc() {
   if (!pApp)
     return FALSE;
 
-  m_pXFADoc.reset(pApp->CreateDoc(this, m_pPDFDoc));
+  m_pXFADoc.reset(pApp->CreateDoc(this, m_pPDFDoc.get()));
   if (!m_pXFADoc) {
     SetLastError(FPDF_ERR_XFALOAD);
     return FALSE;
diff --git a/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h b/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h
index ed4c8727fc..c83c770f97 100644
--- a/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h
+++ b/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h
@@ -27,12 +27,13 @@ class CXFA_FFDocHandler;
 
 class CPDFXFA_Document : public IXFA_DocProvider {
  public:
-  CPDFXFA_Document(CPDF_Document* pPDFDoc, CPDFXFA_App* pProvider);
+  CPDFXFA_Document(std::unique_ptr<CPDF_Document> pPDFDoc,
+                   CPDFXFA_App* pProvider);
   ~CPDFXFA_Document() override;
 
   FX_BOOL LoadXFADoc();
   CPDFXFA_App* GetApp() { return m_pApp; }
-  CPDF_Document* GetPDFDoc() { return m_pPDFDoc; }
+  CPDF_Document* GetPDFDoc() { return m_pPDFDoc.get(); }
   CXFA_FFDoc* GetXFADoc() { return m_pXFADoc.get(); }
   CXFA_FFDocView* GetXFADocView() { return m_pXFADocView; }
 
@@ -200,7 +201,10 @@ class CPDFXFA_Document : public IXFA_DocProvider {
   }
 
   int m_iDocType;
-  CPDF_Document* m_pPDFDoc;
+
+  // |m_pSDKDoc| has to be released before |m_pPDFDoc| since it needs to access
+  // it to kill focused annotations.
+  std::unique_ptr<CPDF_Document> m_pPDFDoc;
   std::unique_ptr<CPDFSDK_Document> m_pSDKDoc;
   std::unique_ptr<CXFA_FFDoc> m_pXFADoc;
   CXFA_FFDocView* m_pXFADocView;  // not owned.
diff --git a/fpdfsdk/pdfwindow/PWL_FontMap.cpp b/fpdfsdk/pdfwindow/PWL_FontMap.cpp
index 96acb78ec2..131334ac22 100644
--- a/fpdfsdk/pdfwindow/PWL_FontMap.cpp
+++ b/fpdfsdk/pdfwindow/PWL_FontMap.cpp
@@ -9,6 +9,7 @@
 #include "core/fpdfapi/fpdf_font/include/cpdf_font.h"
 #include "core/fpdfapi/fpdf_font/include/cpdf_fontencoding.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h"
+#include "core/fpdfapi/fpdf_parser/include/cpdf_parser.h"
 #include "core/fpdfapi/include/cpdf_modulemgr.h"
 #include "core/fpdfdoc/include/ipvt_fontmap.h"
 #include "fpdfsdk/pdfwindow/PWL_Wnd.h"
@@ -50,7 +51,7 @@ void CPWL_FontMap::SetSystemHandler(CFX_SystemHandler* pSystemHandler) {
 CPDF_Document* CPWL_FontMap::GetDocument() {
   if (!m_pPDFDoc) {
     if (CPDF_ModuleMgr::Get()) {
-      m_pPDFDoc.reset(new CPDF_Document(nullptr));
+      m_pPDFDoc.reset(new CPDF_Document(std::unique_ptr<CPDF_Parser>()));
       m_pPDFDoc->CreateNewDoc();
     }
   }