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
diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
index b11d0c1..89b41d5 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
@@ -483,9 +483,9 @@
} // namespace
-CPDF_Document::CPDF_Document(CPDF_Parser* pParser)
+CPDF_Document::CPDF_Document(std::unique_ptr<CPDF_Parser> pParser)
: CPDF_IndirectObjectHolder(),
- m_pParser(pParser),
+ m_pParser(std::move(pParser)),
m_pRootDict(nullptr),
m_pInfoDict(nullptr),
m_bLinearized(false),
diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
index ffd3f79..e2bab45 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
@@ -126,7 +126,7 @@
void CPDF_Parser::CloseParser() {
m_bVersionUpdated = false;
- m_pDocument.reset();
+ m_pDocument = nullptr;
if (m_pTrailer) {
m_pTrailer->Release();
@@ -158,9 +158,8 @@
}
}
-CPDF_Parser::Error CPDF_Parser::StartParse(
- IFX_FileRead* pFileAccess,
- std::unique_ptr<CPDF_Document> pDocument) {
+CPDF_Parser::Error CPDF_Parser::StartParse(IFX_FileRead* pFileAccess,
+ CPDF_Document* pDocument) {
CloseParser();
m_bXRefStream = FALSE;
@@ -190,7 +189,7 @@
return FORMAT_ERROR;
m_pSyntax->RestorePos(m_pSyntax->m_FileLen - m_pSyntax->m_HeaderOffset - 9);
- m_pDocument = std::move(pDocument);
+ m_pDocument = pDocument;
FX_BOOL bXRefRebuilt = FALSE;
if (m_pSyntax->SearchWord("startxref", TRUE, FALSE, 4096)) {
@@ -765,7 +764,7 @@
last_obj = start_pos;
FX_FILESIZE obj_end = 0;
CPDF_Object* pObject = ParseIndirectObjectAtByStrict(
- m_pDocument.get(), obj_pos, objnum, &obj_end);
+ m_pDocument, obj_pos, objnum, &obj_end);
if (CPDF_Stream* pStream = ToStream(pObject)) {
if (CPDF_Dictionary* pDict = pStream->GetDict()) {
if ((pDict->KeyExist("Type")) &&
@@ -828,8 +827,7 @@
last_trailer = pos + i - 7;
m_pSyntax->RestorePos(pos + i - m_pSyntax->m_HeaderOffset);
- CPDF_Object* pObj =
- m_pSyntax->GetObject(m_pDocument.get(), 0, 0, true);
+ CPDF_Object* pObj = m_pSyntax->GetObject(m_pDocument, 0, 0, true);
if (pObj) {
if (!pObj->IsDictionary() && !pObj->AsStream()) {
pObj->Release();
@@ -851,7 +849,7 @@
uint32_t dwObjNum =
pElement ? pElement->GetObjNum() : 0;
if (dwObjNum) {
- m_pTrailer->SetAtReference(key, m_pDocument.get(),
+ m_pTrailer->SetAtReference(key, m_pDocument,
dwObjNum);
} else {
m_pTrailer->SetAt(key, pElement->Clone());
@@ -975,7 +973,7 @@
}
FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
- CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument.get(), *pos, 0);
+ CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, *pos, 0);
if (!pObject)
return FALSE;
@@ -1478,7 +1476,7 @@
return nullptr;
std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pObj(
- m_pSyntax->GetObject(m_pDocument.get(), 0, 0, true));
+ m_pSyntax->GetObject(m_pDocument, 0, 0, true));
if (!ToDictionary(pObj.get()))
return nullptr;
return pObj.release()->AsDictionary();
@@ -1550,9 +1548,8 @@
return FALSE;
}
-CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(
- IFX_FileRead* pFileAccess,
- std::unique_ptr<CPDF_Document> pDocument) {
+CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(IFX_FileRead* pFileAccess,
+ CPDF_Document* pDocument) {
CloseParser();
m_bXRefStream = FALSE;
m_LastXRefOffset = 0;
@@ -1567,7 +1564,7 @@
return StartParse(pFileAccess, std::move(pDocument));
}
- m_pDocument = std::move(pDocument);
+ m_pDocument = pDocument;
FX_FILESIZE dwFirstXRefOffset = m_pSyntax->SavePos();
FX_BOOL bXRefRebuilt = FALSE;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_document.h b/core/fpdfapi/fpdf_parser/include/cpdf_document.h
index 31988d8..f332242 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_document.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_document.h
@@ -40,10 +40,10 @@
class CPDF_Document : public CPDF_IndirectObjectHolder {
public:
- explicit CPDF_Document(CPDF_Parser* pParser);
+ explicit CPDF_Document(std::unique_ptr<CPDF_Parser> pParser);
~CPDF_Document() override;
- CPDF_Parser* GetParser() const { return m_pParser; }
+ CPDF_Parser* GetParser() const { return m_pParser.get(); }
CPDF_Dictionary* GetRoot() const { return m_pRootDict; }
CPDF_Dictionary* GetInfo() const { return m_pInfoDict; }
@@ -128,7 +128,7 @@
FX_BOOL CheckOCGVisible(CPDF_Dictionary* pOCG, FX_BOOL bPrinting);
CPDF_Object* ParseIndirectObject(uint32_t objnum) override;
- CPDF_Parser* m_pParser;
+ std::unique_ptr<CPDF_Parser> m_pParser;
CPDF_Dictionary* m_pRootDict;
CPDF_Dictionary* m_pInfoDict;
CFX_ByteString m_ID1;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
index a69f0fe..d6a5d57 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
@@ -37,17 +37,14 @@
CPDF_Parser();
~CPDF_Parser();
- Error StartParse(IFX_FileRead* pFile,
- std::unique_ptr<CPDF_Document> pDocument);
-
- Error StartLinearizedParse(IFX_FileRead* pFile,
- std::unique_ptr<CPDF_Document> pDocument);
+ Error StartParse(IFX_FileRead* pFile, CPDF_Document* pDocument);
+ Error StartLinearizedParse(IFX_FileRead* pFile, CPDF_Document* pDocument);
void SetPassword(const FX_CHAR* password) { m_Password = password; }
CFX_ByteString GetPassword() { return m_Password; }
CPDF_Dictionary* GetTrailer() const { return m_pTrailer; }
FX_FILESIZE GetLastXRefOffset() const { return m_LastXRefOffset; }
- CPDF_Document* GetDocument() const { return m_pDocument.get(); }
+ CPDF_Document* GetDocument() const { return m_pDocument; }
uint32_t GetPermissions() const;
uint32_t GetRootObjNum();
@@ -120,7 +117,7 @@
// the objects.
bool VerifyCrossRefV4();
- std::unique_ptr<CPDF_Document> m_pDocument;
+ CPDF_Document* m_pDocument; // not owned
std::unique_ptr<CPDF_SyntaxParser> m_pSyntax;
bool m_bOwnFileRead;
int m_FileVersion;
diff --git a/fpdfsdk/fpdf_dataavail.cpp b/fpdfsdk/fpdf_dataavail.cpp
index 5dd4240..7b9ba32 100644
--- a/fpdfsdk/fpdf_dataavail.cpp
+++ b/fpdfsdk/fpdf_dataavail.cpp
@@ -139,16 +139,17 @@
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 896c7d0..a555e96 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_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 29fe88d..0c3a95d 100644
--- a/fpdfsdk/fpdfview.cpp
+++ b/fpdfsdk/fpdfview.cpp
@@ -66,8 +66,8 @@
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 @@
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 @@
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 @@
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_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 605e53c..d4c43c5 100644
--- a/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
+++ b/fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp
@@ -45,10 +45,10 @@
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 @@
}
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 @@
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 ed4c872..c83c770 100644
--- a/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h
+++ b/fpdfsdk/fpdfxfa/include/fpdfxfa_doc.h
@@ -27,12 +27,13 @@
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 @@
}
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 96acb78..131334a 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 @@
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();
}
}
