Cleanup CPDF_DocPageData release methods and callers.

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

5 files changed, 133 insertions, 123 deletions

diff --git a/core/fpdfapi/fpdf_page/cpdf_color.cpp b/core/fpdfapi/fpdf_page/cpdf_color.cpp
index e6fe3e88e9..054b1ecf40 100644
--- a/core/fpdfapi/fpdf_page/cpdf_color.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_color.cpp
@@ -41,7 +41,7 @@ void CPDF_Color::ReleaseBuffer() {
 }
 
 void CPDF_Color::ReleaseColorSpace() {
-  if (m_pCS && m_pCS->m_pDocument && m_pCS->GetArray()) {
+  if (m_pCS && m_pCS->m_pDocument) {
     m_pCS->m_pDocument->GetPageData()->ReleaseColorSpace(m_pCS->GetArray());
     m_pCS = nullptr;
   }
diff --git a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
index e7f68224a4..059679a8cd 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
@@ -6,6 +6,9 @@
 
 #include "core/fpdfapi/fpdf_page/pageint.h"
 
+#include <algorithm>
+#include <set>
+
 #include "core/fdrm/crypto/include/fx_crypt.h"
 #include "core/fpdfapi/fpdf_font/cpdf_type1font.h"
 #include "core/fpdfapi/fpdf_font/font_int.h"
@@ -100,26 +103,26 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease) {
 
   for (auto it = m_FontFileMap.begin(); it != m_FontFileMap.end();) {
     auto curr_it = it++;
-    CPDF_CountedStreamAcc* ftData = curr_it->second;
-    if (!ftData->get())
+    CPDF_CountedStreamAcc* pCountedFont = curr_it->second;
+    if (!pCountedFont->get())
       continue;
 
-    if (bForceRelease || ftData->use_count() < 2) {
-      delete ftData->get();
-      delete ftData;
+    if (bForceRelease || pCountedFont->use_count() < 2) {
+      delete pCountedFont->get();
+      delete pCountedFont;
       m_FontFileMap.erase(curr_it);
     }
   }
 
   for (auto it = m_ImageMap.begin(); it != m_ImageMap.end();) {
     auto curr_it = it++;
-    CPDF_CountedImage* imageData = curr_it->second;
-    if (!imageData->get())
+    CPDF_CountedImage* pCountedImage = curr_it->second;
+    if (!pCountedImage->get())
       continue;
 
-    if (bForceRelease || imageData->use_count() < 2) {
-      delete imageData->get();
-      delete imageData;
+    if (bForceRelease || pCountedImage->use_count() < 2) {
+      delete pCountedImage->get();
+      delete pCountedImage;
       m_ImageMap.erase(curr_it);
     }
   }
@@ -127,37 +130,32 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease) {
 
 CPDF_Font* CPDF_DocPageData::GetFont(CPDF_Dictionary* pFontDict,
                                      FX_BOOL findOnly) {
-  if (!pFontDict) {
-    return nullptr;
-  }
-  if (findOnly) {
-    auto it = m_FontMap.find(pFontDict);
-    if (it != m_FontMap.end() && it->second->get()) {
-      return it->second->AddRef();
-    }
+  if (!pFontDict)
     return nullptr;
-  }
 
-  CPDF_CountedFont* fontData = nullptr;
+  CPDF_CountedFont* pFontData = nullptr;
   auto it = m_FontMap.find(pFontDict);
   if (it != m_FontMap.end()) {
-    fontData = it->second;
-    if (fontData->get()) {
-      return fontData->AddRef();
+    pFontData = it->second;
+    if (pFontData->get()) {
+      return pFontData->AddRef();
     }
   }
 
+  if (findOnly)
+    return nullptr;
+
   CPDF_Font* pFont = CPDF_Font::CreateFontF(m_pPDFDoc, pFontDict);
-  if (!pFont) {
+  if (!pFont)
     return nullptr;
-  }
-  if (!fontData) {
-    fontData = new CPDF_CountedFont(pFont);
-    m_FontMap[pFontDict] = fontData;
+
+  if (pFontData) {
+    pFontData->reset(pFont);
   } else {
-    fontData->reset(pFont);
+    pFontData = new CPDF_CountedFont(pFont);
+    m_FontMap[pFontDict] = pFontData;
   }
-  return fontData->AddRef();
+  return pFontData->AddRef();
 }
 
 CPDF_Font* CPDF_DocPageData::GetStandardFont(const CFX_ByteString& fontName,
@@ -203,7 +201,7 @@ CPDF_Font* CPDF_DocPageData::GetStandardFont(const CFX_ByteString& fontName,
   return fontData->AddRef();
 }
 
-void CPDF_DocPageData::ReleaseFont(CPDF_Dictionary* pFontDict) {
+void CPDF_DocPageData::ReleaseFont(const CPDF_Dictionary* pFontDict) {
   if (!pFontDict)
     return;
 
@@ -211,13 +209,15 @@ void CPDF_DocPageData::ReleaseFont(CPDF_Dictionary* pFontDict) {
   if (it == m_FontMap.end())
     return;
 
-  CPDF_CountedFont* fontData = it->second;
-  if (fontData->get()) {
-    fontData->RemoveRef();
-    if (fontData->use_count() == 0) {
-      fontData->clear();
-    }
-  }
+  CPDF_CountedFont* pFontData = it->second;
+  if (!pFontData->get())
+    return;
+
+  pFontData->RemoveRef();
+  if (pFontData->use_count() != 0)
+    return;
+
+  pFontData->clear();
 }
 
 CPDF_ColorSpace* CPDF_DocPageData::GetColorSpace(
@@ -317,7 +317,7 @@ CPDF_ColorSpace* CPDF_DocPageData::GetCopiedColorSpace(CPDF_Object* pCSObj) {
   return nullptr;
 }
 
-void CPDF_DocPageData::ReleaseColorSpace(CPDF_Object* pColorSpace) {
+void CPDF_DocPageData::ReleaseColorSpace(const CPDF_Object* pColorSpace) {
   if (!pColorSpace)
     return;
 
@@ -325,14 +325,16 @@ void CPDF_DocPageData::ReleaseColorSpace(CPDF_Object* pColorSpace) {
   if (it == m_ColorSpaceMap.end())
     return;
 
-  CPDF_CountedColorSpace* csData = it->second;
-  if (csData->get()) {
-    csData->RemoveRef();
-    if (csData->use_count() == 0) {
-      csData->get()->ReleaseCS();
-      csData->reset(nullptr);
-    }
-  }
+  CPDF_CountedColorSpace* pCountedColorSpace = it->second;
+  if (!pCountedColorSpace->get())
+    return;
+
+  pCountedColorSpace->RemoveRef();
+  if (pCountedColorSpace->use_count() != 0)
+    return;
+
+  pCountedColorSpace->get()->ReleaseCS();
+  pCountedColorSpace->reset(nullptr);
 }
 
 CPDF_Pattern* CPDF_DocPageData::GetPattern(CPDF_Object* pPatternObj,
@@ -376,7 +378,7 @@ CPDF_Pattern* CPDF_DocPageData::GetPattern(CPDF_Object* pPatternObj,
   return ptData->AddRef();
 }
 
-void CPDF_DocPageData::ReleasePattern(CPDF_Object* pPatternObj) {
+void CPDF_DocPageData::ReleasePattern(const CPDF_Object* pPatternObj) {
   if (!pPatternObj)
     return;
 
@@ -384,13 +386,15 @@ void CPDF_DocPageData::ReleasePattern(CPDF_Object* pPatternObj) {
   if (it == m_PatternMap.end())
     return;
 
-  CPDF_CountedPattern* ptData = it->second;
-  if (ptData->get()) {
-    ptData->RemoveRef();
-    if (ptData->use_count() == 0) {
-      ptData->clear();
-    }
-  }
+  CPDF_CountedPattern* pPattern = it->second;
+  if (!pPattern->get())
+    return;
+
+  pPattern->RemoveRef();
+  if (pPattern->use_count() != 0)
+    return;
+
+  pPattern->clear();
 }
 
 CPDF_Image* CPDF_DocPageData::GetImage(CPDF_Object* pImageStream) {
@@ -399,36 +403,40 @@ CPDF_Image* CPDF_DocPageData::GetImage(CPDF_Object* pImageStream) {
 
   const uint32_t dwImageObjNum = pImageStream->GetObjNum();
   auto it = m_ImageMap.find(dwImageObjNum);
-  if (it != m_ImageMap.end()) {
+  if (it != m_ImageMap.end())
     return it->second->AddRef();
-  }
 
   CPDF_Image* pImage = new CPDF_Image(m_pPDFDoc);
-  pImage->LoadImageF(pImageStream->AsStream(), FALSE);
+  pImage->LoadImageF(pImageStream->AsStream(), false);
 
-  CPDF_CountedImage* imageData = new CPDF_CountedImage(pImage);
-  m_ImageMap[dwImageObjNum] = imageData;
-  return imageData->AddRef();
+  CPDF_CountedImage* pCountedImage = new CPDF_CountedImage(pImage);
+  m_ImageMap[dwImageObjNum] = pCountedImage;
+  return pCountedImage->AddRef();
 }
 
-void CPDF_DocPageData::ReleaseImage(CPDF_Object* pImageStream) {
-  if (!pImageStream || !pImageStream->GetObjNum())
+void CPDF_DocPageData::ReleaseImage(const CPDF_Object* pImageStream) {
+  if (!pImageStream)
+    return;
+
+  uint32_t dwObjNum = pImageStream->GetObjNum();
+  if (!dwObjNum)
     return;
 
-  auto it = m_ImageMap.find(pImageStream->GetObjNum());
+  auto it = m_ImageMap.find(dwObjNum);
   if (it == m_ImageMap.end())
     return;
 
-  CPDF_CountedImage* image = it->second;
-  if (!image)
+  CPDF_CountedImage* pCountedImage = it->second;
+  if (!pCountedImage)
     return;
 
-  image->RemoveRef();
-  if (image->use_count() == 0) {
-    delete image->get();
-    delete image;
-    m_ImageMap.erase(it);
-  }
+  pCountedImage->RemoveRef();
+  if (pCountedImage->use_count() != 0)
+    return;
+
+  delete pCountedImage->get();
+  delete pCountedImage;
+  m_ImageMap.erase(it);
 }
 
 CPDF_IccProfile* CPDF_DocPageData::GetIccProfile(
@@ -437,9 +445,8 @@ CPDF_IccProfile* CPDF_DocPageData::GetIccProfile(
     return nullptr;
 
   auto it = m_IccProfileMap.find(pIccProfileStream);
-  if (it != m_IccProfileMap.end()) {
+  if (it != m_IccProfileMap.end())
     return it->second->AddRef();
-  }
 
   CPDF_StreamAcc stream;
   stream.LoadAllData(pIccProfileStream, FALSE);
@@ -459,7 +466,7 @@ CPDF_IccProfile* CPDF_DocPageData::GetIccProfile(
   return ipData->AddRef();
 }
 
-void CPDF_DocPageData::ReleaseIccProfile(CPDF_IccProfile* pIccProfile) {
+void CPDF_DocPageData::ReleaseIccProfile(const CPDF_IccProfile* pIccProfile) {
   ASSERT(pIccProfile);
 
   for (auto it = m_IccProfileMap.begin(); it != m_IccProfileMap.end(); ++it) {
@@ -489,19 +496,18 @@ CPDF_StreamAcc* CPDF_DocPageData::GetFontFileStreamAcc(
   int32_t org_size = pFontDict->GetIntegerBy("Length1") +
                      pFontDict->GetIntegerBy("Length2") +
                      pFontDict->GetIntegerBy("Length3");
-  if (org_size < 0)
-    org_size = 0;
+  org_size = std::max(org_size, 0);
 
   CPDF_StreamAcc* pFontFile = new CPDF_StreamAcc;
   pFontFile->LoadAllData(pFontStream, FALSE, org_size);
 
-  CPDF_CountedStreamAcc* ftData = new CPDF_CountedStreamAcc(pFontFile);
-  m_FontFileMap[pFontStream] = ftData;
-  return ftData->AddRef();
+  CPDF_CountedStreamAcc* pCountedFont = new CPDF_CountedStreamAcc(pFontFile);
+  m_FontFileMap[pFontStream] = pCountedFont;
+  return pCountedFont->AddRef();
 }
 
-void CPDF_DocPageData::ReleaseFontFileStreamAcc(CPDF_Stream* pFontStream,
-                                                FX_BOOL bForce) {
+void CPDF_DocPageData::ReleaseFontFileStreamAcc(
+    const CPDF_Stream* pFontStream) {
   if (!pFontStream)
     return;
 
@@ -509,16 +515,17 @@ void CPDF_DocPageData::ReleaseFontFileStreamAcc(CPDF_Stream* pFontStream,
   if (it == m_FontFileMap.end())
     return;
 
-  CPDF_CountedStreamAcc* findData = it->second;
-  if (!findData)
+  CPDF_CountedStreamAcc* pCountedStream = it->second;
+  if (!pCountedStream)
     return;
 
-  findData->RemoveRef();
-  if (findData->use_count() == 0 || bForce) {
-    delete findData->get();
-    delete findData;
-    m_FontFileMap.erase(it);
-  }
+  pCountedStream->RemoveRef();
+  if (pCountedStream->use_count() != 0)
+    return;
+
+  delete pCountedStream->get();
+  delete pCountedStream;
+  m_FontFileMap.erase(it);
 }
 
 CPDF_CountedColorSpace* CPDF_DocPageData::FindColorSpacePtr(
diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
index 1881244a0f..5a77ba70b2 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
@@ -6,6 +6,8 @@
 
 #include "core/fpdfapi/fpdf_page/pageint.h"
 
+#include <memory>
+#include <utility>
 #include <vector>
 
 #include "core/fpdfapi/fpdf_edit/include/cpdf_creator.h"
@@ -643,7 +645,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() {
     }
   }
   pDict->SetAtName("Subtype", "Image");
-  CPDF_ImageObject* pImgObj = AddImage(pStream, nullptr, TRUE);
+  CPDF_ImageObject* pImgObj = AddImage(pStream, nullptr, true);
   if (!pImgObj) {
     if (pStream) {
       pStream->Release();
@@ -724,7 +726,7 @@ void CPDF_StreamContentParser::Handle_ExecuteXObject() {
   CFX_ByteString name = GetString(0);
   if (name == m_LastImageName && m_pLastImage && m_pLastImage->GetStream() &&
       m_pLastImage->GetStream()->GetObjNum()) {
-    AddImage(nullptr, m_pLastImage, FALSE);
+    AddImage(nullptr, m_pLastImage, false);
     return;
   }
 
@@ -739,7 +741,7 @@ void CPDF_StreamContentParser::Handle_ExecuteXObject() {
     type = pXObject->GetDict()->GetStringBy("Subtype");
 
   if (type == "Image") {
-    CPDF_ImageObject* pObj = AddImage(pXObject, nullptr, FALSE);
+    CPDF_ImageObject* pObj = AddImage(pXObject, nullptr, false);
     m_LastImageName = name;
     m_pLastImage = pObj->m_pImage;
     if (!m_pObjectHolder->HasImageMask())
@@ -772,10 +774,10 @@ void CPDF_StreamContentParser::AddForm(CPDF_Stream* pStream) {
 
 CPDF_ImageObject* CPDF_StreamContentParser::AddImage(CPDF_Stream* pStream,
                                                      CPDF_Image* pImage,
-                                                     FX_BOOL bInline) {
-  if (!pStream && !pImage) {
+                                                     bool bInline) {
+  if (!pStream && !pImage)
     return nullptr;
-  }
+
   CFX_Matrix ImageMatrix;
   ImageMatrix.Copy(m_pCurStates->m_CTM);
   ImageMatrix.Concat(m_mtContentToUser);
diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
index 42adb84638..2639d7aa5c 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp
@@ -160,36 +160,36 @@ CPDF_Stream* CPDF_StreamParser::ReadInlineStream(CPDF_Document* pDoc,
     uint32_t bpc = pDict->GetIntegerBy("BitsPerComponent");
     uint32_t nComponents = 1;
     CPDF_ColorSpace* pCS = pDoc->LoadColorSpace(pCSObj);
-    if (!pCS) {
-      nComponents = 3;
-    } else {
+    if (pCS) {
       nComponents = pCS->CountComponents();
       pDoc->GetPageData()->ReleaseColorSpace(pCSObj);
+    } else {
+      nComponents = 3;
     }
     uint32_t pitch = width;
-    if (bpc && pitch > INT_MAX / bpc) {
+    if (bpc && pitch > INT_MAX / bpc)
       return nullptr;
-    }
+
     pitch *= bpc;
-    if (nComponents && pitch > INT_MAX / nComponents) {
+    if (nComponents && pitch > INT_MAX / nComponents)
       return nullptr;
-    }
+
     pitch *= nComponents;
-    if (pitch > INT_MAX - 7) {
+    if (pitch > INT_MAX - 7)
       return nullptr;
-    }
+
     pitch += 7;
     pitch /= 8;
     OrigSize = pitch;
   } else {
-    if (width > INT_MAX - 7) {
+    if (width > INT_MAX - 7)
       return nullptr;
-    }
+
     OrigSize = ((width + 7) / 8);
   }
-  if (height && OrigSize > INT_MAX / height) {
+  if (height && OrigSize > INT_MAX / height)
     return nullptr;
-  }
+
   OrigSize *= height;
   uint8_t* pData = nullptr;
   uint32_t dwStreamSize;
diff --git a/core/fpdfapi/fpdf_page/pageint.h b/core/fpdfapi/fpdf_page/pageint.h
index 08cfd0472a..30c58745c6 100644
--- a/core/fpdfapi/fpdf_page/pageint.h
+++ b/core/fpdfapi/fpdf_page/pageint.h
@@ -147,7 +147,7 @@ class CPDF_StreamContentParser {
   void AddPathObject(int FillType, FX_BOOL bStroke);
   CPDF_ImageObject* AddImage(CPDF_Stream* pStream,
                              CPDF_Image* pImage,
-                             FX_BOOL bInline);
+                             bool bInline);
   void AddDuplicateImage();
   void AddForm(CPDF_Stream* pStream);
   void SetGraphicStates(CPDF_PageObject* pObj,
@@ -324,22 +324,21 @@ class CPDF_DocPageData {
   CPDF_Font* GetFont(CPDF_Dictionary* pFontDict, FX_BOOL findOnly);
   CPDF_Font* GetStandardFont(const CFX_ByteString& fontName,
                              CPDF_FontEncoding* pEncoding);
-  void ReleaseFont(CPDF_Dictionary* pFontDict);
+  void ReleaseFont(const CPDF_Dictionary* pFontDict);
   CPDF_ColorSpace* GetColorSpace(CPDF_Object* pCSObj,
                                  const CPDF_Dictionary* pResources);
   CPDF_ColorSpace* GetCopiedColorSpace(CPDF_Object* pCSObj);
-  void ReleaseColorSpace(CPDF_Object* pColorSpace);
+  void ReleaseColorSpace(const CPDF_Object* pColorSpace);
   CPDF_Pattern* GetPattern(CPDF_Object* pPatternObj,
                            FX_BOOL bShading,
                            const CFX_Matrix& matrix);
-  void ReleasePattern(CPDF_Object* pPatternObj);
+  void ReleasePattern(const CPDF_Object* pPatternObj);
   CPDF_Image* GetImage(CPDF_Object* pImageStream);
-  void ReleaseImage(CPDF_Object* pImageStream);
+  void ReleaseImage(const CPDF_Object* pImageStream);
   CPDF_IccProfile* GetIccProfile(CPDF_Stream* pIccProfileStream);
-  void ReleaseIccProfile(CPDF_IccProfile* pIccProfile);
+  void ReleaseIccProfile(const CPDF_IccProfile* pIccProfile);
   CPDF_StreamAcc* GetFontFileStreamAcc(CPDF_Stream* pFontStream);
-  void ReleaseFontFileStreamAcc(CPDF_Stream* pFontStream,
-                                FX_BOOL bForce = FALSE);
+  void ReleaseFontFileStreamAcc(const CPDF_Stream* pFontStream);
   FX_BOOL IsForceClear() const { return m_bForceClear; }
   CPDF_CountedColorSpace* FindColorSpacePtr(CPDF_Object* pCSObj) const;
   CPDF_CountedPattern* FindPatternPtr(CPDF_Object* pPatternObj) const;
@@ -350,12 +349,14 @@ class CPDF_DocPageData {
   using CPDF_CountedImage = CPDF_CountedObject<CPDF_Image>;
   using CPDF_CountedStreamAcc = CPDF_CountedObject<CPDF_StreamAcc>;
 
-  using CPDF_ColorSpaceMap = std::map<CPDF_Object*, CPDF_CountedColorSpace*>;
-  using CPDF_FontFileMap = std::map<CPDF_Stream*, CPDF_CountedStreamAcc*>;
-  using CPDF_FontMap = std::map<CPDF_Dictionary*, CPDF_CountedFont*>;
-  using CPDF_IccProfileMap = std::map<CPDF_Stream*, CPDF_CountedIccProfile*>;
+  using CPDF_ColorSpaceMap =
+      std::map<const CPDF_Object*, CPDF_CountedColorSpace*>;
+  using CPDF_FontFileMap = std::map<const CPDF_Stream*, CPDF_CountedStreamAcc*>;
+  using CPDF_FontMap = std::map<const CPDF_Dictionary*, CPDF_CountedFont*>;
+  using CPDF_IccProfileMap =
+      std::map<const CPDF_Stream*, CPDF_CountedIccProfile*>;
   using CPDF_ImageMap = std::map<uint32_t, CPDF_CountedImage*>;
-  using CPDF_PatternMap = std::map<CPDF_Object*, CPDF_CountedPattern*>;
+  using CPDF_PatternMap = std::map<const CPDF_Object*, CPDF_CountedPattern*>;
 
   CPDF_ColorSpace* GetColorSpaceImpl(CPDF_Object* pCSObj,
                                      const CPDF_Dictionary* pResources,