Refcount CPDF_Image all the time.chromium/3061chromium/3060

Remove the old externally-counted CPDF_CountedImage type.

Change-Id: Ia0b288586272da3f2daf7dfc153f08e62794321a
Reviewed-on: https://pdfium-review.googlesource.com/3553
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

12 files changed, 73 insertions, 89 deletions

diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
index f7c4630063..14e4ed32c5 100644
--- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
+++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp
@@ -114,7 +114,7 @@ void CPDF_PageContentGenerator::ProcessImage(CFX_ByteTextBuf* buf,
   }
   *buf << "q " << pImageObj->matrix() << " cm ";
 
-  CPDF_Image* pImage = pImageObj->GetImage();
+  CFX_RetainPtr<CPDF_Image> pImage = pImageObj->GetImage();
   if (pImage->IsInline())
     return;
 
@@ -129,7 +129,7 @@ void CPDF_PageContentGenerator::ProcessImage(CFX_ByteTextBuf* buf,
   uint32_t dwObjNum = pStream->GetObjNum();
   CFX_ByteString name = RealizeResource(dwObjNum, "XObject");
   if (bWasInline)
-    pImageObj->SetUnownedImage(m_pDocument->GetPageData()->GetImage(dwObjNum));
+    pImageObj->SetImage(m_pDocument->GetPageData()->GetImage(dwObjNum));
 
   *buf << "/" << PDF_NameEncode(name) << " Do Q\n";
 }
diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp
index af2618286e..5789a531f0 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.cpp
+++ b/core/fpdfapi/page/cpdf_docpagedata.cpp
@@ -113,18 +113,7 @@ void CPDF_DocPageData::Clear(bool bForceRelease) {
     }
   }
 
