Make indirect object holder private.

This CL moves the m_IndirectObjs map to be private to the IndirectObjectHolder.
Various bits of code have been updated to use the accessors to the map.

This CL fixes the issue with the last time this landed by removing the objnum
check from GetIndirectObject() which appears to have caused the crashes.

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

5 files changed, 34 insertions, 30 deletions

diff --git a/core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp b/core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp
index 9ad1d060cc..7ece85a602 100644
--- a/core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp
+++ b/core/fpdfapi/fpdf_edit/fpdf_edit_create.cpp
@@ -1233,8 +1233,7 @@ int32_t CPDF_Creator::WriteOldIndirectObject(uint32_t objnum) {
     return 0;
 
   m_ObjectOffset[objnum] = m_Offset;
-  FX_BOOL bExistInMap =
-      pdfium::ContainsKey(m_pDocument->m_IndirectObjs, objnum);
+  FX_BOOL bExistInMap = !!m_pDocument->GetIndirectObject(objnum);
   const uint8_t object_type = m_pParser->GetObjectType(objnum);
   bool bObjStm = (object_type == 2) && m_pEncryptDict && !m_pXRefStream;
   if (m_pParser->IsVersionUpdated() || m_bSecurityChanged || bExistInMap ||
@@ -1320,15 +1319,15 @@ int32_t CPDF_Creator::WriteNewObjs(FX_BOOL bIncremental, IFX_Pause* pPause) {
   int32_t index = (int32_t)(uintptr_t)m_Pos;
   while (index < iCount) {
     uint32_t objnum = m_NewObjNumArray.ElementAt(index);
-    auto it = m_pDocument->m_IndirectObjs.find(objnum);
-    if (it == m_pDocument->m_IndirectObjs.end()) {
+    CPDF_Object* pObj = m_pDocument->GetIndirectObject(objnum);
+    if (!pObj) {
       ++index;
       continue;
     }
     m_ObjectOffset[objnum] = m_Offset;
-    if (WriteIndirectObj(it->second)) {
+    if (WriteIndirectObj(pObj))
       return -1;
-    }
+
     index++;
     if (pPause && pPause->NeedToPauseNow()) {
       m_Pos = (FX_POSITION)(uintptr_t)index;
@@ -1363,7 +1362,7 @@ void CPDF_Creator::InitOldObjNumOffsets() {
 void CPDF_Creator::InitNewObjNumOffsets() {
   FX_BOOL bIncremental = (m_dwFlags & FPDFCREATE_INCREMENTAL) != 0;
   FX_BOOL bNoOriginal = (m_dwFlags & FPDFCREATE_NO_ORIGINAL) != 0;
-  for (const auto& pair : m_pDocument->m_IndirectObjs) {
+  for (const auto& pair : *m_pDocument) {
     const uint32_t objnum = pair.first;
     const CPDF_Object* pObj = pair.second;
     if (bIncremental || pObj->GetObjNum() == CPDF_Object::kInvalidObjNum)
diff --git a/core/fpdfapi/fpdf_parser/cfdf_document.cpp b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
index c268503715..773b97af2b 100644
--- a/core/fpdfapi/fpdf_parser/cfdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cfdf_document.cpp
@@ -84,13 +84,13 @@ void CFDF_Document::ParseStream(IFX_FileRead* pFile, FX_BOOL bOwnFile) {
 }
 
 FX_BOOL CFDF_Document::WriteBuf(CFX_ByteTextBuf& buf) const {
-  if (!m_pRootDict) {
+  if (!m_pRootDict)
     return FALSE;
-  }
+
   buf << "%FDF-1.2\r\n";
-  for (const auto& pair : m_IndirectObjs) {
+  for (const auto& pair : *this)
     buf << pair.first << " 0 obj\r\n" << pair.second << "\r\nendobj\r\n\r\n";
-  }
+
   buf << "trailer\r\n<</Root " << m_pRootDict->GetObjNum()
       << " 0 R>>\r\n%%EOF\r\n";
   return TRUE;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
index a05a24e07f..31d44b01fe 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
@@ -729,9 +729,9 @@ uint32_t CPDF_Document::GetUserPermissions() const {
 }
 
 FX_BOOL CPDF_Document::IsFormStream(uint32_t objnum, FX_BOOL& bForm) const {
-  auto it = m_IndirectObjs.find(objnum);
-  if (it != m_IndirectObjs.end()) {
-    CPDF_Stream* pStream = it->second->AsStream();
+  CPDF_Object* pObj = GetIndirectObject(objnum);
+  if (pObj) {
+    CPDF_Stream* pStream = pObj->AsStream();
     bForm = pStream && pStream->GetDict()->GetStringBy("Subtype") == "Form";
     return TRUE;
   }
diff --git a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
index bb29da5052..d00b4a7961 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp
@@ -20,20 +20,25 @@ CPDF_IndirectObjectHolder::~CPDF_IndirectObjectHolder() {
     pair.second->Destroy();
 }
 
+CPDF_Object* CPDF_IndirectObjectHolder::GetIndirectObject(
+    uint32_t objnum) const {
+  auto it = m_IndirectObjs.find(objnum);
+  return it != m_IndirectObjs.end() ? it->second : nullptr;
+}
+
 CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
     uint32_t objnum) {
   if (objnum == 0)
     return nullptr;
 
-  auto it = m_IndirectObjs.find(objnum);
-  if (it != m_IndirectObjs.end())
-    return it->second->GetObjNum() != CPDF_Object::kInvalidObjNum ? it->second
-                                                                  : nullptr;
+  CPDF_Object* pObj = GetIndirectObject(objnum);
+  if (pObj)
+    return pObj->GetObjNum() != CPDF_Object::kInvalidObjNum ? pObj : nullptr;
 
   if (!m_pParser)
     return nullptr;
 
-  CPDF_Object* pObj = m_pParser->ParseIndirectObject(this, objnum);
+  pObj = m_pParser->ParseIndirectObject(this, objnum);
   if (!pObj)
     return nullptr;
 
@@ -62,13 +67,13 @@ bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration(
   if (!objnum || !pObj)
     return false;
 
-  auto it = m_IndirectObjs.find(objnum);
-  if (it != m_IndirectObjs.end()) {
-    if (pObj->GetGenNum() <= it->second->GetGenNum()) {
+  CPDF_Object* pOldObj = GetIndirectObject(objnum);
+  if (pOldObj) {
+    if (pObj->GetGenNum() <= pOldObj->GetGenNum()) {
       pObj->Destroy();
       return false;
     }
-    it->second->Destroy();
+    pOldObj->Destroy();
   }
   pObj->m_ObjNum = objnum;
   m_IndirectObjs[objnum] = pObj;
@@ -77,11 +82,10 @@ bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration(
 }
 
 void CPDF_IndirectObjectHolder::ReleaseIndirectObject(uint32_t objnum) {
-  auto it = m_IndirectObjs.find(objnum);
-  if (it == m_IndirectObjs.end() ||
-      it->second->GetObjNum() == CPDF_Object::kInvalidObjNum) {
+  CPDF_Object* pObj = GetIndirectObject(objnum);
+  if (!pObj || pObj->GetObjNum() == CPDF_Object::kInvalidObjNum)
     return;
-  }
-  it->second->Destroy();
-  m_IndirectObjs.erase(it);
+
+  pObj->Destroy();
+  m_IndirectObjs.erase(objnum);
 }
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 31e0aba5ea..27a9bc39db 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h
@@ -22,6 +22,7 @@ class CPDF_IndirectObjectHolder {
   explicit CPDF_IndirectObjectHolder(CPDF_Parser* pParser);
   ~CPDF_IndirectObjectHolder();
 
+  CPDF_Object* GetIndirectObject(uint32_t objnum) const;
   CPDF_Object* GetOrParseIndirectObject(uint32_t objnum);
   void ReleaseIndirectObject(uint32_t objnum);
 
@@ -40,10 +41,10 @@ class CPDF_IndirectObjectHolder {
 
  protected:
   CPDF_Parser* m_pParser;
-  std::map<uint32_t, CPDF_Object*> m_IndirectObjs;
 
  private:
   uint32_t m_LastObjNum;
+  std::map<uint32_t, CPDF_Object*> m_IndirectObjs;
 };
 
 #endif  // CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_INDIRECT_OBJECT_HOLDER_H_