Move parser out of IndirectObjectHolder

This Cl moves the parser out of the indirect object holder and into the
CPDF_Document where it is used.

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

9 files changed, 38 insertions, 28 deletions

diff --git a/core/fpdfapi/fpdf_parser/cfdf_document.cpp b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
index 773b97af2b..e2e4d0cf1e 100644
--- a/core/fpdfapi/fpdf_parser/cfdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
@@ -10,16 +10,17 @@
 #include "core/fpdfapi/fpdf_parser/cpdf_syntax_parser.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
 
-CFDF_Document::CFDF_Document() : CPDF_IndirectObjectHolder(nullptr) {
-  m_pRootDict = nullptr;
-  m_pFile = nullptr;
-  m_bOwnFile = FALSE;
-}
+CFDF_Document::CFDF_Document()
+    : CPDF_IndirectObjectHolder(),
+      m_pRootDict(nullptr),
+      m_pFile(nullptr),
+      m_bOwnFile(FALSE) {}
+
 CFDF_Document::~CFDF_Document() {
-  if (m_bOwnFile && m_pFile) {
+  if (m_bOwnFile && m_pFile)
     m_pFile->Release();
-  }
 }
+
 CFDF_Document* CFDF_Document::CreateNewDoc() {
   CFDF_Document* pDoc = new CFDF_Document;
   pDoc->m_pRootDict = new CPDF_Dictionary;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
index 41a49cadab..ef9b65c5c4 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
@@ -109,7 +109,7 @@ TEST(cpdf_array, Clone) {
     ScopedArray arr(new CPDF_Array);
     // Indirect references to indirect objects.
     std::unique_ptr<CPDF_IndirectObjectHolder> obj_holder(
-        new CPDF_IndirectObjectHolder(nullptr));
+        new CPDF_IndirectObjectHolder());
     for (size_t i = 0; i < kNumOfRows; ++i) {
       CPDF_Array* arr_elem = new CPDF_Array;
       for (size_t j = 0; j < kNumOfRowElems; ++j) {
diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
index 31d44b01fe..b11d0c1b79 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
@@ -484,20 +484,28 @@ int CountPages(CPDF_Dictionary* pPages,
 }  // namespace
 
 CPDF_Document::CPDF_Document(CPDF_Parser* pParser)
-    : CPDF_IndirectObjectHolder(pParser),
+    : CPDF_IndirectObjectHolder(),
+      m_pParser(pParser),
       m_pRootDict(nullptr),
       m_pInfoDict(nullptr),
       m_bLinearized(false),
       m_iFirstPageNo(0),
       m_dwFirstPageObjNum(0),
       m_pDocPage(new CPDF_DocPageData(this)),
-      m_pDocRender(new CPDF_DocRenderData(this)) {}
+      m_pDocRender(new CPDF_DocRenderData(this)) {
+  if (pParser)
+    SetLastObjNum(m_pParser->GetLastObjNum());
+}
 
 CPDF_Document::~CPDF_Document() {
   delete m_pDocPage;
   CPDF_ModuleMgr::Get()->GetPageModule()->ClearStockFont(this);
 }
 
+CPDF_Object* CPDF_Document::ParseIndirectObject(uint32_t objnum) {
+  return m_pParser ? m_pParser->ParseIndirectObject(this, objnum) : nullptr;
+}
+
 void CPDF_Document::LoadDocInternal() {
   SetLastObjNum(m_pParser->GetLastObjNum());
 
diff --git a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
index d00b4a7961..711b3da614 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
@@ -9,11 +9,7 @@
 #include "core/fpdfapi/fpdf_parser/include/cpdf_object.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_parser.h"
 
-CPDF_IndirectObjectHolder::CPDF_IndirectObjectHolder(CPDF_Parser* pParser)
-    : m_pParser(pParser), m_LastObjNum(0) {
-  if (pParser)
-    m_LastObjNum = m_pParser->GetLastObjNum();
-}
+CPDF_IndirectObjectHolder::CPDF_IndirectObjectHolder() : m_LastObjNum(0) {}
 
 CPDF_IndirectObjectHolder::~CPDF_IndirectObjectHolder() {
   for (const auto& pair : m_IndirectObjs)
@@ -35,10 +31,7 @@ CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
   if (pObj)
     return pObj->GetObjNum() != CPDF_Object::kInvalidObjNum ? pObj : nullptr;
 
-  if (!m_pParser)
-    return nullptr;
-
-  pObj = m_pParser->ParseIndirectObject(this, objnum);
+  pObj = ParseIndirectObject(objnum);
   if (!pObj)
     return nullptr;
 
@@ -51,6 +44,10 @@ CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
   return pObj;
 }
 
+CPDF_Object* CPDF_IndirectObjectHolder::ParseIndirectObject(uint32_t objnum) {
+  return nullptr;
+}
+
 uint32_t CPDF_IndirectObjectHolder::AddIndirectObject(CPDF_Object* pObj) {
   if (pObj->m_ObjNum)
     return pObj->m_ObjNum;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
index 018300d462..02e50bcfec 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
@@ -94,7 +94,7 @@ class PDFObjectsTest : public testing::Test {
       m_DirectObjs.emplace_back(objs[i]);
 
     // Indirect references to indirect objects.
-    m_ObjHolder.reset(new CPDF_IndirectObjectHolder(nullptr));
+    m_ObjHolder.reset(new CPDF_IndirectObjectHolder());
     m_IndirectObjs = {boolean_true_obj, number_int_obj, str_spec_obj, name_obj,
                       m_ArrayObj,       m_DictObj,      stream_obj};
     for (size_t i = 0; i < m_IndirectObjs.size(); ++i) {
@@ -712,7 +712,7 @@ TEST(PDFArrayTest, AddStringAndName) {
 
 TEST(PDFArrayTest, AddReferenceAndGetObjectAt) {
   std::unique_ptr<CPDF_IndirectObjectHolder> holder(
-      new CPDF_IndirectObjectHolder(nullptr));
+      new CPDF_IndirectObjectHolder());
   CPDF_Boolean* boolean_obj = new CPDF_Boolean(true);
   CPDF_Number* int_obj = new CPDF_Number(-1234);
   CPDF_Number* float_obj = new CPDF_Number(2345.089f);
@@ -769,7 +769,7 @@ TEST(PDFObjectTest, CloneCheckLoop) {
   }
   {
     std::unique_ptr<CPDF_IndirectObjectHolder> m_ObjHolder(
-        new CPDF_IndirectObjectHolder(nullptr));
+        new CPDF_IndirectObjectHolder());
     // Create an object with a reference loop.
     CPDF_Dictionary* dict_obj = new CPDF_Dictionary;
     CPDF_Array* arr_obj = new CPDF_Array;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
index 26adf7bb13..ffd3f79a62 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
@@ -982,6 +982,8 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
   if (m_pDocument) {
     CPDF_Dictionary* pRootDict = m_pDocument->GetRoot();
     if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum) {
+      // If |pObject| has an objnum assigned then this will leak as Release()
+      // will early exit.
       if (pObject->IsStream())
         pObject->Release();
       return FALSE;
diff --git a/core/fpdfapi/fpdf_parser/include/cfdf_document.h b/core/fpdfapi/fpdf_parser/include/cfdf_document.h
index 18b9442fa9..834aecd1ba 100644
--- a/core/fpdfapi/fpdf_parser/include/cfdf_document.h
+++ b/core/fpdfapi/fpdf_parser/include/cfdf_document.h
@@ -19,7 +19,7 @@ class CFDF_Document : public CPDF_IndirectObjectHolder {
   static CFDF_Document* ParseFile(IFX_FileRead* pFile,
                                   FX_BOOL bOwnFile = FALSE);
   static CFDF_Document* ParseMemory(const uint8_t* pData, uint32_t size);
-  ~CFDF_Document();
+  ~CFDF_Document() override;
 
   FX_BOOL WriteBuf(CFX_ByteTextBuf& buf) const;
   CPDF_Dictionary* GetRoot() const { return m_pRootDict; }
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_document.h b/core/fpdfapi/fpdf_parser/include/cpdf_document.h
index 4a12ea97dc..31988d8c9d 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_document.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_document.h
@@ -23,6 +23,7 @@ class CPDF_Font;
 class CPDF_FontEncoding;
 class CPDF_IccProfile;
 class CPDF_Image;
+class CPDF_Parser;
 class CPDF_Pattern;
 class CPDF_StreamAcc;
 class JBig2_DocumentContext;
@@ -40,7 +41,7 @@ class JBig2_DocumentContext;
 class CPDF_Document : public CPDF_IndirectObjectHolder {
  public:
   explicit CPDF_Document(CPDF_Parser* pParser);
-  ~CPDF_Document();
+  ~CPDF_Document() override;
 
   CPDF_Parser* GetParser() const { return m_pParser; }
   CPDF_Dictionary* GetRoot() const { return m_pRootDict; }
@@ -125,7 +126,9 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
                     int& index,
                     int level = 0);
   FX_BOOL CheckOCGVisible(CPDF_Dictionary* pOCG, FX_BOOL bPrinting);
+  CPDF_Object* ParseIndirectObject(uint32_t objnum) override;
 
+  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_indirect_object_holder.h b/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
index 27a9bc39db..57dafb0528 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
@@ -12,15 +12,14 @@
 #include "core/fxcrt/include/fx_system.h"
 
 class CPDF_Object;
-class CPDF_Parser;
 
 class CPDF_IndirectObjectHolder {
  public:
   using iterator = std::map<uint32_t, CPDF_Object*>::iterator;
   using const_iterator = std::map<uint32_t, CPDF_Object*>::const_iterator;
 
-  explicit CPDF_IndirectObjectHolder(CPDF_Parser* pParser);
-  ~CPDF_IndirectObjectHolder();
+  CPDF_IndirectObjectHolder();
+  virtual ~CPDF_IndirectObjectHolder();
 
   CPDF_Object* GetIndirectObject(uint32_t objnum) const;
   CPDF_Object* GetOrParseIndirectObject(uint32_t objnum);
@@ -40,7 +39,7 @@ class CPDF_IndirectObjectHolder {
   const_iterator end() const { return m_IndirectObjs.end(); }
 
  protected:
-  CPDF_Parser* m_pParser;
+  virtual CPDF_Object* ParseIndirectObject(uint32_t objnum);
 
  private:
   uint32_t m_LastObjNum;