-  for (auto it = m_ImageMap.begin(); it != m_ImageMap.end();) {
-    auto curr_it = it++;
-    CPDF_CountedImage* pCountedImage = curr_it->second;
-    if (!pCountedImage->get())
-      continue;
-
-    if (bForceRelease || pCountedImage->use_count() < 2) {
-      delete pCountedImage->get();
-      delete pCountedImage;
-      m_ImageMap.erase(curr_it);
-    }
-  }
+  m_ImageMap.clear();
 }
 
 CPDF_Font* CPDF_DocPageData::GetFont(CPDF_Dictionary* pFontDict) {
@@ -398,36 +387,22 @@ void CPDF_DocPageData::ReleasePattern(const CPDF_Object* pPatternObj) {
   pPattern->clear();
 }
 
-CPDF_Image* CPDF_DocPageData::GetImage(uint32_t dwStreamObjNum) {
+CFX_RetainPtr<CPDF_Image> CPDF_DocPageData::GetImage(uint32_t dwStreamObjNum) {
   ASSERT(dwStreamObjNum);
   auto it = m_ImageMap.find(dwStreamObjNum);
   if (it != m_ImageMap.end())
-    return it->second->AddRef();
+    return it->second;
 
-  CPDF_CountedImage* pCountedImage = new CPDF_CountedImage(
-      pdfium::MakeUnique<CPDF_Image>(m_pPDFDoc, dwStreamObjNum));
-  m_ImageMap[dwStreamObjNum] = pCountedImage;
-  return pCountedImage->AddRef();
+  auto pImage = pdfium::MakeRetain<CPDF_Image>(m_pPDFDoc, dwStreamObjNum);
+  m_ImageMap[dwStreamObjNum] = pImage;
+  return pImage;
 }
 
-void CPDF_DocPageData::ReleaseImage(uint32_t dwStreamObjNum) {
+void CPDF_DocPageData::MaybePurgeImage(uint32_t dwStreamObjNum) {
   ASSERT(dwStreamObjNum);
   auto it = m_ImageMap.find(dwStreamObjNum);
-  if (it == m_ImageMap.end())
-    return;
-
-  CPDF_CountedImage* pCountedImage = it->second;
-  if (!pCountedImage)
-    return;
-
-  pCountedImage->RemoveRef();
-  if (pCountedImage->use_count() > 1)
-    return;
-
-  // We have item only in m_ImageMap cache. Clean it.
-  delete pCountedImage->get();
-  delete pCountedImage;
-  m_ImageMap.erase(it);
+  if (it != m_ImageMap.end() && it->second->HasOneRef())
+    m_ImageMap.erase(it);
 }
 
 CPDF_IccProfile* CPDF_DocPageData::GetIccProfile(
diff --git a/core/fpdfapi/page/cpdf_docpagedata.h b/core/fpdfapi/page/cpdf_docpagedata.h
index 37538eb329..6ce6985e09 100644
--- a/core/fpdfapi/page/cpdf_docpagedata.h
+++ b/core/fpdfapi/page/cpdf_docpagedata.h
@@ -30,32 +30,38 @@ class CPDF_DocPageData {
   ~CPDF_DocPageData();
 
   void Clear(bool bRelease = false);
+  bool IsForceClear() const { return m_bForceClear; }
+
   CPDF_Font* GetFont(CPDF_Dictionary* pFontDict);
   CPDF_Font* GetStandardFont(const CFX_ByteString& fontName,
                              CPDF_FontEncoding* pEncoding);
   void ReleaseFont(const CPDF_Dictionary* pFontDict);
+
   CPDF_ColorSpace* GetColorSpace(CPDF_Object* pCSObj,
                                  const CPDF_Dictionary* pResources);
   CPDF_ColorSpace* GetCopiedColorSpace(CPDF_Object* pCSObj);
   void ReleaseColorSpace(const CPDF_Object* pColorSpace);
+
   CPDF_Pattern* GetPattern(CPDF_Object* pPatternObj,
                            bool bShading,
                            const CFX_Matrix& matrix);
   void ReleasePattern(const CPDF_Object* pPatternObj);
-  CPDF_Image* GetImage(uint32_t dwStreamObjNum);
-  void ReleaseImage(uint32_t dwStreamObjNum);
+
+  CFX_RetainPtr<CPDF_Image> GetImage(uint32_t dwStreamObjNum);
+  void MaybePurgeImage(uint32_t dwStreamObjNum);
+
   CPDF_IccProfile* GetIccProfile(CPDF_Stream* pIccProfileStream);
   void ReleaseIccProfile(const CPDF_IccProfile* pIccProfile);
+
   CPDF_StreamAcc* GetFontFileStreamAcc(CPDF_Stream* pFontStream);
   void ReleaseFontFileStreamAcc(const CPDF_Stream* pFontStream);
-  bool IsForceClear() const { return m_bForceClear; }
+
   CPDF_CountedColorSpace* FindColorSpacePtr(CPDF_Object* pCSObj) const;
   CPDF_CountedPattern* FindPatternPtr(CPDF_Object* pPatternObj) const;
 
  private:
   using CPDF_CountedFont = CPDF_CountedObject<CPDF_Font>;
   using CPDF_CountedIccProfile = CPDF_CountedObject<CPDF_IccProfile>;
-  using CPDF_CountedImage = CPDF_CountedObject<CPDF_Image>;
   using CPDF_CountedStreamAcc = CPDF_CountedObject<CPDF_StreamAcc>;
 
   using CPDF_ColorSpaceMap =
@@ -64,7 +70,7 @@ class CPDF_DocPageData {
   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_ImageMap = std::map<uint32_t, CFX_RetainPtr<CPDF_Image>>;
   using CPDF_PatternMap = std::map<const CPDF_Object*, CPDF_CountedPattern*>;
 
   CPDF_ColorSpace* GetColorSpaceImpl(CPDF_Object* pCSObj,
diff --git a/core/fpdfapi/page/cpdf_image.h b/core/fpdfapi/page/cpdf_image.h
index 5b737e67b9..7b7e090b49 100644
--- a/core/fpdfapi/page/cpdf_image.h
+++ b/core/fpdfapi/page/cpdf_image.h
@@ -11,6 +11,7 @@
 
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fxcrt/cfx_maybe_owned.h"
+#include "core/fxcrt/cfx_retain_ptr.h"
 #include "core/fxcrt/fx_system.h"
 
 class CFX_DIBSource;
@@ -20,12 +21,10 @@ class CPDF_Page;
 class IFX_Pause;
 class IFX_SeekableReadStream;
 
-class CPDF_Image {
+class CPDF_Image : public CFX_Retainable {
  public:
-  explicit CPDF_Image(CPDF_Document* pDoc);
-  CPDF_Image(CPDF_Document* pDoc, std::unique_ptr<CPDF_Stream> pStream);
-  CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum);
-  ~CPDF_Image();
+  template <typename T, typename... Args>
+  friend CFX_RetainPtr<T> pdfium::MakeRetain(Args&&... args);
 
   void ConvertStreamToIndirectObject();
 
@@ -66,6 +65,11 @@ class CPDF_Image {
   uint32_t m_MatteColor = 0;
 
  private:
+  explicit CPDF_Image(CPDF_Document* pDoc);
+  CPDF_Image(CPDF_Document* pDoc, std::unique_ptr<CPDF_Stream> pStream);
+  CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum);
+  ~CPDF_Image() override;
+
   void FinishInitialization();
   std::unique_ptr<CPDF_Dictionary> InitJPEG(uint8_t* pData, uint32_t size);
 
diff --git a/core/fpdfapi/page/cpdf_imageobject.cpp b/core/fpdfapi/page/cpdf_imageobject.cpp
index 01d2df7421..1c2cc6ba7e 100644
--- a/core/fpdfapi/page/cpdf_imageobject.cpp
+++ b/core/fpdfapi/page/cpdf_imageobject.cpp
@@ -12,11 +12,10 @@
 #include "core/fpdfapi/page/cpdf_image.h"
 #include "core/fpdfapi/parser/cpdf_document.h"
 
-CPDF_ImageObject::CPDF_ImageObject()
-    : m_pImage(nullptr), m_pImageOwned(false) {}
+CPDF_ImageObject::CPDF_ImageObject() {}
 
 CPDF_ImageObject::~CPDF_ImageObject() {
-  Release();
+  MaybePurgeCache();
 }
 
 CPDF_PageObject::Type CPDF_ImageObject::GetType() const {
@@ -48,30 +47,31 @@ void CPDF_ImageObject::CalcBoundingBox() {
   m_Matrix.TransformRect(m_Left, m_Right, m_Top, m_Bottom);
 }
 
-void CPDF_ImageObject::SetOwnedImage(std::unique_ptr<CPDF_Image> pImage) {
-  Release();
-  m_pImage = pImage.release();
-  m_pImageOwned = true;
-}
-
-void CPDF_ImageObject::SetUnownedImage(CPDF_Image* pImage) {
-  Release();
+void CPDF_ImageObject::SetImage(const CFX_RetainPtr<CPDF_Image>& pImage) {
+  MaybePurgeCache();
   m_pImage = pImage;
-  m_pImageOwned = false;
 }
 
-void CPDF_ImageObject::Release() {
-  if (m_pImageOwned) {
-    delete m_pImage;
-    m_pImage = nullptr;
-    m_pImageOwned = false;
+void CPDF_ImageObject::MaybePurgeCache() {
+  if (!m_pImage)
     return;
-  }
 
-  if (!m_pImage)
+  CPDF_Document* pDocument = m_pImage->GetDocument();
+  if (!pDocument)
+    return;
+
+  CPDF_DocPageData* pPageData = pDocument->GetPageData();
+  if (pPageData)
+    return;
+
+  CPDF_Stream* pStream = m_pImage->GetStream();
+  if (!pStream)
+    return;
+
+  uint32_t objnum = pStream->GetObjNum();
+  if (!objnum)
     return;
 
-  CPDF_DocPageData* pPageData = m_pImage->GetDocument()->GetPageData();
-  pPageData->ReleaseImage(m_pImage->GetStream()->GetObjNum());
-  m_pImage = nullptr;
+  m_pImage.Reset();  // Clear my reference before asking the cache.
+  pPageData->MaybePurgeImage(objnum);
 }
diff --git a/core/fpdfapi/page/cpdf_imageobject.h b/core/fpdfapi/page/cpdf_imageobject.h
index 1bbfc0509c..484fb7baa6 100644
--- a/core/fpdfapi/page/cpdf_imageobject.h
+++ b/core/fpdfapi/page/cpdf_imageobject.h
@@ -27,19 +27,16 @@ class CPDF_ImageObject : public CPDF_PageObject {
   const CPDF_ImageObject* AsImage() const override;
 
   void CalcBoundingBox();
-  CPDF_Image* GetImage() const { return m_pImage; }
-  void SetOwnedImage(std::unique_ptr<CPDF_Image> pImage);
-  void SetUnownedImage(CPDF_Image* pImage);
-
+  CFX_RetainPtr<CPDF_Image> GetImage() const { return m_pImage; }
+  void SetImage(const CFX_RetainPtr<CPDF_Image>& pImage);
   void set_matrix(const CFX_Matrix& matrix) { m_Matrix = matrix; }
   const CFX_Matrix& matrix() const { return m_Matrix; }
 
  private:
-  void Release();
+  void MaybePurgeCache();
 
   CFX_Matrix m_Matrix;
-  CPDF_Image* m_pImage;
-  bool m_pImageOwned;
+  CFX_RetainPtr<CPDF_Image> m_pImage;
 };
 
 #endif  // CORE_FPDFAPI_PAGE_CPDF_IMAGEOBJECT_H_
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 4c64e13b06..59de3c50cb 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -791,23 +791,24 @@ CPDF_ImageObject* CPDF_StreamContentParser::AddImage(
     return nullptr;
 
   auto pImageObj = pdfium::MakeUnique<CPDF_ImageObject>();
-  pImageObj->SetOwnedImage(
-      pdfium::MakeUnique<CPDF_Image>(m_pDocument, std::move(pStream)));
+  pImageObj->SetImage(
+      pdfium::MakeRetain<CPDF_Image>(m_pDocument, std::move(pStream)));
   return AddImageObject(std::move(pImageObj));
 }
 
 CPDF_ImageObject* CPDF_StreamContentParser::AddImage(uint32_t streamObjNum) {
   auto pImageObj = pdfium::MakeUnique<CPDF_ImageObject>();
-  pImageObj->SetUnownedImage(m_pDocument->LoadImageFromPageData(streamObjNum));
+  pImageObj->SetImage(m_pDocument->LoadImageFromPageData(streamObjNum));
   return AddImageObject(std::move(pImageObj));
 }
 
-CPDF_ImageObject* CPDF_StreamContentParser::AddImage(CPDF_Image* pImage) {
+CPDF_ImageObject* CPDF_StreamContentParser::AddImage(
+    const CFX_RetainPtr<CPDF_Image>& pImage) {
   if (!pImage)
     return nullptr;
 
   auto pImageObj = pdfium::MakeUnique<CPDF_ImageObject>();
-  pImageObj->SetUnownedImage(
+  pImageObj->SetImage(
       m_pDocument->GetPageData()->GetImage(pImage->GetStream()->GetObjNum()));
 
   return AddImageObject(std::move(pImageObj));
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h
index 2eac3c22c6..4440aabb43 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.h
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.h
@@ -99,7 +99,7 @@ class CPDF_StreamContentParser {
   void AddPathObject(int FillType, bool bStroke);
   CPDF_ImageObject* AddImage(std::unique_ptr<CPDF_Stream> pStream);
   CPDF_ImageObject* AddImage(uint32_t streamObjNum);
-  CPDF_ImageObject* AddImage(CPDF_Image* pImage);
+  CPDF_ImageObject* AddImage(const CFX_RetainPtr<CPDF_Image>& pImage);
 
   void AddForm(CPDF_Stream* pStream);
   void SetGraphicStates(CPDF_PageObject* pObj,
@@ -211,7 +211,7 @@ class CPDF_StreamContentParser {
   float m_PathCurrentY;
   uint8_t m_PathClipType;
   CFX_ByteString m_LastImageName;
-  CPDF_Image* m_pLastImage;
+  CFX_RetainPtr<CPDF_Image> m_pLastImage;
   bool m_bColored;
   float m_Type3Data[6];
   bool m_bResourceMissing;
diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp
index dec0f5097e..fb2c723ce4 100644
--- a/core/fpdfapi/parser/cpdf_document.cpp
+++ b/core/fpdfapi/parser/cpdf_document.cpp
@@ -649,7 +649,8 @@ CPDF_IccProfile* CPDF_Document::LoadIccProfile(CPDF_Stream* pStream) {
   return m_pDocPage->GetIccProfile(pStream);
 }
 
-CPDF_Image* CPDF_Document::LoadImageFromPageData(uint32_t dwStreamObjNum) {
+CFX_RetainPtr<CPDF_Image> CPDF_Document::LoadImageFromPageData(
+    uint32_t dwStreamObjNum) {
   ASSERT(dwStreamObjNum);
   return m_pDocPage->GetImage(dwStreamObjNum);
 }
diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index 3524231c48..6f09e2230c 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -13,6 +13,7 @@
 #include <utility>
 #include <vector>
 
+#include "core/fpdfapi/page/cpdf_image.h"
 #include "core/fpdfapi/parser/cpdf_indirect_object_holder.h"
 #include "core/fpdfapi/parser/cpdf_object.h"
 #include "core/fpdfdoc/cpdf_linklist.h"
@@ -26,7 +27,6 @@ class CPDF_DocRenderData;
 class CPDF_Font;
 class CPDF_FontEncoding;
 class CPDF_IccProfile;
-class CPDF_Image;
 class CPDF_LinearizedHeader;
 class CPDF_Parser;
 class CPDF_Pattern;
@@ -78,7 +78,7 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
                             bool bShading,
                             const CFX_Matrix& matrix);
 
-  CPDF_Image* LoadImageFromPageData(uint32_t dwStreamObjNum);
+  CFX_RetainPtr<CPDF_Image> LoadImageFromPageData(uint32_t dwStreamObjNum);
   CPDF_StreamAcc* LoadFontFile(CPDF_Stream* pStream);
   CPDF_IccProfile* LoadIccProfile(CPDF_Stream* pStream);
 
diff --git a/core/fpdfapi/render/cpdf_imageloader.cpp b/core/fpdfapi/render/cpdf_imageloader.cpp
index 856e340325..230dc02fe5 100644
--- a/core/fpdfapi/render/cpdf_imageloader.cpp
+++ b/core/fpdfapi/render/cpdf_imageloader.cpp
@@ -68,7 +68,7 @@ void CPDF_ImageLoader::HandleFailure() {
     m_MatteColor = entry->m_MatteColor;
     return;
   }
-  CPDF_Image* pImage = m_pImage->GetImage();
+  CFX_RetainPtr<CPDF_Image> pImage = m_pImage->GetImage();
   m_bCached = false;
   m_pBitmap = pImage->DetachBitmap();
   m_pMask = pImage->DetachMask();
diff --git a/fpdfsdk/fpdfeditimg.cpp b/fpdfsdk/fpdfeditimg.cpp
index ca78088793..4738bd1325 100644
--- a/fpdfsdk/fpdfeditimg.cpp
+++ b/fpdfsdk/fpdfeditimg.cpp
@@ -48,9 +48,9 @@ FPDFPageObj_NewImageObj(FPDF_DOCUMENT document) {
   if (!pDoc)
     return nullptr;
 
-  CPDF_ImageObject* pImageObj = new CPDF_ImageObject;
-  pImageObj->SetOwnedImage(pdfium::MakeUnique<CPDF_Image>(pDoc));
-  return pImageObj;
+  auto pImageObj = pdfium::MakeUnique<CPDF_ImageObject>();
+  pImageObj->SetImage(pdfium::MakeRetain<CPDF_Image>(pDoc));
+  return pImageObj.release();
 }
 
 DLLEXPORT FPDF_PAGEOBJECT STDCALL