diff options
author | dsinclair <dsinclair@chromium.org> | 2016-11-04 08:25:34 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-11-04 08:25:34 -0700 |
commit | f0d5b6c35fa343108a3ab7a25bc2cc2b3cf105b3 (patch) | |
tree | edc3d5f35225971679e581c8ef951de8275a944b | |
parent | 4de3d095c9d9e961f93750cf1ebd489fd515be12 (diff) | |
download | pdfium-f0d5b6c35fa343108a3ab7a25bc2cc2b3cf105b3.tar.xz |
Revert of Remove CPDF_Object::Release() in favor of direct delete (patchset #11 id:200001 of https://codereview.chromium.org/2384883003/ )
Reason for revert:
Looks like it's blocking the roll.
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_ng/builds/186619
Original issue's description:
> Remove CPDF_Object::Release() in favor of direct delete
>
> Follow-on once we prove Release always deletes in previous CL.
>
> Committed: https://pdfium.googlesource.com/pdfium/+/4de3d095c9d9e961f93750cf1ebd489fd515be12
TBR=thestig@chromium.org,tsepez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review-Url: https://codereview.chromium.org/2478253002
36 files changed, 274 insertions, 199 deletions
diff --git a/core/fpdfapi/edit/cpdf_creator.h b/core/fpdfapi/edit/cpdf_creator.h index c04db6eb37..f462115cf7 100644 --- a/core/fpdfapi/edit/cpdf_creator.h +++ b/core/fpdfapi/edit/cpdf_creator.h @@ -94,7 +94,7 @@ class CPDF_Creator { FX_FILESIZE m_XrefStart; CFX_FileSizeListArray m_ObjectOffset; CFX_ArrayTemplate<uint32_t> m_NewObjNumArray; - std::unique_ptr<CPDF_Array> m_pIDArray; + std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>> m_pIDArray; int32_t m_FileVersion; }; diff --git a/core/fpdfapi/edit/fpdf_edit_create.cpp b/core/fpdfapi/edit/fpdf_edit_create.cpp index a578a0a114..ed638d4c40 100644 --- a/core/fpdfapi/edit/fpdf_edit_create.cpp +++ b/core/fpdfapi/edit/fpdf_edit_create.cpp @@ -470,8 +470,8 @@ CPDF_FlateEncoder::CPDF_FlateEncoder(const uint8_t* pBuffer, } CPDF_FlateEncoder::~CPDF_FlateEncoder() { - if (m_bCloned) - delete m_pDict; + if (m_bCloned && m_pDict) + m_pDict->Release(); if (m_bNewData) FX_Free(m_pData); } @@ -902,8 +902,8 @@ CPDF_Creator::CPDF_Creator(CPDF_Document* pDoc) CPDF_Creator::~CPDF_Creator() { ResetStandardSecurity(); - if (m_bEncryptCloned) { - delete m_pEncryptDict; + if (m_bEncryptCloned && m_pEncryptDict) { + m_pEncryptDict->Release(); m_pEncryptDict = nullptr; } Clear(); @@ -1247,7 +1247,7 @@ int32_t CPDF_Creator::WriteOldIndirectObject(uint32_t objnum) { return -1; } if (!bExistInMap) { - m_pDocument->DeleteIndirectObject(objnum); + m_pDocument->ReleaseIndirectObject(objnum); } } else { uint8_t* pBuffer; @@ -1934,7 +1934,7 @@ void CPDF_Creator::InitID(bool bDefault) { m_pIDArray->Add(pID1->Clone()); } else { std::vector<uint8_t> buffer = - PDF_GenerateFileID((uint32_t)(uintptr_t) this, m_dwLastObjNum); + PDF_GenerateFileID((uint32_t)(uintptr_t)this, m_dwLastObjNum); CFX_ByteString bsBuffer(buffer.data(), buffer.size()); m_pIDArray->Add(new CPDF_String(bsBuffer, true)); } @@ -1949,7 +1949,7 @@ void CPDF_Creator::InitID(bool bDefault) { return; } std::vector<uint8_t> buffer = - PDF_GenerateFileID((uint32_t)(uintptr_t) this, m_dwLastObjNum); + PDF_GenerateFileID((uint32_t)(uintptr_t)this, m_dwLastObjNum); CFX_ByteString bsBuffer(buffer.data(), buffer.size()); m_pIDArray->Add(new CPDF_String(bsBuffer, true)); return; diff --git a/core/fpdfapi/font/fpdf_font.cpp b/core/fpdfapi/font/fpdf_font.cpp index b681edf70b..c827ea5793 100644 --- a/core/fpdfapi/font/fpdf_font.cpp +++ b/core/fpdfapi/font/fpdf_font.cpp @@ -47,8 +47,11 @@ CFX_StockFontArray::CFX_StockFontArray() {} CFX_StockFontArray::~CFX_StockFontArray() { for (size_t i = 0; i < FX_ArraySize(m_StockFonts); ++i) { - if (m_StockFonts[i]) - delete m_StockFonts[i]->GetFontDict(); + if (!m_StockFonts[i]) + continue; + CPDF_Dictionary* pFontDict = m_StockFonts[i]->GetFontDict(); + if (pFontDict) + pFontDict->Release(); } } diff --git a/core/fpdfapi/page/cpdf_contentmark.cpp b/core/fpdfapi/page/cpdf_contentmark.cpp index a867409335..d60e144d6e 100644 --- a/core/fpdfapi/page/cpdf_contentmark.cpp +++ b/core/fpdfapi/page/cpdf_contentmark.cpp @@ -110,7 +110,8 @@ void CPDF_ContentMark::MarkData::AddMark(const CFX_ByteString& name, if (pDict) { if (bDirect) { item.SetDirectDict( - std::unique_ptr<CPDF_Dictionary>(ToDictionary(pDict->Clone()))); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>( + ToDictionary(pDict->Clone()))); } else { item.SetPropertiesDict(pDict); } diff --git a/core/fpdfapi/page/cpdf_contentmarkitem.cpp b/core/fpdfapi/page/cpdf_contentmarkitem.cpp index dffeada707..597f8a595c 100644 --- a/core/fpdfapi/page/cpdf_contentmarkitem.cpp +++ b/core/fpdfapi/page/cpdf_contentmarkitem.cpp @@ -39,7 +39,7 @@ bool CPDF_ContentMarkItem::HasMCID() const { } void CPDF_ContentMarkItem::SetDirectDict( - std::unique_ptr<CPDF_Dictionary> pDict) { + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict) { m_ParamType = DirectDict; m_pDirectDict = std::move(pDict); } diff --git a/core/fpdfapi/page/cpdf_contentmarkitem.h b/core/fpdfapi/page/cpdf_contentmarkitem.h index ed2737111b..f1f06c3a38 100644 --- a/core/fpdfapi/page/cpdf_contentmarkitem.h +++ b/core/fpdfapi/page/cpdf_contentmarkitem.h @@ -31,14 +31,16 @@ class CPDF_ContentMarkItem { bool HasMCID() const; void SetName(const CFX_ByteString& name) { m_MarkName = name; } - void SetDirectDict(std::unique_ptr<CPDF_Dictionary> pDict); + void SetDirectDict( + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict); void SetPropertiesDict(CPDF_Dictionary* pDict); private: CFX_ByteString m_MarkName; ParamType m_ParamType; CPDF_Dictionary* m_pPropertiesDict; // not owned. - std::unique_ptr<CPDF_Dictionary> m_pDirectDict; + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> + m_pDirectDict; }; #endif // CORE_FPDFAPI_PAGE_CPDF_CONTENTMARKITEM_H_ diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp index 976d6d8c47..23c6e4f786 100644 --- a/core/fpdfapi/page/cpdf_image.cpp +++ b/core/fpdfapi/page/cpdf_image.cpp @@ -25,16 +25,14 @@ CPDF_Image::CPDF_Image(CPDF_Document* pDoc) : m_pDocument(pDoc) {} -CPDF_Image::CPDF_Image(CPDF_Document* pDoc, - std::unique_ptr<CPDF_Stream> pStream) +CPDF_Image::CPDF_Image(CPDF_Document* pDoc, UniqueStream pStream) : m_pDocument(pDoc), m_pStream(pStream.get()), m_pOwnedStream(std::move(pStream)) { if (!m_pStream) return; - m_pOwnedDict = - ToDictionary(std::unique_ptr<CPDF_Object>(m_pStream->GetDict()->Clone())); + m_pOwnedDict = ToDictionary(UniqueObject(m_pStream->GetDict()->Clone())); m_pDict = m_pOwnedDict.get(); FinishInitialization(); } @@ -63,15 +61,13 @@ void CPDF_Image::FinishInitialization() { CPDF_Image* CPDF_Image::Clone() { CPDF_Image* pImage = new CPDF_Image(m_pDocument); if (m_pOwnedStream) { - pImage->m_pOwnedStream = - ToStream(std::unique_ptr<CPDF_Object>(m_pOwnedStream->Clone())); + pImage->m_pOwnedStream = ToStream(UniqueObject(m_pOwnedStream->Clone())); pImage->m_pStream = pImage->m_pOwnedStream.get(); } else { pImage->m_pStream = m_pStream; } if (m_pOwnedDict) { - pImage->m_pOwnedDict = - ToDictionary(std::unique_ptr<CPDF_Object>(m_pOwnedDict->Clone())); + pImage->m_pOwnedDict = ToDictionary(UniqueObject(m_pOwnedDict->Clone())); pImage->m_pDict = pImage->m_pOwnedDict.get(); } else { pImage->m_pDict = m_pDict; @@ -293,8 +289,10 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) { pNewBitmap->Copy(pBitmap); pNewBitmap->ConvertFormat(FXDIB_Rgb); SetImage(pNewBitmap, iCompress); - delete pDict; - pDict = nullptr; + if (pDict) { + pDict->Release(); + pDict = nullptr; + } FX_Free(dest_buf); dest_buf = nullptr; dest_size = 0; diff --git a/core/fpdfapi/page/cpdf_image.h b/core/fpdfapi/page/cpdf_image.h index 02308db647..f619845597 100644 --- a/core/fpdfapi/page/cpdf_image.h +++ b/core/fpdfapi/page/cpdf_image.h @@ -28,7 +28,7 @@ class IFX_SeekableWriteStream; class CPDF_Image { public: explicit CPDF_Image(CPDF_Document* pDoc); - CPDF_Image(CPDF_Document* pDoc, std::unique_ptr<CPDF_Stream> pStream); + CPDF_Image(CPDF_Document* pDoc, UniqueStream pStream); CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum); ~CPDF_Image(); @@ -84,8 +84,8 @@ class CPDF_Image { CPDF_Document* const m_pDocument; CPDF_Stream* m_pStream = nullptr; CPDF_Dictionary* m_pDict = nullptr; - std::unique_ptr<CPDF_Stream> m_pOwnedStream; - std::unique_ptr<CPDF_Dictionary> m_pOwnedDict; + UniqueStream m_pOwnedStream; + UniqueDictionary m_pOwnedDict; CPDF_Dictionary* m_pOC = nullptr; }; diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 7618f8271f..ed6701382c 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -180,8 +180,12 @@ CPDF_StreamContentParser::CPDF_StreamContentParser( CPDF_StreamContentParser::~CPDF_StreamContentParser() { ClearAllParams(); FX_Free(m_pPathPoints); - delete m_pLastImageDict; - delete m_pLastCloneImageDict; + if (m_pLastImageDict) { + m_pLastImageDict->Release(); + } + if (m_pLastCloneImageDict) { + m_pLastCloneImageDict->Release(); + } } int CPDF_StreamContentParser::GetNextParamPos() { @@ -190,9 +194,10 @@ int CPDF_StreamContentParser::GetNextParamPos() { if (m_ParamStartPos == kParamBufSize) { m_ParamStartPos = 0; } - if (m_ParamBuf[m_ParamStartPos].m_Type == 0) - delete m_ParamBuf[m_ParamStartPos].m_pObject; - + if (m_ParamBuf[m_ParamStartPos].m_Type == 0) { + if (CPDF_Object* pObject = m_ParamBuf[m_ParamStartPos].m_pObject) + pObject->Release(); + } return m_ParamStartPos; } int index = m_ParamStartPos + m_ParamCount; @@ -239,9 +244,10 @@ void CPDF_StreamContentParser::AddObjectParam(CPDF_Object* pObj) { void CPDF_StreamContentParser::ClearAllParams() { uint32_t index = m_ParamStartPos; for (uint32_t i = 0; i < m_ParamCount; i++) { - if (m_ParamBuf[index].m_Type == 0) - delete m_ParamBuf[index].m_pObject; - + if (m_ParamBuf[index].m_Type == 0) { + if (CPDF_Object* pObject = m_ParamBuf[index].m_pObject) + pObject->Release(); + } index++; if (index == kParamBufSize) { index = 0; @@ -525,7 +531,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() { m_pSyntax->GetWordSize()); if (bsKeyword != "ID") { m_pSyntax->SetPos(savePos); - delete pDict; + pDict->Release(); return; } } @@ -534,7 +540,8 @@ void CPDF_StreamContentParser::Handle_BeginImage() { } CFX_ByteString key((const FX_CHAR*)m_pSyntax->GetWordBuf() + 1, m_pSyntax->GetWordSize() - 1); - std::unique_ptr<CPDF_Object> pObj(m_pSyntax->ReadNextObject(false, 0)); + std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pObj( + m_pSyntax->ReadNextObject(false, 0)); if (!key.IsEmpty()) { uint32_t dwObjNum = pObj ? pObj->GetObjNum() : 0; if (dwObjNum) @@ -559,8 +566,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() { } } pDict->SetNameFor("Subtype", "Image"); - std::unique_ptr<CPDF_Stream> pStream( - m_pSyntax->ReadInlineStream(m_pDocument, pDict, pCSObj)); + UniqueStream pStream(m_pSyntax->ReadInlineStream(m_pDocument, pDict, pCSObj)); bool bGaveDictAway = !!pStream; while (1) { CPDF_StreamParser::SyntaxType type = m_pSyntax->ParseNextElement(); @@ -577,7 +583,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() { } CPDF_ImageObject* pImgObj = AddImage(std::move(pStream)); if (!pImgObj && !bGaveDictAway) - delete pDict; + pDict->Release(); } void CPDF_StreamContentParser::Handle_BeginMarkedContent() { @@ -663,10 +669,10 @@ void CPDF_StreamContentParser::Handle_ExecuteXObject() { type = pXObject->GetDict()->GetStringFor("Subtype"); if (type == "Image") { - CPDF_ImageObject* pObj = pXObject->IsInline() - ? AddImage(std::unique_ptr<CPDF_Stream>( - ToStream(pXObject->Clone()))) - : AddImage(pXObject->GetObjNum()); + CPDF_ImageObject* pObj = + pXObject->IsInline() + ? AddImage(UniqueStream(ToStream(pXObject->Clone()))) + : AddImage(pXObject->GetObjNum()); m_LastImageName = name; m_pLastImage = pObj->GetImage(); @@ -698,8 +704,7 @@ void CPDF_StreamContentParser::AddForm(CPDF_Stream* pStream) { m_pObjectHolder->GetPageObjectList()->push_back(std::move(pFormObj)); } -CPDF_ImageObject* CPDF_StreamContentParser::AddImage( - std::unique_ptr<CPDF_Stream> pStream) { +CPDF_ImageObject* CPDF_StreamContentParser::AddImage(UniqueStream pStream) { if (!pStream) return nullptr; diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h index 1ed2aaa4de..58008da96a 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.h +++ b/core/fpdfapi/page/cpdf_streamcontentparser.h @@ -97,7 +97,7 @@ class CPDF_StreamContentParser { void AddPathPoint(FX_FLOAT x, FX_FLOAT y, int flag); void AddPathRect(FX_FLOAT x, FX_FLOAT y, FX_FLOAT w, FX_FLOAT h); void AddPathObject(int FillType, bool bStroke); - CPDF_ImageObject* AddImage(std::unique_ptr<CPDF_Stream> pStream); + CPDF_ImageObject* AddImage(UniqueStream pStream); CPDF_ImageObject* AddImage(uint32_t streamObjNum); CPDF_ImageObject* AddImage(CPDF_Image* pImage); diff --git a/core/fpdfapi/page/fpdf_page_parser_old.cpp b/core/fpdfapi/page/fpdf_page_parser_old.cpp index 51ffc11b03..0d1db43825 100644 --- a/core/fpdfapi/page/fpdf_page_parser_old.cpp +++ b/core/fpdfapi/page/fpdf_page_parser_old.cpp @@ -138,7 +138,9 @@ CPDF_StreamParser::CPDF_StreamParser( m_pPool(pPool) {} CPDF_StreamParser::~CPDF_StreamParser() { - delete m_pLastObj; + if (m_pLastObj) { + m_pLastObj->Release(); + } } CPDF_Stream* CPDF_StreamParser::ReadInlineStream(CPDF_Document* pDoc, @@ -250,8 +252,10 @@ CPDF_Stream* CPDF_StreamParser::ReadInlineStream(CPDF_Document* pDoc, } CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() { - delete m_pLastObj; - m_pLastObj = nullptr; + if (m_pLastObj) { + m_pLastObj->Release(); + m_pLastObj = nullptr; + } m_WordSize = 0; bool bIsNumber = true; @@ -370,7 +374,7 @@ CPDF_Object* CPDF_StreamParser::ReadNextObject(bool bAllowNestedArray, break; if (!m_WordSize || m_WordBuffer[0] != '/') { - delete pDict; + pDict->Release(); return nullptr; } @@ -378,12 +382,12 @@ CPDF_Object* CPDF_StreamParser::ReadNextObject(bool bAllowNestedArray, PDF_NameDecode(CFX_ByteStringC(m_WordBuffer + 1, m_WordSize - 1)); CPDF_Object* pObj = ReadNextObject(true, 0); if (!pObj) { - delete pDict; + pDict->Release(); return nullptr; } if (key.IsEmpty()) - delete pObj; + pObj->Release(); else pDict->SetFor(key, pObj); } diff --git a/core/fpdfapi/parser/cfdf_document.cpp b/core/fpdfapi/parser/cfdf_document.cpp index dcd4b3c109..d39ec31d3c 100644 --- a/core/fpdfapi/parser/cfdf_document.cpp +++ b/core/fpdfapi/parser/cfdf_document.cpp @@ -81,7 +81,7 @@ void CFDF_Document::ParseStream(IFX_SeekableReadStream* pFile, bool bOwnFile) { if (CPDF_Dictionary* pMainDict = ToDictionary(parser.GetObject(this, 0, 0, true))) { m_pRootDict = pMainDict->GetDictFor("Root"); - delete pMainDict; + pMainDict->Release(); } break; } diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index 4000bbc980..e118fd66e6 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp @@ -24,7 +24,7 @@ CPDF_Array::~CPDF_Array() { m_ObjNum = kInvalidObjNum; for (auto& it : m_Objects) { if (it && it->GetObjNum() != kInvalidObjNum) - delete it; + it->Release(); } } @@ -139,9 +139,10 @@ void CPDF_Array::RemoveAt(size_t i, size_t nCount) { if (nCount <= 0 || nCount > m_Objects.size() - i) return; - for (size_t j = 0; j < nCount; ++j) - delete m_Objects[i + j]; - + for (size_t j = 0; j < nCount; ++j) { + if (CPDF_Object* p = m_Objects[i + j]) + p->Release(); + } m_Objects.erase(m_Objects.begin() + i, m_Objects.begin() + i + nCount); } @@ -165,7 +166,9 @@ void CPDF_Array::SetAt(size_t i, CPDF_Object* pObj) { ASSERT(false); return; } - delete m_Objects[i]; + if (CPDF_Object* pOld = m_Objects[i]) + pOld->Release(); + m_Objects[i] = pObj; } diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 9deb478809..8cfa0333bb 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h @@ -21,7 +21,6 @@ class CPDF_Array : public CPDF_Object { using const_iterator = std::vector<CPDF_Object*>::const_iterator; CPDF_Array(); - ~CPDF_Array() override; // CPDF_Object. Type GetType() const override; @@ -62,12 +61,15 @@ class CPDF_Array : public CPDF_Object { const_iterator end() const { return m_Objects.end(); } protected: + ~CPDF_Array() override; + CPDF_Object* CloneNonCyclic( bool bDirect, std::set<const CPDF_Object*>* pVisited) const override; std::vector<CPDF_Object*> m_Objects; }; +using UniqueArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Object>>; inline CPDF_Array* ToArray(CPDF_Object* obj) { return obj ? obj->AsArray() : nullptr; @@ -77,12 +79,12 @@ inline const CPDF_Array* ToArray(const CPDF_Object* obj) { return obj ? obj->AsArray() : nullptr; } -inline std::unique_ptr<CPDF_Array> ToArray(std::unique_ptr<CPDF_Object> obj) { +inline UniqueArray ToArray(UniqueObject obj) { CPDF_Array* pArray = ToArray(obj.get()); if (!pArray) return nullptr; obj.release(); - return std::unique_ptr<CPDF_Array>(pArray); + return UniqueArray(pArray); } #endif // CORE_FPDFAPI_PARSER_CPDF_ARRAY_H_ diff --git a/core/fpdfapi/parser/cpdf_array_unittest.cpp b/core/fpdfapi/parser/cpdf_array_unittest.cpp index 800afb0f9a..bc9f578021 100644 --- a/core/fpdfapi/parser/cpdf_array_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_array_unittest.cpp @@ -10,10 +10,16 @@ #include "testing/gtest/include/gtest/gtest.h" +namespace { + +using ScopedArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>>; + +} // namespace + TEST(cpdf_array, RemoveAt) { { int elems[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->AddInteger(elems[i]); arr->RemoveAt(3, 3); @@ -30,7 +36,7 @@ TEST(cpdf_array, RemoveAt) { { // When the range is out of bound, RemoveAt has no effect. int elems[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->AddInteger(elems[i]); arr->RemoveAt(8, 5); @@ -47,7 +53,7 @@ TEST(cpdf_array, RemoveAt) { TEST(cpdf_array, InsertAt) { { int elems[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->InsertAt(i, new CPDF_Number(elems[i])); EXPECT_EQ(FX_ArraySize(elems), arr->GetCount()); @@ -66,7 +72,7 @@ TEST(cpdf_array, InsertAt) { // an element is inserted at that position while other unfilled // positions have nullptr. int elems[] = {1, 2}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->InsertAt(i, new CPDF_Number(elems[i])); arr->InsertAt(10, new CPDF_Number(10)); @@ -83,10 +89,10 @@ TEST(cpdf_array, Clone) { { // Basic case. int elems[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->InsertAt(i, new CPDF_Number(elems[i])); - std::unique_ptr<CPDF_Array> arr2(arr->Clone()->AsArray()); + ScopedArray arr2(arr->Clone()->AsArray()); EXPECT_EQ(arr->GetCount(), arr2->GetCount()); for (size_t i = 0; i < FX_ArraySize(elems); ++i) { // Clone() always create new objects. @@ -100,7 +106,7 @@ TEST(cpdf_array, Clone) { static const size_t kNumOfRowElems = 5; int elems[kNumOfRows][kNumOfRowElems] = { {1, 2, 3, 4, 5}, {10, 9, 8, 7, 6}, {11, 12, 13, 14, 15}}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); // Indirect references to indirect objects. std::unique_ptr<CPDF_IndirectObjectHolder> obj_holder( new CPDF_IndirectObjectHolder()); @@ -118,10 +124,10 @@ TEST(cpdf_array, Clone) { ASSERT_EQ(kNumOfRows, arr->GetCount()); // Not dereferencing reference objects means just creating new references // instead of new copies of direct objects. - std::unique_ptr<CPDF_Array> arr1(arr->Clone()->AsArray()); + ScopedArray arr1(arr->Clone()->AsArray()); EXPECT_EQ(arr->GetCount(), arr1->GetCount()); // Dereferencing reference objects creates new copies of direct objects. - std::unique_ptr<CPDF_Array> arr2(arr->CloneDirectObject()->AsArray()); + ScopedArray arr2(arr->CloneDirectObject()->AsArray()); EXPECT_EQ(arr->GetCount(), arr2->GetCount()); for (size_t i = 0; i < kNumOfRows; ++i) { CPDF_Array* arr_elem = arr->GetObjectAt(i)->AsArray(); @@ -165,7 +171,7 @@ TEST(cpdf_array, Clone) { TEST(cpdf_array, Iterator) { int elems[] = {-23, -11, 3, 455, 2345877, 0, 7895330, -12564334, 10000, -100000}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->InsertAt(i, new CPDF_Number(elems[i])); size_t index = 0; diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index eadbf1e828..a077ebb715 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -83,12 +83,17 @@ CPDF_DataAvail::CPDF_DataAvail(FileAvail* pFileAvail, CPDF_DataAvail::~CPDF_DataAvail() { m_pHintTables.reset(); - delete m_pLinearized; - delete m_pRoot; - delete m_pTrailer; + if (m_pLinearized) + m_pLinearized->Release(); + + if (m_pRoot) + m_pRoot->Release(); + + if (m_pTrailer) + m_pTrailer->Release(); for (CPDF_Object* pObject : m_arrayAcroforms) - delete pObject; + pObject->Release(); } void CPDF_DataAvail::SetDocument(CPDF_Document* pDoc) { @@ -225,7 +230,7 @@ bool CPDF_DataAvail::CheckAcroFormSubObject(DownloadHints* pHints) { } for (CPDF_Object* pObject : m_arrayAcroforms) - delete pObject; + pObject->Release(); m_arrayAcroforms.clear(); return true; @@ -395,7 +400,9 @@ bool CPDF_DataAvail::CheckInfo(DownloadHints* pHints) { return false; } - delete pInfo; + if (pInfo) + pInfo->Release(); + m_docStatus = (m_bHaveAcroForm ? PDF_DATAAVAIL_ACROFORM : PDF_DATAAVAIL_PAGETREE); @@ -492,7 +499,7 @@ bool CPDF_DataAvail::CheckPage(DownloadHints* pHints) { } if (!pObj->IsDictionary()) { - delete pObj; + pObj->Release(); continue; } @@ -501,7 +508,7 @@ bool CPDF_DataAvail::CheckPage(DownloadHints* pHints) { m_PagesArray.push_back(pObj); continue; } - delete pObj; + pObj->Release(); } m_PageObjList.RemoveAll(); @@ -517,15 +524,15 @@ bool CPDF_DataAvail::CheckPage(DownloadHints* pHints) { continue; if (!GetPageKids(m_pCurrentParser, pPages)) { - delete pPages; + pPages->Release(); while (++i < iPages) - delete m_PagesArray[i]; + m_PagesArray[i]->Release(); m_PagesArray.clear(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } - delete pPages; + pPages->Release(); } m_PagesArray.clear(); @@ -580,12 +587,12 @@ bool CPDF_DataAvail::CheckPages(DownloadHints* pHints) { } if (!GetPageKids(m_pCurrentParser, pPages)) { - delete pPages; + pPages->Release(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } - delete pPages; + pPages->Release(); m_docStatus = PDF_DATAAVAIL_PAGE; return true; } @@ -756,7 +763,7 @@ bool CPDF_DataAvail::CheckHintTables(DownloadHints* pHints) { std::unique_ptr<CPDF_HintTables> pHintTables( new CPDF_HintTables(this, pDict)); - std::unique_ptr<CPDF_Object> pHintStream( + std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pHintStream( ParseIndirectObjectAt(szHintStart, 0)); CPDF_Stream* pStream = ToStream(pHintStream.get()); if (pStream && pHintTables->LoadHintStream(pStream)) @@ -944,11 +951,11 @@ int32_t CPDF_DataAvail::CheckCrossRefStream(DownloadHints* pHints, if (pName->GetString() == "XRef") { m_Pos += m_parser.m_pSyntax->SavePos(); xref_offset = pObj->GetDict()->GetIntegerFor("Prev"); - delete pObj; + pObj->Release(); return 1; } } - delete pObj; + pObj->Release(); return -1; } pHints->AddSegment(m_Pos, req_size); @@ -1174,7 +1181,7 @@ bool CPDF_DataAvail::CheckTrailer(DownloadHints* pHints) { ScopedFileStream file(FX_CreateMemoryStream(pBuf, (size_t)iSize, false)); m_syntaxParser.InitParser(file.get(), 0); - std::unique_ptr<CPDF_Object> pTrailer( + std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pTrailer( m_syntaxParser.GetObject(nullptr, 0, 0, true)); if (!pTrailer) { m_Pos += m_syntaxParser.SavePos(); @@ -1259,7 +1266,7 @@ bool CPDF_DataAvail::CheckArrayPageNode(uint32_t dwPageNo, CPDF_Array* pArray = pPages->AsArray(); if (!pArray) { - delete pPages; + pPages->Release(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } @@ -1274,7 +1281,7 @@ bool CPDF_DataAvail::CheckArrayPageNode(uint32_t dwPageNo, pPageNode->m_childNode.Add(pNode); pNode->m_dwPageNo = pKid->GetRefObjNum(); } - delete pPages; + pPages->Release(); return true; } @@ -1297,12 +1304,12 @@ bool CPDF_DataAvail::CheckUnkownPageNode(uint32_t dwPageNo, if (pPage->IsArray()) { pPageNode->m_dwPageNo = dwPageNo; pPageNode->m_type = PDF_PAGENODE_ARRAY; - delete pPage; + pPage->Release(); return true; } if (!pPage->IsDictionary()) { - delete pPage; + pPage->Release(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } @@ -1343,11 +1350,11 @@ bool CPDF_DataAvail::CheckUnkownPageNode(uint32_t dwPageNo, } else if (type == "Page") { pPageNode->m_type = PDF_PAGENODE_PAGE; } else { - delete pPage; + pPage->Release(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } - delete pPage; + pPage->Release(); return true; } @@ -1435,23 +1442,23 @@ bool CPDF_DataAvail::CheckPageCount(DownloadHints* pHints) { CPDF_Dictionary* pPagesDict = pPages->GetDict(); if (!pPagesDict) { - delete pPages; + pPages->Release(); m_docStatus = PDF_DATAAVAIL_ERROR; return false; } if (!pPagesDict->KeyExist("Kids")) { - delete pPages; + pPages->Release(); return true; } int count = pPagesDict->GetIntegerFor("Count"); if (count > 0) { - delete pPages; + pPages->Release(); return true; } - delete pPages; + pPages->Release(); return false; } diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp index 37efbbc34a..75cb1e859c 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/parser/cpdf_dictionary.cpp @@ -31,7 +31,7 @@ CPDF_Dictionary::~CPDF_Dictionary() { m_ObjNum = kInvalidObjNum; for (const auto& it : m_Map) { if (it.second && it.second->GetObjNum() != kInvalidObjNum) - delete it.second; + it.second->Release(); } } @@ -184,7 +184,7 @@ void CPDF_Dictionary::SetFor(const CFX_ByteString& key, CPDF_Object* pObj) { if (it->second == pObj) return; - delete it->second; + it->second->Release(); if (pObj) it->second = pObj; @@ -208,7 +208,7 @@ void CPDF_Dictionary::RemoveFor(const CFX_ByteString& key) { if (it == m_Map.end()) return; - delete it->second; + it->second->Release(); m_Map.erase(it); } @@ -223,7 +223,7 @@ void CPDF_Dictionary::ReplaceKey(const CFX_ByteString& oldkey, return; if (new_it != m_Map.end()) { - delete new_it->second; + new_it->second->Release(); new_it->second = old_it->second; } else { m_Map.insert(std::make_pair(MaybeIntern(newkey), old_it->second)); diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index 4ef2f96ce7..fb8200f78c 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h @@ -26,7 +26,6 @@ class CPDF_Dictionary : public CPDF_Object { CPDF_Dictionary(); explicit CPDF_Dictionary(const CFX_WeakPtr<CFX_ByteStringPool>& pPool); - ~CPDF_Dictionary() override; // CPDF_Object. Type GetType() const override; @@ -89,6 +88,8 @@ class CPDF_Dictionary : public CPDF_Object { CFX_WeakPtr<CFX_ByteStringPool> GetByteStringPool() const { return m_pPool; } protected: + ~CPDF_Dictionary() override; + CFX_ByteString MaybeIntern(const CFX_ByteString& str); CPDF_Object* CloneNonCyclic( bool bDirect, @@ -98,6 +99,9 @@ class CPDF_Dictionary : public CPDF_Object { std::map<CFX_ByteString, CPDF_Object*> m_Map; }; +using UniqueDictionary = + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Object>>; + inline CPDF_Dictionary* ToDictionary(CPDF_Object* obj) { return obj ? obj->AsDictionary() : nullptr; } @@ -106,13 +110,12 @@ inline const CPDF_Dictionary* ToDictionary(const CPDF_Object* obj) { return obj ? obj->AsDictionary() : nullptr; } -inline std::unique_ptr<CPDF_Dictionary> ToDictionary( - std::unique_ptr<CPDF_Object> obj) { +inline UniqueDictionary ToDictionary(UniqueObject obj) { CPDF_Dictionary* pDict = ToDictionary(obj.get()); if (!pDict) return nullptr; obj.release(); - return std::unique_ptr<CPDF_Dictionary>(pDict); + return UniqueDictionary(pDict); } #endif // CORE_FPDFAPI_PARSER_CPDF_DICTIONARY_H_ diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp index 64574047e5..ad84a15a97 100644 --- a/core/fpdfapi/parser/cpdf_document.cpp +++ b/core/fpdfapi/parser/cpdf_document.cpp @@ -634,7 +634,7 @@ CPDF_Dictionary* CPDF_Document::CreateNewPage(int iPage) { pDict->SetNameFor("Type", "Page"); uint32_t dwObjNum = AddIndirectObject(pDict); if (!InsertNewPage(iPage, pDict)) { - DeleteIndirectObject(dwObjNum); + ReleaseIndirectObject(dwObjNum); return nullptr; } return pDict; diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp index c09665b716..9336626f45 100644 --- a/core/fpdfapi/parser/cpdf_document_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp @@ -76,7 +76,8 @@ class CPDF_TestDocumentForPages : public CPDF_Document { } private: - std::unique_ptr<CPDF_Dictionary> m_pOwnedRootDict; + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> + m_pOwnedRootDict; }; } // namespace @@ -120,7 +121,7 @@ TEST_F(cpdf_document_test, UseCachedPageObjNumIfHaveNotPagesDict) { // can be not exists in this case. // (case, when hint table is used to page check in CPDF_DataAvail). CPDF_Document document(pdfium::MakeUnique<CPDF_Parser>()); - std::unique_ptr<CPDF_Dictionary> dict(new CPDF_Dictionary()); + ScopedDictionary dict(new CPDF_Dictionary()); const int page_count = 100; dict->SetIntegerFor("N", page_count); document.LoadLinearizedDoc(dict.get()); diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp index 427fac66e4..6e549de5a7 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp @@ -70,7 +70,7 @@ bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration( return true; } -void CPDF_IndirectObjectHolder::DeleteIndirectObject(uint32_t objnum) { +void CPDF_IndirectObjectHolder::ReleaseIndirectObject(uint32_t objnum) { CPDF_Object* pObj = GetIndirectObject(objnum); if (!pObj || pObj->GetObjNum() == CPDF_Object::kInvalidObjNum) return; diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h index 83a31535c4..da4e942b5d 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h @@ -24,7 +24,7 @@ class CPDF_IndirectObjectHolder { CPDF_Object* GetIndirectObject(uint32_t objnum) const; CPDF_Object* GetOrParseIndirectObject(uint32_t objnum); - void DeleteIndirectObject(uint32_t objnum); + void ReleaseIndirectObject(uint32_t objnum); // Take ownership of |pObj|. uint32_t AddIndirectObject(CPDF_Object* pObj); diff --git a/core/fpdfapi/parser/cpdf_object.cpp b/core/fpdfapi/parser/cpdf_object.cpp index e9c215ce19..cc410d10c8 100644 --- a/core/fpdfapi/parser/cpdf_object.cpp +++ b/core/fpdfapi/parser/cpdf_object.cpp @@ -37,6 +37,11 @@ CPDF_Object* CPDF_Object::CloneNonCyclic( return Clone(); } +void CPDF_Object::Release() { + CHECK(!m_ObjNum); + delete this; +} + CFX_ByteString CPDF_Object::GetString() const { return CFX_ByteString(); } diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h index 8f6491ec72..c888605d72 100644 --- a/core/fpdfapi/parser/cpdf_object.h +++ b/core/fpdfapi/parser/cpdf_object.h @@ -38,8 +38,6 @@ class CPDF_Object { REFERENCE }; - virtual ~CPDF_Object(); - virtual Type GetType() const = 0; uint32_t GetObjNum() const { return m_ObjNum; } uint32_t GetGenNum() const { return m_GenNum; } @@ -52,6 +50,8 @@ class CPDF_Object { virtual CPDF_Object* CloneDirectObject() const; virtual CPDF_Object* GetDirect() const; + void Release(); + virtual CFX_ByteString GetString() const; virtual CFX_WideString GetUnicodeText() const; virtual FX_FLOAT GetNumber() const; @@ -94,8 +94,10 @@ class CPDF_Object { friend class CPDF_Parser; friend class CPDF_Reference; friend class CPDF_Stream; + friend struct std::default_delete<CPDF_Object>; CPDF_Object() : m_ObjNum(0), m_GenNum(0) {} + virtual ~CPDF_Object(); CPDF_Object* CloneObjectNonCyclic(bool bDirect) const; @@ -116,4 +118,6 @@ class CPDF_Object { CPDF_Object(const CPDF_Object& src) {} }; +using UniqueObject = std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>>; + #endif // CORE_FPDFAPI_PARSER_CPDF_OBJECT_H_ diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp index 64dc8c63ec..8215226ef3 100644 --- a/core/fpdfapi/parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp @@ -22,6 +22,11 @@ namespace { +using ScopedArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>>; +using ScopedDict = + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>; +using ScopedStream = std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Stream>>; + void TestArrayAccessors(const CPDF_Array* arr, size_t index, const char* str_val, @@ -168,12 +173,14 @@ class PDFObjectsTest : public testing::Test { } protected: + using ScopedObj = std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>>; + // m_ObjHolder needs to be declared first and destructed last since it also // refers to some objects in m_DirectObjs. std::unique_ptr<CPDF_IndirectObjectHolder> m_ObjHolder; - std::vector<std::unique_ptr<CPDF_Object>> m_DirectObjs; + std::vector<ScopedObj> m_DirectObjs; std::vector<int> m_DirectObjTypes; - std::vector<std::unique_ptr<CPDF_Object>> m_RefObjs; + std::vector<ScopedObj> m_RefObjs; CPDF_Dictionary* m_DictObj; CPDF_Dictionary* m_StreamDictObj; CPDF_Array* m_ArrayObj; @@ -268,13 +275,13 @@ TEST_F(PDFObjectsTest, GetArray) { TEST_F(PDFObjectsTest, Clone) { // Check for direct objects. for (size_t i = 0; i < m_DirectObjs.size(); ++i) { - std::unique_ptr<CPDF_Object> obj(m_DirectObjs[i]->Clone()); + ScopedObj obj(m_DirectObjs[i]->Clone()); EXPECT_TRUE(Equal(m_DirectObjs[i].get(), obj.get())); } // Check indirect references. for (const auto& it : m_RefObjs) { - std::unique_ptr<CPDF_Object> obj(it->Clone()); + ScopedObj obj(it->Clone()); EXPECT_TRUE(Equal(it.get(), obj.get())); } } @@ -386,7 +393,7 @@ TEST(PDFArrayTest, GetMatrix) { {2.3f, 4.05f, 3, -2, -3, 0.0f}, {0.05f, 0.1f, 0.56f, 0.67f, 1.34f, 99.9f}}; for (size_t i = 0; i < FX_ArraySize(elems); ++i) { - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); CFX_Matrix matrix(elems[i][0], elems[i][1], elems[i][2], elems[i][3], elems[i][4], elems[i][5]); for (size_t j = 0; j < 6; ++j) @@ -407,7 +414,7 @@ TEST(PDFArrayTest, GetRect) { {2.3f, 4.05f, -3, 0.0f}, {0.05f, 0.1f, 1.34f, 99.9f}}; for (size_t i = 0; i < FX_ArraySize(elems); ++i) { - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); CFX_FloatRect rect(elems[i]); for (size_t j = 0; j < 4; ++j) arr->AddNumber(elems[i][j]); @@ -423,7 +430,7 @@ TEST(PDFArrayTest, GetTypeAt) { { // Boolean array. const bool vals[] = {true, false, false, true, true}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) arr->InsertAt(i, new CPDF_Boolean(vals[i])); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { @@ -440,7 +447,7 @@ TEST(PDFArrayTest, GetTypeAt) { { // Integer array. const int vals[] = {10, 0, -345, 2089345456, -1000000000, 567, 93658767}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) arr->InsertAt(i, new CPDF_Number(vals[i])); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { @@ -461,7 +468,7 @@ TEST(PDFArrayTest, GetTypeAt) { 897.34f, -2.5f, -1.0f, -345.0f, -0.0f}; const char* const expected_str[] = { "0", "0", "10", "10", "0.0345", "897.34", "-2.5", "-1", "-345", "0"}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { arr->InsertAt(i, new CPDF_Number(vals[i])); } @@ -480,8 +487,8 @@ TEST(PDFArrayTest, GetTypeAt) { // String and name array const char* const vals[] = {"this", "adsde$%^", "\r\t", "\"012", ".", "EYREW", "It is a joke :)"}; - std::unique_ptr<CPDF_Array> string_array(new CPDF_Array); - std::unique_ptr<CPDF_Array> name_array(new CPDF_Array); + ScopedArray string_array(new CPDF_Array); + ScopedArray name_array(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { string_array->InsertAt(i, new CPDF_String(vals[i], false)); name_array->InsertAt(i, new CPDF_Name(vals[i])); @@ -507,7 +514,7 @@ TEST(PDFArrayTest, GetTypeAt) { } { // Null element array. - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < 3; ++i) arr->InsertAt(i, new CPDF_Null); for (size_t i = 0; i < 3; ++i) { @@ -524,7 +531,7 @@ TEST(PDFArrayTest, GetTypeAt) { { // Array of array. CPDF_Array* vals[3]; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < 3; ++i) { vals[i] = new CPDF_Array; for (size_t j = 0; j < 3; ++j) { @@ -547,7 +554,7 @@ TEST(PDFArrayTest, GetTypeAt) { { // Dictionary array. CPDF_Dictionary* vals[3]; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < 3; ++i) { vals[i] = new CPDF_Dictionary(); for (size_t j = 0; j < 3; ++j) { @@ -574,7 +581,7 @@ TEST(PDFArrayTest, GetTypeAt) { // Stream array. CPDF_Dictionary* vals[3]; CPDF_Stream* stream_vals[3]; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < 3; ++i) { vals[i] = new CPDF_Dictionary(); for (size_t j = 0; j < 3; ++j) { @@ -604,7 +611,7 @@ TEST(PDFArrayTest, GetTypeAt) { } { // Mixed array. - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); // Array arr will take ownership of all the objects inserted. arr->InsertAt(0, new CPDF_Boolean(true)); arr->InsertAt(1, new CPDF_Boolean(false)); @@ -669,7 +676,7 @@ TEST(PDFArrayTest, GetTypeAt) { TEST(PDFArrayTest, AddNumber) { float vals[] = {1.0f, -1.0f, 0, 0.456734f, 12345.54321f, 0.5f, 1000, 0.000045f}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) arr->AddNumber(vals[i]); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { @@ -680,7 +687,7 @@ TEST(PDFArrayTest, AddNumber) { TEST(PDFArrayTest, AddInteger) { int vals[] = {0, 1, 934435456, 876, 10000, -1, -24354656, -100}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); + ScopedArray arr(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) arr->AddInteger(vals[i]); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { @@ -692,8 +699,8 @@ TEST(PDFArrayTest, AddInteger) { TEST(PDFArrayTest, AddStringAndName) { const char* vals[] = {"", "a", "ehjhRIOYTTFdfcdnv", "122323", "$#%^&**", " ", "This is a test.\r\n"}; - std::unique_ptr<CPDF_Array> string_array(new CPDF_Array); - std::unique_ptr<CPDF_Array> name_array(new CPDF_Array); + ScopedArray string_array(new CPDF_Array); + ScopedArray name_array(new CPDF_Array); for (size_t i = 0; i < FX_ArraySize(vals); ++i) { string_array->AddString(vals[i]); name_array->AddName(vals[i]); @@ -718,8 +725,8 @@ TEST(PDFArrayTest, AddReferenceAndGetObjectAt) { CPDF_Object* indirect_objs[] = {boolean_obj, int_obj, float_obj, str_obj, name_obj, null_obj}; unsigned int obj_nums[] = {2, 4, 7, 2345, 799887, 1}; - std::unique_ptr<CPDF_Array> arr(new CPDF_Array); - std::unique_ptr<CPDF_Array> arr1(new CPDF_Array); + ScopedArray arr(new CPDF_Array); + ScopedArray arr1(new CPDF_Array); // Create two arrays of references by different AddReference() APIs. for (size_t i = 0; i < FX_ArraySize(indirect_objs); ++i) { // All the indirect objects inserted will be owned by holder. @@ -745,7 +752,7 @@ TEST(PDFArrayTest, AddReferenceAndGetObjectAt) { TEST(PDFArrayTest, CloneDirectObject) { CPDF_IndirectObjectHolder objects_holder; - std::unique_ptr<CPDF_Array> array(new CPDF_Array); + ScopedArray array(new CPDF_Array); array->AddReference(&objects_holder, 1234); ASSERT_EQ(1U, array->GetCount()); CPDF_Object* obj = array->GetObjectAt(0); @@ -756,7 +763,7 @@ TEST(PDFArrayTest, CloneDirectObject) { ASSERT_TRUE(cloned_array_object); ASSERT_TRUE(cloned_array_object->IsArray()); - std::unique_ptr<CPDF_Array> cloned_array(cloned_array_object->AsArray()); + ScopedArray cloned_array(cloned_array_object->AsArray()); ASSERT_EQ(1U, cloned_array->GetCount()); CPDF_Object* cloned_obj = cloned_array->GetObjectAt(0); EXPECT_FALSE(cloned_obj); @@ -764,7 +771,7 @@ TEST(PDFArrayTest, CloneDirectObject) { TEST(PDFArrayTest, ConvertIndirect) { CPDF_IndirectObjectHolder objects_holder; - std::unique_ptr<CPDF_Array> array(new CPDF_Array); + ScopedArray array(new CPDF_Array); CPDF_Object* pObj = new CPDF_Number(42); array->Add(pObj); array->ConvertToIndirectObjectAt(0, &objects_holder); @@ -779,7 +786,7 @@ TEST(PDFArrayTest, ConvertIndirect) { TEST(PDFDictionaryTest, CloneDirectObject) { CPDF_IndirectObjectHolder objects_holder; - std::unique_ptr<CPDF_Dictionary> dict(new CPDF_Dictionary()); + ScopedDict dict(new CPDF_Dictionary()); dict->SetReferenceFor("foo", &objects_holder, 1234); ASSERT_EQ(1U, dict->GetCount()); CPDF_Object* obj = dict->GetObjectFor("foo"); @@ -790,8 +797,7 @@ TEST(PDFDictionaryTest, CloneDirectObject) { ASSERT_TRUE(cloned_dict_object); ASSERT_TRUE(cloned_dict_object->IsDictionary()); - std::unique_ptr<CPDF_Dictionary> cloned_dict( - cloned_dict_object->AsDictionary()); + ScopedDict cloned_dict(cloned_dict_object->AsDictionary()); ASSERT_EQ(1U, cloned_dict->GetCount()); CPDF_Object* cloned_obj = cloned_dict->GetObjectFor("foo"); EXPECT_FALSE(cloned_obj); @@ -801,12 +807,12 @@ TEST(PDFObjectTest, CloneCheckLoop) { { // Create a dictionary/array pair with a reference loop. CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); - std::unique_ptr<CPDF_Array> arr_obj(new CPDF_Array); + ScopedArray arr_obj(new CPDF_Array); dict_obj->SetFor("arr", arr_obj.get()); arr_obj->InsertAt(0, dict_obj); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr<CPDF_Array> cloned_array(arr_obj->Clone()->AsArray()); + ScopedArray cloned_array(arr_obj->Clone()->AsArray()); // Cloned object should be the same as the original. ASSERT_TRUE(cloned_array); EXPECT_EQ(1u, cloned_array->GetCount()); @@ -819,12 +825,11 @@ TEST(PDFObjectTest, CloneCheckLoop) { { // Create a dictionary/stream pair with a reference loop. CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); - std::unique_ptr<CPDF_Stream> stream_obj( - new CPDF_Stream(nullptr, 0, dict_obj)); + ScopedStream stream_obj(new CPDF_Stream(nullptr, 0, dict_obj)); dict_obj->SetFor("stream", stream_obj.get()); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr<CPDF_Stream> cloned_stream(stream_obj->Clone()->AsStream()); + ScopedStream cloned_stream(stream_obj->Clone()->AsStream()); // Cloned object should be the same as the original. ASSERT_TRUE(cloned_stream); CPDF_Object* cloned_dict = cloned_stream->GetDict(); @@ -850,8 +855,7 @@ TEST(PDFObjectTest, CloneCheckLoop) { EXPECT_EQ(dict_obj, elem0->AsReference()->GetDirect()); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr<CPDF_Dictionary> cloned_dict( - ToDictionary(dict_obj->CloneDirectObject())); + ScopedDict cloned_dict(ToDictionary(dict_obj->CloneDirectObject())); // Cloned object should be the same as the original. ASSERT_TRUE(cloned_dict); CPDF_Object* cloned_arr = cloned_dict->GetObjectFor("arr"); @@ -865,7 +869,7 @@ TEST(PDFObjectTest, CloneCheckLoop) { TEST(PDFDictionaryTest, ConvertIndirect) { CPDF_IndirectObjectHolder objects_holder; - std::unique_ptr<CPDF_Dictionary> dict(new CPDF_Dictionary); + ScopedDict dict(new CPDF_Dictionary); CPDF_Object* pObj = new CPDF_Number(42); dict->SetFor("clams", pObj); dict->ConvertToIndirectObjectFor("clams", &objects_holder); diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index da95cc5f24..cff0f77b32 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp @@ -61,7 +61,9 @@ CPDF_Parser::CPDF_Parser() } CPDF_Parser::~CPDF_Parser() { - delete m_pTrailer; + if (m_pTrailer) + m_pTrailer->Release(); + ReleaseEncryptHandler(); SetEncryptDictionary(nullptr); @@ -70,10 +72,13 @@ CPDF_Parser::~CPDF_Parser() { m_pSyntax->m_pFileAccess = nullptr; } - for (CPDF_Dictionary* trailer : m_Trailers) - delete trailer; + for (CPDF_Dictionary* trailer : m_Trailers) { + if (trailer) + trailer->Release(); + } - delete m_pLinearized; + if (m_pLinearized) + m_pLinearized->Release(); } uint32_t CPDF_Parser::GetLastObjNum() const { @@ -352,7 +357,8 @@ bool CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { CrossRefList.insert(CrossRefList.begin(), xrefpos); LoadCrossRefV4(xrefpos, 0, true); - std::unique_ptr<CPDF_Dictionary> pDict(LoadTrailerV4()); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict( + LoadTrailerV4()); if (!pDict) return false; @@ -406,7 +412,8 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, CrossRefList.insert(CrossRefList.begin(), xrefpos); LoadCrossRefV4(xrefpos, 0, true); - std::unique_ptr<CPDF_Dictionary> pDict(LoadTrailerV4()); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict( + LoadTrailerV4()); if (!pDict) return false; @@ -583,14 +590,18 @@ bool CPDF_Parser::LoadAllCrossRefV5(FX_FILESIZE xrefpos) { bool CPDF_Parser::RebuildCrossRef() { m_ObjectInfo.clear(); m_SortedOffset.clear(); - delete m_pTrailer; - m_pTrailer = nullptr; + if (m_pTrailer) { + m_pTrailer->Release(); + m_pTrailer = nullptr; + } ParserState state = ParserState::kDefault; + int32_t inside_index = 0; uint32_t objnum = 0; uint32_t gennum = 0; int32_t depth = 0; + const uint32_t kBufferSize = 4096; std::vector<uint8_t> buffer(kBufferSize); @@ -744,7 +755,8 @@ bool CPDF_Parser::RebuildCrossRef() { CPDF_Object* pRoot = pDict->GetObjectFor("Root"); if (pRoot && pRoot->GetDict() && pRoot->GetDict()->GetObjectFor("Pages")) { - delete m_pTrailer; + if (m_pTrailer) + m_pTrailer->Release(); m_pTrailer = ToDictionary(pDict->Clone()); } } @@ -782,7 +794,8 @@ bool CPDF_Parser::RebuildCrossRef() { m_ObjectInfo[objnum].gennum = gennum; } - delete pObject; + if (pObject) + pObject->Release(); } --i; state = ParserState::kDefault; @@ -799,7 +812,7 @@ bool CPDF_Parser::RebuildCrossRef() { CPDF_Object* pObj = m_pSyntax->GetObject(m_pDocument, 0, 0, true); if (pObj) { if (!pObj->IsDictionary() && !pObj->AsStream()) { - delete pObj; + pObj->Release(); } else { CPDF_Stream* pStream = pObj->AsStream(); if (CPDF_Dictionary* pTrailer = @@ -825,11 +838,11 @@ bool CPDF_Parser::RebuildCrossRef() { } } } - delete pObj; + pObj->Release(); } else { if (pObj->IsStream()) { m_pTrailer = ToDictionary(pTrailer->Clone()); - delete pObj; + pObj->Release(); } else { m_pTrailer = pTrailer; } @@ -846,7 +859,7 @@ bool CPDF_Parser::RebuildCrossRef() { m_pSyntax->RestorePos(dwSavePos); } } else { - delete pObj; + pObj->Release(); } } } @@ -1405,7 +1418,7 @@ CPDF_Dictionary* CPDF_Parser::LoadTrailerV4() { if (m_pSyntax->GetKeyword() != "trailer") return nullptr; - std::unique_ptr<CPDF_Object> pObj( + std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pObj( m_pSyntax->GetObject(m_pDocument, 0, 0, true)); if (!ToDictionary(pObj.get())) return nullptr; @@ -1457,7 +1470,7 @@ bool CPDF_Parser::IsLinearizedFile(IFX_SeekableReadStream* pFileAccess, CPDF_Object* pLen = pDict->GetObjectFor("L"); if (!pLen) { - delete m_pLinearized; + m_pLinearized->Release(); m_pLinearized = nullptr; return false; } @@ -1473,7 +1486,7 @@ bool CPDF_Parser::IsLinearizedFile(IFX_SeekableReadStream* pFileAccess, return true; } - delete m_pLinearized; + m_pLinearized->Release(); m_pLinearized = nullptr; return false; } @@ -1582,11 +1595,12 @@ bool CPDF_Parser::LoadLinearizedAllCrossRefV5(FX_FILESIZE xrefpos) { CPDF_Parser::Error CPDF_Parser::LoadLinearizedMainXRefTable() { uint32_t dwSaveMetadataObjnum = m_pSyntax->m_MetadataObjnum; m_pSyntax->m_MetadataObjnum = 0; + if (m_pTrailer) { + m_pTrailer->Release(); + m_pTrailer = nullptr; + } - delete m_pTrailer; - m_pTrailer = nullptr; m_pSyntax->RestorePos(m_LastXRefOffset - m_pSyntax->m_HeaderOffset); - uint8_t ch = 0; uint32_t dwCount = 0; m_pSyntax->GetNextChar(ch); diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp index 11ef1d2dc1..c6e99c84b5 100644 --- a/core/fpdfapi/parser/cpdf_stream.cpp +++ b/core/fpdfapi/parser/cpdf_stream.cpp @@ -15,7 +15,7 @@ CPDF_Stream::CPDF_Stream() {} CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict) - : m_dwSize(size), m_pDict(pDict), m_pDataBuf(pData) {} + : m_pDict(pDict), m_dwSize(size), m_pDataBuf(pData) {} CPDF_Stream::~CPDF_Stream() { m_ObjNum = kInvalidObjNum; diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h index f0ba31924e..73484d8335 100644 --- a/core/fpdfapi/parser/cpdf_stream.h +++ b/core/fpdfapi/parser/cpdf_stream.h @@ -20,7 +20,6 @@ class CPDF_Stream : public CPDF_Object { // Takes ownership of |pData| and |pDict|. CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict); - ~CPDF_Stream() override; // CPDF_Object. Type GetType() const override; @@ -48,17 +47,20 @@ class CPDF_Stream : public CPDF_Object { bool IsMemoryBased() const { return m_bMemoryBased; } protected: + ~CPDF_Stream() override; CPDF_Object* CloneNonCyclic( bool bDirect, std::set<const CPDF_Object*>* pVisited) const override; + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> m_pDict; bool m_bMemoryBased = true; uint32_t m_dwSize = 0; - std::unique_ptr<CPDF_Dictionary> m_pDict; std::unique_ptr<uint8_t, FxFreeDeleter> m_pDataBuf; IFX_SeekableReadStream* m_pFile = nullptr; }; +using UniqueStream = std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Object>>; + inline CPDF_Stream* ToStream(CPDF_Object* obj) { return obj ? obj->AsStream() : nullptr; } @@ -67,12 +69,12 @@ inline const CPDF_Stream* ToStream(const CPDF_Object* obj) { return obj ? obj->AsStream() : nullptr; } -inline std::unique_ptr<CPDF_Stream> ToStream(std::unique_ptr<CPDF_Object> obj) { +inline UniqueStream ToStream(UniqueObject obj) { CPDF_Stream* pStream = ToStream(obj.get()); if (!pStream) return nullptr; obj.release(); - return std::unique_ptr<CPDF_Stream>(pStream); + return UniqueStream(pStream); } #endif // CORE_FPDFAPI_PARSER_CPDF_STREAM_H_ diff --git a/core/fpdfapi/parser/cpdf_string.h b/core/fpdfapi/parser/cpdf_string.h index 49834c03f2..efc6d076c7 100644 --- a/core/fpdfapi/parser/cpdf_string.h +++ b/core/fpdfapi/parser/cpdf_string.h @@ -16,7 +16,6 @@ class CPDF_String : public CPDF_Object { CPDF_String(); CPDF_String(const CFX_ByteString& str, bool bHex); explicit CPDF_String(const CFX_WideString& str); - ~CPDF_String() override; // CPDF_Object. Type GetType() const override; @@ -31,6 +30,8 @@ class CPDF_String : public CPDF_Object { bool IsHex() const { return m_bHex; } protected: + ~CPDF_String() override; + CFX_ByteString m_String; bool m_bHex; }; diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index da3c8b0c79..178af3bb9b 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp @@ -425,7 +425,8 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjectHolder* pObjList, int32_t nKeys = 0; FX_FILESIZE dwSignValuePos = 0; - std::unique_ptr<CPDF_Dictionary> pDict(new CPDF_Dictionary(m_pPool)); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict( + new CPDF_Dictionary(m_pPool)); while (1) { CFX_ByteString key = GetNextWord(nullptr); if (key.IsEmpty()) @@ -529,7 +530,8 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectForStrict( } if (word == "[") { - std::unique_ptr<CPDF_Array> pArray(new CPDF_Array); + std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>> pArray( + new CPDF_Array); while (CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, true)) pArray->Add(pObj); @@ -542,7 +544,8 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectForStrict( } if (word == "<<") { - std::unique_ptr<CPDF_Dictionary> pDict(new CPDF_Dictionary(m_pPool)); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> pDict( + new CPDF_Dictionary(m_pPool)); while (1) { FX_FILESIZE SavedPos = m_Pos; CFX_ByteString key = GetNextWord(nullptr); @@ -561,7 +564,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectForStrict( continue; key = PDF_NameDecode(key); - std::unique_ptr<CPDF_Object> obj( + std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> obj( GetObject(pObjList, objnum, gennum, true)); if (!obj) { uint8_t ch; @@ -689,7 +692,7 @@ CPDF_Stream* CPDF_SyntaxParser::ReadStream(CPDF_Dictionary* pDict, // Can't find "endstream" or "endobj". if (endStreamOffset < 0 && endObjOffset < 0) { - delete pDict; + pDict->Release(); return nullptr; } @@ -715,7 +718,7 @@ CPDF_Stream* CPDF_SyntaxParser::ReadStream(CPDF_Dictionary* pDict, } if (len < 0) { - delete pDict; + pDict->Release(); return nullptr; } pDict->SetIntegerFor("Length", len); @@ -724,7 +727,7 @@ CPDF_Stream* CPDF_SyntaxParser::ReadStream(CPDF_Dictionary* pDict, } if (len < 0) { - delete pDict; + pDict->Release(); return nullptr; } diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp index 2f3fc804f3..80edde8a8f 100644 --- a/core/fpdfdoc/cpdf_annot.cpp +++ b/core/fpdfdoc/cpdf_annot.cpp @@ -57,7 +57,7 @@ CPDF_Annot::CPDF_Annot(CPDF_Dictionary* pDict, CPDF_Annot::~CPDF_Annot() { if (m_bOwnedAnnotDict) - delete m_pAnnotDict; + m_pAnnotDict->Release(); ClearCachedAP(); } diff --git a/core/fpdfdoc/cpdf_filespec_unittest.cpp b/core/fpdfdoc/cpdf_filespec_unittest.cpp index 01989ee0bd..72b073510b 100644 --- a/core/fpdfdoc/cpdf_filespec_unittest.cpp +++ b/core/fpdfdoc/cpdf_filespec_unittest.cpp @@ -12,6 +12,13 @@ #include "testing/gtest/include/gtest/gtest.h" #include "testing/test_support.h" +namespace { + +using ScopedObj = std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>>; +using ScopedDict = + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>; +} + TEST(cpdf_filespec, EncodeDecodeFileName) { std::vector<pdfium::NullTermWstrFuncTestData> test_data = { // Empty src string. @@ -67,7 +74,7 @@ TEST(cpdf_filespec, GetFileName) { L"/docs/test.pdf" #endif }; - std::unique_ptr<CPDF_Object> str_obj(new CPDF_String(test_data.input)); + ScopedObj str_obj(new CPDF_String(test_data.input)); CPDF_FileSpec file_spec(str_obj.get()); CFX_WideString file_name; EXPECT_TRUE(file_spec.GetFileName(&file_name)); @@ -98,7 +105,7 @@ TEST(cpdf_filespec, GetFileName) { }; // Keyword fields in reverse order of precedence to retrieve the file name. const char* const keywords[5] = {"Unix", "Mac", "DOS", "F", "UF"}; - std::unique_ptr<CPDF_Dictionary> dict_obj(new CPDF_Dictionary()); + ScopedDict dict_obj(new CPDF_Dictionary()); CPDF_FileSpec file_spec(dict_obj.get()); CFX_WideString file_name; for (int i = 0; i < 5; ++i) { @@ -115,7 +122,7 @@ TEST(cpdf_filespec, GetFileName) { } { // Invalid object. - std::unique_ptr<CPDF_Object> name_obj(new CPDF_Name("test.pdf")); + ScopedObj name_obj(new CPDF_Name("test.pdf")); CPDF_FileSpec file_spec(name_obj.get()); CFX_WideString file_name; EXPECT_FALSE(file_spec.GetFileName(&file_name)); @@ -136,7 +143,7 @@ TEST(cpdf_filespec, SetFileName) { #endif }; // String object. - std::unique_ptr<CPDF_Object> str_obj(new CPDF_String(L"babababa")); + ScopedObj str_obj(new CPDF_String(L"babababa")); CPDF_FileSpec file_spec1(str_obj.get()); file_spec1.SetFileName(test_data.input); // Check internal object value. @@ -148,7 +155,7 @@ TEST(cpdf_filespec, SetFileName) { EXPECT_TRUE(file_name == test_data.input); // Dictionary object. - std::unique_ptr<CPDF_Dictionary> dict_obj(new CPDF_Dictionary()); + ScopedDict dict_obj(new CPDF_Dictionary()); CPDF_FileSpec file_spec2(dict_obj.get()); file_spec2.SetFileName(test_data.input); // Check internal object value. diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp index e2240c9f2f..e82ef7800e 100644 --- a/core/fpdfdoc/cpdf_formfield.cpp +++ b/core/fpdfdoc/cpdf_formfield.cpp @@ -575,7 +575,8 @@ bool CPDF_FormField::SetItemSelection(int index, bool bSelected, bool bNotify) { if (pValue->GetUnicodeText() == opt_value) m_pDict->RemoveFor("V"); } else if (pValue->IsArray()) { - std::unique_ptr<CPDF_Array> pArray(new CPDF_Array); + std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>> pArray( + new CPDF_Array); for (int i = 0; i < CountOptions(); i++) { if (i != index && IsItemSelected(i)) { opt_value = GetOptionValue(i); diff --git a/core/fxge/dib/fx_dib_engine_unittest.cpp b/core/fxge/dib/fx_dib_engine_unittest.cpp index 57e829e9d2..3f8abe1c97 100644 --- a/core/fxge/dib/fx_dib_engine_unittest.cpp +++ b/core/fxge/dib/fx_dib_engine_unittest.cpp @@ -12,16 +12,15 @@ #include "core/fxge/dib/dib_int.h" #include "core/fxge/fx_dib.h" #include "testing/gtest/include/gtest/gtest.h" -#include "third_party/base/ptr_util.h" TEST(CStretchEngine, OverflowInCtor) { FX_RECT clip_rect; - std::unique_ptr<CPDF_Dictionary> dict_obj = - pdfium::MakeUnique<CPDF_Dictionary>(); + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> dict_obj( + new CPDF_Dictionary()); dict_obj->SetFor("Width", new CPDF_Number(71000)); dict_obj->SetFor("Height", new CPDF_Number(12500)); - std::unique_ptr<CPDF_Stream> stream = - pdfium::MakeUnique<CPDF_Stream>(nullptr, 0, dict_obj.release()); + std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Stream>> stream( + new CPDF_Stream(nullptr, 0, dict_obj.release())); CPDF_DIBSource dib_source; dib_source.Load(nullptr, stream.get(), nullptr, nullptr, nullptr, nullptr, false, 0, false); diff --git a/fpdfsdk/fpdfdoc_unittest.cpp b/fpdfsdk/fpdfdoc_unittest.cpp index 39e81d52e8..408d2fa0d8 100644 --- a/fpdfsdk/fpdfdoc_unittest.cpp +++ b/fpdfsdk/fpdfdoc_unittest.cpp @@ -87,7 +87,7 @@ class PDFDocTest : public testing::Test { protected: std::unique_ptr<CPDF_TestPdfDocument> m_pDoc; CPDF_IndirectObjectHolder* m_pIndirectObjs; - std::unique_ptr<CPDF_Dictionary> m_pRootObj; + std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>> m_pRootObj; }; TEST_F(PDFDocTest, FindBookmark) { diff --git a/fpdfsdk/fpdfppo.cpp b/fpdfsdk/fpdfppo.cpp index ccfd141db2..22b23d1db1 100644 --- a/fpdfsdk/fpdfppo.cpp +++ b/fpdfsdk/fpdfppo.cpp @@ -285,11 +285,11 @@ uint32_t CPDF_PageOrganizer::GetNewObjId(CPDF_Document* pDoc, if (pDictClone->KeyExist("Type")) { CFX_ByteString strType = pDictClone->GetStringFor("Type"); if (!FXSYS_stricmp(strType.c_str(), "Pages")) { - delete pDictClone; + pDictClone->Release(); return 4; } if (!FXSYS_stricmp(strType.c_str(), "Page")) { - delete pDictClone; + pDictClone->Release(); return 0; } } @@ -297,7 +297,7 @@ uint32_t CPDF_PageOrganizer::GetNewObjId(CPDF_Document* pDoc, dwNewObjNum = pDoc->AddIndirectObject(pClone); (*pObjNumberMap)[dwObjnum] = dwNewObjNum; if (!UpdateReference(pClone, pDoc, pObjNumberMap)) { - delete pClone; + pClone->Release(); return 0; } return dwNewObjNum; |