From 917d24852841f4919f418076c0277d2742f961ca Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 22 Sep 2016 18:46:45 -0700 Subject: Make CPDF_ColorSpace::Load() return a unique_ptr Specialize default_delete to avoid having to say ReleaseDeleter throughout the code. Review-Url: https://codereview.chromium.org/2368433003 --- core/fpdfapi/fpdf_page/cpdf_colorspace.cpp | 113 ++++++++++------------- core/fpdfapi/fpdf_page/fpdf_page_doc.cpp | 11 ++- core/fpdfapi/fpdf_page/include/cpdf_colorspace.h | 21 ++++- 3 files changed, 73 insertions(+), 72 deletions(-) (limited to 'core/fpdfapi') diff --git a/core/fpdfapi/fpdf_page/cpdf_colorspace.cpp b/core/fpdfapi/fpdf_page/cpdf_colorspace.cpp index 88a74f0189..87607648b6 100644 --- a/core/fpdfapi/fpdf_page/cpdf_colorspace.cpp +++ b/core/fpdfapi/fpdf_page/cpdf_colorspace.cpp @@ -221,7 +221,7 @@ class CPDF_SeparationCS : public CPDF_ColorSpace { FX_FLOAT& B) const override; void EnableStdConversion(FX_BOOL bEnabled) override; - CPDF_ColorSpace* m_pAltCS; + std::unique_ptr m_pAltCS; std::unique_ptr m_pFunc; enum { None, All, Colorant } m_Type; }; @@ -243,7 +243,7 @@ class CPDF_DeviceNCS : public CPDF_ColorSpace { FX_FLOAT& B) const override; void EnableStdConversion(FX_BOOL bEnabled) override; - CPDF_ColorSpace* m_pAltCS; + std::unique_ptr m_pAltCS; std::unique_ptr m_pFunc; }; @@ -313,18 +313,14 @@ void XYZ_to_sRGB_WhitePoint(FX_FLOAT X, CPDF_ColorSpace* CPDF_ColorSpace::ColorspaceFromName( const CFX_ByteString& name) { - if (name == "DeviceRGB" || name == "RGB") { + if (name == "DeviceRGB" || name == "RGB") return CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB); - } - if (name == "DeviceGray" || name == "G") { + if (name == "DeviceGray" || name == "G") return CPDF_ColorSpace::GetStockCS(PDFCS_DEVICEGRAY); - } - if (name == "DeviceCMYK" || name == "CMYK") { + if (name == "DeviceCMYK" || name == "CMYK") return CPDF_ColorSpace::GetStockCS(PDFCS_DEVICECMYK); - } - if (name == "Pattern") { + if (name == "Pattern") return CPDF_ColorSpace::GetStockCS(PDFCS_PATTERN); - } return nullptr; } @@ -332,81 +328,77 @@ CPDF_ColorSpace* CPDF_ColorSpace::GetStockCS(int family) { return CPDF_ModuleMgr::Get()->GetPageModule()->GetStockCS(family); } -CPDF_ColorSpace* CPDF_ColorSpace::Load(CPDF_Document* pDoc, CPDF_Object* pObj) { +std::unique_ptr CPDF_ColorSpace::Load(CPDF_Document* pDoc, + CPDF_Object* pObj) { if (!pObj) - return nullptr; - if (pObj->IsName()) - return ColorspaceFromName(pObj->GetString()); + return std::unique_ptr(); + if (pObj->IsName()) { + return std::unique_ptr( + ColorspaceFromName(pObj->GetString())); + } if (CPDF_Stream* pStream = pObj->AsStream()) { CPDF_Dictionary* pDict = pStream->GetDict(); if (!pDict) - return nullptr; + return std::unique_ptr(); for (const auto& it : *pDict) { - CPDF_ColorSpace* pRet = nullptr; + std::unique_ptr pRet; CPDF_Object* pValue = it.second; if (ToName(pValue)) - pRet = ColorspaceFromName(pValue->GetString()); + pRet.reset(ColorspaceFromName(pValue->GetString())); if (pRet) return pRet; } - return nullptr; + return std::unique_ptr(); } CPDF_Array* pArray = pObj->AsArray(); if (!pArray || pArray->IsEmpty()) - return nullptr; + return std::unique_ptr(); CPDF_Object* pFamilyObj = pArray->GetDirectObjectAt(0); if (!pFamilyObj) - return nullptr; + return std::unique_ptr(); CFX_ByteString familyname = pFamilyObj->GetString(); if (pArray->GetCount() == 1) - return ColorspaceFromName(familyname); + return std::unique_ptr(ColorspaceFromName(familyname)); - CPDF_ColorSpace* pCS = nullptr; + std::unique_ptr pCS; uint32_t id = familyname.GetID(); if (id == FXBSTR_ID('C', 'a', 'l', 'G')) { - pCS = new CPDF_CalGray(pDoc); + pCS.reset(new CPDF_CalGray(pDoc)); } else if (id == FXBSTR_ID('C', 'a', 'l', 'R')) { - pCS = new CPDF_CalRGB(pDoc); + pCS.reset(new CPDF_CalRGB(pDoc)); } else if (id == FXBSTR_ID('L', 'a', 'b', 0)) { - pCS = new CPDF_LabCS(pDoc); + pCS.reset(new CPDF_LabCS(pDoc)); } else if (id == FXBSTR_ID('I', 'C', 'C', 'B')) { - pCS = new CPDF_ICCBasedCS(pDoc); + pCS.reset(new CPDF_ICCBasedCS(pDoc)); } else if (id == FXBSTR_ID('I', 'n', 'd', 'e') || id == FXBSTR_ID('I', 0, 0, 0)) { - pCS = new CPDF_IndexedCS(pDoc); + pCS.reset(new CPDF_IndexedCS(pDoc)); } else if (id == FXBSTR_ID('S', 'e', 'p', 'a')) { - pCS = new CPDF_SeparationCS(pDoc); + pCS.reset(new CPDF_SeparationCS(pDoc)); } else if (id == FXBSTR_ID('D', 'e', 'v', 'i')) { - pCS = new CPDF_DeviceNCS(pDoc); + pCS.reset(new CPDF_DeviceNCS(pDoc)); } else if (id == FXBSTR_ID('P', 'a', 't', 't')) { - pCS = new CPDF_PatternCS(pDoc); + pCS.reset(new CPDF_PatternCS(pDoc)); } else { - return nullptr; + return std::unique_ptr(); } pCS->m_pArray = pArray; - if (!pCS->v_Load(pDoc, pArray)) { - pCS->ReleaseCS(); - return nullptr; - } + if (!pCS->v_Load(pDoc, pArray)) + return std::unique_ptr(); + return pCS; } -void CPDF_ColorSpace::ReleaseCS() { - if (this == GetStockCS(PDFCS_DEVICERGB)) { - return; - } - if (this == GetStockCS(PDFCS_DEVICEGRAY)) { - return; - } - if (this == GetStockCS(PDFCS_DEVICECMYK)) { - return; - } - if (this == GetStockCS(PDFCS_PATTERN)) { +void CPDF_ColorSpace::Release() { + if (this == GetStockCS(PDFCS_DEVICERGB) || + this == GetStockCS(PDFCS_DEVICEGRAY) || + this == GetStockCS(PDFCS_DEVICECMYK) || + this == GetStockCS(PDFCS_PATTERN)) { return; } delete this; @@ -832,7 +824,7 @@ CPDF_ICCBasedCS::~CPDF_ICCBasedCS() { FX_Free(m_pCache); FX_Free(m_pRanges); if (m_pAlterCS && m_bOwn) - m_pAlterCS->ReleaseCS(); + m_pAlterCS->Release(); if (m_pProfile && m_pDocument) m_pDocument->GetPageData()->ReleaseIccProfile(m_pProfile); } @@ -854,15 +846,15 @@ FX_BOOL CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) { CPDF_Object* pAlterCSObj = pDict ? pDict->GetDirectObjectFor("Alternate") : nullptr; if (pAlterCSObj) { - CPDF_ColorSpace* pAlterCS = CPDF_ColorSpace::Load(pDoc, pAlterCSObj); + std::unique_ptr pAlterCS = + CPDF_ColorSpace::Load(pDoc, pAlterCSObj); if (pAlterCS) { if (m_nComponents == 0) { // NO valid ICC profile if (pAlterCS->CountComponents() > 0) { // Use Alternative colorspace m_nComponents = pAlterCS->CountComponents(); - m_pAlterCS = pAlterCS; + m_pAlterCS = pAlterCS.release(); m_bOwn = TRUE; } else { // No valid alternative colorspace - pAlterCS->ReleaseCS(); int32_t nDictComponents = pDict ? pDict->GetIntegerFor("N") : 0; if (nDictComponents != 1 && nDictComponents != 3 && nDictComponents != 4) { @@ -870,12 +862,9 @@ FX_BOOL CPDF_ICCBasedCS::v_Load(CPDF_Document* pDoc, CPDF_Array* pArray) { } m_nComponents = nDictComponents; } - } else { // Using sRGB - if (pAlterCS->CountComponents() != m_nComponents) { - pAlterCS->ReleaseCS(); - } else { - m_pAlterCS = pAlterCS; + if (pAlterCS->CountComponents() == m_nComponents) { + m_pAlterCS = pAlterCS.release(); m_bOwn = TRUE; } } @@ -1155,12 +1144,9 @@ CPDF_ColorSpace* CPDF_PatternCS::GetBaseCS() const { } CPDF_SeparationCS::CPDF_SeparationCS(CPDF_Document* pDoc) - : CPDF_ColorSpace(pDoc, PDFCS_SEPARATION, 1), m_pAltCS(nullptr) {} + : CPDF_ColorSpace(pDoc, PDFCS_SEPARATION, 1) {} -CPDF_SeparationCS::~CPDF_SeparationCS() { - if (m_pAltCS) - m_pAltCS->ReleaseCS(); -} +CPDF_SeparationCS::~CPDF_SeparationCS() {} void CPDF_SeparationCS::GetDefaultValue(int iComponent, FX_FLOAT& value, @@ -1236,12 +1222,9 @@ void CPDF_SeparationCS::EnableStdConversion(FX_BOOL bEnabled) { } CPDF_DeviceNCS::CPDF_DeviceNCS(CPDF_Document* pDoc) - : CPDF_ColorSpace(pDoc, PDFCS_DEVICEN, 0), m_pAltCS(nullptr) {} + : CPDF_ColorSpace(pDoc, PDFCS_DEVICEN, 0) {} -CPDF_DeviceNCS::~CPDF_DeviceNCS() { - if (m_pAltCS) - m_pAltCS->ReleaseCS(); -} +CPDF_DeviceNCS::~CPDF_DeviceNCS() {} void CPDF_DeviceNCS::GetDefaultValue(int iComponent, FX_FLOAT& value, diff --git a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp index e050060a9e..77c18df5fa 100644 --- a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp +++ b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp @@ -76,7 +76,7 @@ void CPDF_DocPageData::Clear(FX_BOOL bForceRelease) { continue; if (bForceRelease || csData->use_count() < 2) { - csData->get()->ReleaseCS(); + csData->get()->Release(); csData->reset(nullptr); } } @@ -294,15 +294,16 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpaceImpl( } } - CPDF_ColorSpace* pCS = CPDF_ColorSpace::Load(m_pPDFDoc, pArray); + std::unique_ptr pCS = + CPDF_ColorSpace::Load(m_pPDFDoc, pArray); if (!pCS) return nullptr; if (!csData) { - csData = new CPDF_CountedColorSpace(pCS); + csData = new CPDF_CountedColorSpace(pCS.release()); m_ColorSpaceMap[pCSObj] = csData; } else { - csData->reset(pCS); + csData->reset(pCS.release()); } return csData->AddRef(); } @@ -335,7 +336,7 @@ void CPDF_DocPageData::ReleaseColorSpace(const CPDF_Object* pColorSpace) { return; // We have item only in m_ColorSpaceMap cache. Clean it. - pCountedColorSpace->get()->ReleaseCS(); + pCountedColorSpace->get()->Release(); pCountedColorSpace->reset(nullptr); } diff --git a/core/fpdfapi/fpdf_page/include/cpdf_colorspace.h b/core/fpdfapi/fpdf_page/include/cpdf_colorspace.h index fc0433f52f..2166f9b3a8 100644 --- a/core/fpdfapi/fpdf_page/include/cpdf_colorspace.h +++ b/core/fpdfapi/fpdf_page/include/cpdf_colorspace.h @@ -7,6 +7,8 @@ #ifndef CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_COLORSPACE_H_ #define CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_COLORSPACE_H_ +#include + #include "core/fxcrt/include/cfx_weak_ptr.h" #include "core/fxcrt/include/fx_string.h" #include "core/fxcrt/include/fx_system.h" @@ -30,10 +32,11 @@ class CPDF_Object; class CPDF_ColorSpace { public: static CPDF_ColorSpace* GetStockCS(int Family); - static CPDF_ColorSpace* Load(CPDF_Document* pDoc, CPDF_Object* pCSObj); static CPDF_ColorSpace* ColorspaceFromName(const CFX_ByteString& name); + static std::unique_ptr Load(CPDF_Document* pDoc, + CPDF_Object* pCSObj); - void ReleaseCS(); + void Release(); int GetBufSize() const; FX_FLOAT* CreateBuf(); @@ -104,4 +107,18 @@ class CPDF_ColorSpace { using CPDF_CountedColorSpace = CFX_WeakPtr::Handle; +namespace std { + +// Make std::unique_ptr call Release() rather than +// simply deleting the object. +template <> +struct default_delete { + void operator()(CPDF_ColorSpace* pColorSpace) const { + if (pColorSpace) + pColorSpace->Release(); + } +}; + +} // namespace std + #endif // CORE_FPDFAPI_FPDF_PAGE_INCLUDE_CPDF_COLORSPACE_H_ -- cgit v1.2.3