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

4 files changed, 28 insertions, 23 deletions

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_