From 70c4afd5c3c5c44cd24f814423a23a6ef05bba02 Mon Sep 17 00:00:00 2001 From: tsepez Date: Tue, 15 Nov 2016 11:33:44 -0800 Subject: Make AddIndirectObject() take a unique_ptr. Add convenience routines to create and add object in one step. Review-Url: https://codereview.chromium.org/2489283003 --- core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp | 24 ++++---- core/fpdfapi/page/cpdf_docpagedata.cpp | 5 +- core/fpdfapi/page/cpdf_image.cpp | 17 +++--- core/fpdfapi/parser/cfdf_document.cpp | 4 +- core/fpdfapi/parser/cpdf_array.cpp | 10 +++- core/fpdfapi/parser/cpdf_array.h | 1 + core/fpdfapi/parser/cpdf_dictionary.cpp | 12 +++- core/fpdfapi/parser/cpdf_dictionary.h | 6 +- core/fpdfapi/parser/cpdf_document.cpp | 64 +++++++++++----------- core/fpdfapi/parser/cpdf_document_unittest.cpp | 63 +++++++++++---------- .../fpdfapi/parser/cpdf_indirect_object_holder.cpp | 11 ++-- core/fpdfapi/parser/cpdf_indirect_object_holder.h | 14 ++++- core/fpdfapi/parser/cpdf_object_unittest.cpp | 30 +++++----- 13 files changed, 144 insertions(+), 117 deletions(-) (limited to 'core/fpdfapi') diff --git a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp index c27ca044e6..8e7e7b3504 100644 --- a/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp +++ b/core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp @@ -52,10 +52,9 @@ void CPDF_PageContentGenerator::GenerateContent() { if (pContent) pPageDict->RemoveFor("Contents"); - CPDF_Stream* pStream = new CPDF_Stream; + CPDF_Stream* pStream = m_pDocument->NewIndirect(); pStream->SetData(buf.GetBuffer(), buf.GetLength()); - pPageDict->SetReferenceFor("Contents", m_pDocument, - m_pDocument->AddIndirectObject(pStream)); + pPageDict->SetReferenceFor("Contents", m_pDocument, pStream); } CFX_ByteString CPDF_PageContentGenerator::RealizeResource( @@ -63,11 +62,10 @@ CFX_ByteString CPDF_PageContentGenerator::RealizeResource( const CFX_ByteString& bsType) { ASSERT(dwResourceObjNum); if (!m_pPage->m_pResources) { - m_pPage->m_pResources = - new CPDF_Dictionary(m_pDocument->GetByteStringPool()); - m_pPage->m_pFormDict->SetReferenceFor( - "Resources", m_pDocument, - m_pDocument->AddIndirectObject(m_pPage->m_pResources)); + m_pPage->m_pResources = m_pDocument->NewIndirect( + m_pDocument->GetByteStringPool()); + m_pPage->m_pFormDict->SetReferenceFor("Resources", m_pDocument, + m_pPage->m_pResources); } CPDF_Dictionary* pResList = m_pPage->m_pResources->GetDictFor(bsType); if (!pResList) { @@ -133,11 +131,10 @@ void CPDF_PageContentGenerator::ProcessForm(CFX_ByteTextBuf& buf, pFormDict->SetNameFor("Subtype", "Form"); pFormDict->SetRectFor("BBox", bbox); - CPDF_Stream* pStream = new CPDF_Stream; + CPDF_Stream* pStream = m_pDocument->NewIndirect(); pStream->InitStream(data, size, pFormDict); - CFX_ByteString name = - RealizeResource(m_pDocument->AddIndirectObject(pStream), "XObject"); + CFX_ByteString name = RealizeResource(pStream->GetObjNum(), "XObject"); buf << "/" << PDF_NameEncode(name) << " Do Q\n"; } @@ -181,8 +178,7 @@ void CPDF_PageContentGenerator::TransformContent(CFX_Matrix& matrix) { contentStream.LoadAllData(pStream); ProcessForm(buf, contentStream.GetData(), contentStream.GetSize(), matrix); } - CPDF_Stream* pStream = new CPDF_Stream; + CPDF_Stream* pStream = m_pDocument->NewIndirect(); pStream->SetData(buf.GetBuffer(), buf.GetLength()); - m_pPage->m_pFormDict->SetReferenceFor( - "Contents", m_pDocument, m_pDocument->AddIndirectObject(pStream)); + m_pPage->m_pFormDict->SetReferenceFor("Contents", m_pDocument, pStream); } diff --git a/core/fpdfapi/page/cpdf_docpagedata.cpp b/core/fpdfapi/page/cpdf_docpagedata.cpp index 7c45a04af1..2c9f4bb1f7 100644 --- a/core/fpdfapi/page/cpdf_docpagedata.cpp +++ b/core/fpdfapi/page/cpdf_docpagedata.cpp @@ -175,7 +175,8 @@ CPDF_Font* CPDF_DocPageData::GetStandardFont(const CFX_ByteString& fontName, return fontData->AddRef(); } - CPDF_Dictionary* pDict = new CPDF_Dictionary(m_pPDFDoc->GetByteStringPool()); + CPDF_Dictionary* pDict = + m_pPDFDoc->NewIndirect(m_pPDFDoc->GetByteStringPool()); pDict->SetNameFor("Type", "Font"); pDict->SetNameFor("Subtype", "Type1"); pDict->SetNameFor("BaseFont", fontName); @@ -183,7 +184,7 @@ CPDF_Font* CPDF_DocPageData::GetStandardFont(const CFX_ByteString& fontName, pDict->SetFor("Encoding", pEncoding->Realize(m_pPDFDoc->GetByteStringPool())); } - m_pPDFDoc->AddIndirectObject(pDict); + std::unique_ptr pFont = CPDF_Font::Create(m_pPDFDoc, pDict); if (!pFont) return nullptr; diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp index 4048c9bb4e..e21d38a56f 100644 --- a/core/fpdfapi/page/cpdf_image.cpp +++ b/core/fpdfapi/page/cpdf_image.cpp @@ -78,7 +78,7 @@ void CPDF_Image::ConvertStreamToIndirectObject() { return; ASSERT(m_pOwnedStream); - m_pDocument->AddIndirectObject(m_pOwnedStream.release()); + m_pDocument->AddIndirectObject(std::move(m_pOwnedStream)); } CPDF_Dictionary* CPDF_Image::InitJPEG(uint8_t* pData, uint32_t size) { @@ -212,7 +212,7 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) { } else if (bpp == 8) { int32_t iPalette = pBitmap->GetPaletteSize(); if (iPalette > 0) { - CPDF_Array* pCS = new CPDF_Array; + CPDF_Array* pCS = m_pDocument->NewIndirect(); pCS->AddName("Indexed"); pCS->AddName("DeviceRGB"); pCS->AddInteger(iPalette - 1); @@ -225,12 +225,11 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) { ptr[2] = (uint8_t)argb; ptr += 3; } - CPDF_Stream* pCTS = new CPDF_Stream( + CPDF_Stream* pCTS = m_pDocument->NewIndirect( pColorTable, iPalette * 3, new CPDF_Dictionary(m_pDocument->GetByteStringPool())); - pCS->AddReference(m_pDocument, m_pDocument->AddIndirectObject(pCTS)); - pDict->SetReferenceFor("ColorSpace", m_pDocument, - m_pDocument->AddIndirectObject(pCS)); + pCS->AddReference(m_pDocument, pCTS); + pDict->SetReferenceFor("ColorSpace", m_pDocument, pCS); } else { pDict->SetNameFor("ColorSpace", "DeviceGray"); } @@ -282,9 +281,9 @@ void CPDF_Image::SetImage(const CFX_DIBitmap* pBitmap, int32_t iCompress) { } } pMaskDict->SetIntegerFor("Length", mask_size); - pDict->SetReferenceFor("SMask", m_pDocument, - m_pDocument->AddIndirectObject(new CPDF_Stream( - mask_buf, mask_size, pMaskDict))); + pDict->SetReferenceFor( + "SMask", m_pDocument, + m_pDocument->NewIndirect(mask_buf, mask_size, pMaskDict)); if (bDeleteMask) delete pMaskBitmap; } diff --git a/core/fpdfapi/parser/cfdf_document.cpp b/core/fpdfapi/parser/cfdf_document.cpp index f54e598506..d76ae1e642 100644 --- a/core/fpdfapi/parser/cfdf_document.cpp +++ b/core/fpdfapi/parser/cfdf_document.cpp @@ -26,8 +26,8 @@ CFDF_Document::~CFDF_Document() { CFDF_Document* CFDF_Document::CreateNewDoc() { CFDF_Document* pDoc = new CFDF_Document; - pDoc->m_pRootDict = new CPDF_Dictionary(pDoc->GetByteStringPool()); - pDoc->AddIndirectObject(pDoc->m_pRootDict); + pDoc->m_pRootDict = + pDoc->NewIndirect(pDoc->GetByteStringPool()); pDoc->m_pRootDict->SetFor("FDF", new CPDF_Dictionary(pDoc->GetByteStringPool())); return pDoc; diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index af9b544f0a..f8ec46acc9 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp @@ -155,8 +155,8 @@ void CPDF_Array::ConvertToIndirectObjectAt(size_t i, if (!pObj || pObj->IsReference()) return; - uint32_t dwObjNum = pHolder->AddIndirectObject(pObj); - m_Objects[i] = new CPDF_Reference(pHolder, dwObjNum); + CPDF_Object* pNew = pHolder->AddIndirectObject(pdfium::WrapUnique(pObj)); + m_Objects[i] = new CPDF_Reference(pHolder, pNew->GetObjNum()); } void CPDF_Array::SetAt(size_t i, CPDF_Object* pObj) { @@ -209,3 +209,9 @@ void CPDF_Array::AddReference(CPDF_IndirectObjectHolder* pDoc, uint32_t objnum) { Add(new CPDF_Reference(pDoc, objnum)); } + +void CPDF_Array::AddReference(CPDF_IndirectObjectHolder* pDoc, + CPDF_Object* pObj) { + ASSERT(!pObj->IsInline()); + Add(new CPDF_Reference(pDoc, pObj->GetObjNum())); +} diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 5a9b10cb3c..bf4b8a626d 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h @@ -55,6 +55,7 @@ class CPDF_Array : public CPDF_Object { void AddString(const CFX_ByteString& str); void AddName(const CFX_ByteString& str); void AddReference(CPDF_IndirectObjectHolder* pDoc, uint32_t objnum); + void AddReference(CPDF_IndirectObjectHolder* pDoc, CPDF_Object* pObj); iterator begin() { return m_Objects.begin(); } iterator end() { return m_Objects.end(); } diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp index 403c29fa0d..02cdfa37da 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/parser/cpdf_dictionary.cpp @@ -199,8 +199,9 @@ void CPDF_Dictionary::ConvertToIndirectObjectFor( if (it == m_Map.end() || it->second->IsReference()) return; - uint32_t objnum = pHolder->AddIndirectObject(it->second); - it->second = new CPDF_Reference(pHolder, objnum); + CPDF_Object* pObj = + pHolder->AddIndirectObject(pdfium::WrapUnique(it->second)); + it->second = new CPDF_Reference(pHolder, pObj->GetObjNum()); } void CPDF_Dictionary::RemoveFor(const CFX_ByteString& key) { @@ -251,6 +252,13 @@ void CPDF_Dictionary::SetReferenceFor(const CFX_ByteString& key, SetFor(key, new CPDF_Reference(pDoc, objnum)); } +void CPDF_Dictionary::SetReferenceFor(const CFX_ByteString& key, + CPDF_IndirectObjectHolder* pDoc, + CPDF_Object* pObj) { + ASSERT(!pObj->IsInline()); + SetFor(key, new CPDF_Reference(pDoc, pObj->GetObjNum())); +} + void CPDF_Dictionary::SetNumberFor(const CFX_ByteString& key, FX_FLOAT f) { SetFor(key, new CPDF_Number(f)); } diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index fffe03463c..ebfd7e92ee 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h @@ -61,6 +61,7 @@ class CPDF_Dictionary : public CPDF_Object { // Set* functions invalidate iterators for the element with the key |key|. void SetFor(const CFX_ByteString& key, CPDF_Object* pObj); + void SetBooleanFor(const CFX_ByteString& key, bool bValue); void SetNameFor(const CFX_ByteString& key, const CFX_ByteString& name); void SetStringFor(const CFX_ByteString& key, const CFX_ByteString& str); void SetIntegerFor(const CFX_ByteString& key, int i); @@ -68,9 +69,12 @@ class CPDF_Dictionary : public CPDF_Object { void SetReferenceFor(const CFX_ByteString& key, CPDF_IndirectObjectHolder* pDoc, uint32_t objnum); + void SetReferenceFor(const CFX_ByteString& key, + CPDF_IndirectObjectHolder* pDoc, + CPDF_Object* pObj); + void SetRectFor(const CFX_ByteString& key, const CFX_FloatRect& rect); void SetMatrixFor(const CFX_ByteString& key, const CFX_Matrix& matrix); - void SetBooleanFor(const CFX_ByteString& key, bool bValue); void ConvertToIndirectObjectFor(const CFX_ByteString& key, CPDF_IndirectObjectHolder* pHolder); diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp index ebd3156e35..35c18997b9 100644 --- a/core/fpdfapi/parser/cpdf_document.cpp +++ b/core/fpdfapi/parser/cpdf_document.cpp @@ -310,15 +310,16 @@ void ProcessNonbCJK(CPDF_Dictionary* pBaseDict, pBaseDict->SetFor("Widths", pWidths); } -CPDF_Dictionary* CalculateFontDesc(CPDF_Document* pDoc, - CFX_ByteString basefont, - int flags, - int italicangle, - int ascend, - int descend, - CPDF_Array* bbox, - int32_t stemV) { - CPDF_Dictionary* pFontDesc = new CPDF_Dictionary(pDoc->GetByteStringPool()); +std::unique_ptr CalculateFontDesc(CPDF_Document* pDoc, + CFX_ByteString basefont, + int flags, + int italicangle, + int ascend, + int descend, + CPDF_Array* bbox, + int32_t stemV) { + auto pFontDesc = + pdfium::MakeUnique(pDoc->GetByteStringPool()); pFontDesc->SetNameFor("Type", "FontDescriptor"); pFontDesc->SetNameFor("FontName", basefont); pFontDesc->SetIntegerFor("Flags", flags); @@ -648,23 +649,21 @@ CPDF_Image* CPDF_Document::LoadImageFromPageData(uint32_t dwStreamObjNum) { void CPDF_Document::CreateNewDoc() { ASSERT(!m_pRootDict && !m_pInfoDict); - m_pRootDict = new CPDF_Dictionary(m_pByteStringPool); + m_pRootDict = NewIndirect(m_pByteStringPool); m_pRootDict->SetNameFor("Type", "Catalog"); - AddIndirectObject(m_pRootDict); - CPDF_Dictionary* pPages = new CPDF_Dictionary(m_pByteStringPool); + CPDF_Dictionary* pPages = NewIndirect(m_pByteStringPool); pPages->SetNameFor("Type", "Pages"); pPages->SetNumberFor("Count", 0); pPages->SetFor("Kids", new CPDF_Array); - m_pRootDict->SetReferenceFor("Pages", this, AddIndirectObject(pPages)); - m_pInfoDict = new CPDF_Dictionary(m_pByteStringPool); - AddIndirectObject(m_pInfoDict); + m_pRootDict->SetReferenceFor("Pages", this, pPages); + m_pInfoDict = NewIndirect(m_pByteStringPool); } CPDF_Dictionary* CPDF_Document::CreateNewPage(int iPage) { - CPDF_Dictionary* pDict = new CPDF_Dictionary(m_pByteStringPool); + CPDF_Dictionary* pDict = NewIndirect(m_pByteStringPool); pDict->SetNameFor("Type", "Page"); - uint32_t dwObjNum = AddIndirectObject(pDict); + uint32_t dwObjNum = pDict->GetObjNum(); if (!InsertNewPage(iPage, pDict)) { DeleteIndirectObject(dwObjNum); return nullptr; @@ -780,19 +779,21 @@ size_t CPDF_Document::CalculateEncodingDict(int charset, } if (i == FX_ArraySize(g_FX_CharsetUnicodes)) return i; - CPDF_Dictionary* pEncodingDict = new CPDF_Dictionary(m_pByteStringPool); + + CPDF_Dictionary* pEncodingDict = + NewIndirect(m_pByteStringPool); pEncodingDict->SetNameFor("BaseEncoding", "WinAnsiEncoding"); + CPDF_Array* pArray = new CPDF_Array; pArray->AddInteger(128); + const uint16_t* pUnicodes = g_FX_CharsetUnicodes[i].m_pUnicodes; for (int j = 0; j < 128; j++) { CFX_ByteString name = PDF_AdobeNameFromUnicode(pUnicodes[j]); pArray->AddName(name.IsEmpty() ? ".notdef" : name); } pEncodingDict->SetFor("Differences", pArray); - pBaseDict->SetReferenceFor("Encoding", this, - AddIndirectObject(pEncodingDict)); - + pBaseDict->SetReferenceFor("Encoding", this, pEncodingDict); return i; } @@ -802,7 +803,7 @@ CPDF_Dictionary* CPDF_Document::ProcessbCJK( bool bVert, CFX_ByteString basefont, std::function Insert) { - CPDF_Dictionary* pFontDict = new CPDF_Dictionary(m_pByteStringPool); + CPDF_Dictionary* pFontDict = NewIndirect(m_pByteStringPool); CFX_ByteString cmap; CFX_ByteString ordering; int supplement = 0; @@ -859,7 +860,7 @@ CPDF_Dictionary* CPDF_Document::ProcessbCJK( pFontDict->SetFor("CIDSystemInfo", pCIDSysInfo); CPDF_Array* pArray = new CPDF_Array; pBaseDict->SetFor("DescendantFonts", pArray); - pArray->AddReference(this, AddIndirectObject(pFontDict)); + pArray->AddReference(this, pFontDict->GetObjNum()); return pFontDict; } @@ -877,7 +878,7 @@ CPDF_Font* CPDF_Document::AddFont(CFX_Font* pFont, int charset, bool bVert) { CalculateFlags(pFont->IsBold(), pFont->IsItalic(), pFont->IsFixedWidth(), false, false, charset == FXFONT_SYMBOL_CHARSET); - CPDF_Dictionary* pBaseDict = new CPDF_Dictionary(m_pByteStringPool); + CPDF_Dictionary* pBaseDict = NewIndirect(m_pByteStringPool); pBaseDict->SetNameFor("Type", "Font"); std::unique_ptr pEncoding( new CFX_UnicodeEncoding(pFont)); @@ -918,7 +919,6 @@ CPDF_Font* CPDF_Document::AddFont(CFX_Font* pFont, int charset, bool bVert) { end, widthArr); }); } - AddIndirectObject(pBaseDict); int italicangle = pFont->GetSubstFont() ? pFont->GetSubstFont()->m_ItalicAngle : 0; FX_RECT bbox; @@ -943,11 +943,10 @@ CPDF_Font* CPDF_Document::AddFont(CFX_Font* pFont, int charset, bool bVert) { nStemV = width; } } - CPDF_Dictionary* pFontDesc = + CPDF_Dictionary* pFontDesc = ToDictionary(AddIndirectObject( CalculateFontDesc(this, basefont, flags, italicangle, pFont->GetAscent(), - pFont->GetDescent(), pBBox, nStemV); - pFontDict->SetReferenceFor("FontDescriptor", this, - AddIndirectObject(pFontDesc)); + pFont->GetDescent(), pBBox, nStemV))); + pFontDict->SetReferenceFor("FontDescriptor", this, pFontDesc); return LoadFont(pBaseDict); } @@ -1009,7 +1008,7 @@ CPDF_Font* CPDF_Document::AddWindowsFont(LOGFONTA* pLogFont, ptm->otmrcFontBox.right, ptm->otmrcFontBox.top}; FX_Free(tm_buf); basefont.Replace(" ", ""); - CPDF_Dictionary* pBaseDict = new CPDF_Dictionary(m_pByteStringPool); + CPDF_Dictionary* pBaseDict = NewIndirect(m_pByteStringPool); pBaseDict->SetNameFor("Type", "Font"); CPDF_Dictionary* pFontDict = pBaseDict; if (!bCJK) { @@ -1034,16 +1033,15 @@ CPDF_Font* CPDF_Document::AddWindowsFont(LOGFONTA* pLogFont, InsertWidthArray(hDC, start, end, widthArr); }); } - AddIndirectObject(pBaseDict); CPDF_Array* pBBox = new CPDF_Array; for (int i = 0; i < 4; i++) pBBox->AddInteger(bbox[i]); - CPDF_Dictionary* pFontDesc = + std::unique_ptr pFontDesc = CalculateFontDesc(this, basefont, flags, italicangle, ascend, descend, pBBox, pLogFont->lfWeight / 5); pFontDesc->SetIntegerFor("CapHeight", capheight); pFontDict->SetReferenceFor("FontDescriptor", this, - AddIndirectObject(pFontDesc)); + AddIndirectObject(std::move(pFontDesc))); hFont = SelectObject(hDC, hFont); DeleteObject(hFont); DeleteDC(hDC); diff --git a/core/fpdfapi/parser/cpdf_document_unittest.cpp b/core/fpdfapi/parser/cpdf_document_unittest.cpp index f27e7403f6..048a9fead2 100644 --- a/core/fpdfapi/parser/cpdf_document_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_document_unittest.cpp @@ -13,24 +13,25 @@ #include "core/fpdfapi/parser/cpdf_parser.h" #include "core/fxcrt/fx_memory.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/base/ptr_util.h" namespace { -CPDF_Dictionary* CreatePageTreeNode(CPDF_Array* kids, +CPDF_Dictionary* CreatePageTreeNode(std::unique_ptr kids, CPDF_Document* pDoc, int count) { - CPDF_Dictionary* pageNode = new CPDF_Dictionary(); + CPDF_Array* pUnowned = pDoc->AddIndirectObject(std::move(kids))->AsArray(); + CPDF_Dictionary* pageNode = pDoc->NewIndirect(); pageNode->SetStringFor("Type", "Pages"); - pageNode->SetReferenceFor("Kids", pDoc, pDoc->AddIndirectObject(kids)); + pageNode->SetReferenceFor("Kids", pDoc, pUnowned); pageNode->SetIntegerFor("Count", count); - uint32_t pageNodeRef = pDoc->AddIndirectObject(pageNode); - for (size_t i = 0; i < kids->GetCount(); i++) - kids->GetDictAt(i)->SetReferenceFor("Parent", pDoc, pageNodeRef); + for (size_t i = 0; i < pUnowned->GetCount(); i++) + pUnowned->GetDictAt(i)->SetReferenceFor("Parent", pDoc, pageNode); return pageNode; } -CPDF_Dictionary* CreateNumberedPage(size_t number) { - CPDF_Dictionary* page = new CPDF_Dictionary(); +std::unique_ptr CreateNumberedPage(size_t number) { + auto page = pdfium::MakeUnique(); page->SetStringFor("Type", "Page"); page->SetIntegerFor("PageNumbering", number); return page; @@ -40,34 +41,37 @@ class CPDF_TestDocumentForPages : public CPDF_Document { public: CPDF_TestDocumentForPages() : CPDF_Document(nullptr) { // Set up test - CPDF_Array* zeroToTwo = new CPDF_Array(); + auto zeroToTwo = pdfium::MakeUnique(); zeroToTwo->AddReference(this, AddIndirectObject(CreateNumberedPage(0))); zeroToTwo->AddReference(this, AddIndirectObject(CreateNumberedPage(1))); zeroToTwo->AddReference(this, AddIndirectObject(CreateNumberedPage(2))); - CPDF_Dictionary* branch1 = CreatePageTreeNode(zeroToTwo, this, 3); + CPDF_Dictionary* branch1 = + CreatePageTreeNode(std::move(zeroToTwo), this, 3); - CPDF_Array* zeroToThree = new CPDF_Array(); + auto zeroToThree = pdfium::MakeUnique(); zeroToThree->AddReference(this, branch1->GetObjNum()); zeroToThree->AddReference(this, AddIndirectObject(CreateNumberedPage(3))); - CPDF_Dictionary* branch2 = CreatePageTreeNode(zeroToThree, this, 4); + CPDF_Dictionary* branch2 = + CreatePageTreeNode(std::move(zeroToThree), this, 4); - CPDF_Array* fourFive = new CPDF_Array(); + auto fourFive = pdfium::MakeUnique(); fourFive->AddReference(this, AddIndirectObject(CreateNumberedPage(4))); fourFive->AddReference(this, AddIndirectObject(CreateNumberedPage(5))); - CPDF_Dictionary* branch3 = CreatePageTreeNode(fourFive, this, 2); + CPDF_Dictionary* branch3 = CreatePageTreeNode(std::move(fourFive), this, 2); - CPDF_Array* justSix = new CPDF_Array(); + auto justSix = pdfium::MakeUnique(); justSix->AddReference(this, AddIndirectObject(CreateNumberedPage(6))); - CPDF_Dictionary* branch4 = CreatePageTreeNode(justSix, this, 1); + CPDF_Dictionary* branch4 = CreatePageTreeNode(std::move(justSix), this, 1); - CPDF_Array* allPages = new CPDF_Array(); - allPages->AddReference(this, branch2->GetObjNum()); - allPages->AddReference(this, branch3->GetObjNum()); - allPages->AddReference(this, branch4->GetObjNum()); - CPDF_Dictionary* pagesDict = CreatePageTreeNode(allPages, this, 7); + auto allPages = pdfium::MakeUnique(); + allPages->AddReference(this, branch2); + allPages->AddReference(this, branch3); + allPages->AddReference(this, branch4); + CPDF_Dictionary* pagesDict = + CreatePageTreeNode(std::move(allPages), this, 7); - m_pOwnedRootDict.reset(new CPDF_Dictionary()); - m_pOwnedRootDict->SetReferenceFor("Pages", this, pagesDict->GetObjNum()); + m_pOwnedRootDict = pdfium::MakeUnique(); + m_pOwnedRootDict->SetReferenceFor("Pages", this, pagesDict); m_pRootDict = m_pOwnedRootDict.get(); m_PageList.SetSize(7); } @@ -80,13 +84,13 @@ class CPDF_TestDocumentWithPageWithoutPageNum : public CPDF_Document { public: CPDF_TestDocumentWithPageWithoutPageNum() : CPDF_Document(nullptr) { // Set up test - CPDF_Array* allPages = new CPDF_Array(); + auto allPages = pdfium::MakeUnique(); allPages->AddReference(this, AddIndirectObject(CreateNumberedPage(0))); allPages->AddReference(this, AddIndirectObject(CreateNumberedPage(1))); // Page without pageNum. - allPages->Add(CreateNumberedPage(2)); - CPDF_Dictionary* pagesDict = CreatePageTreeNode(allPages, this, 3); - + allPages->Add(CreateNumberedPage(2).release()); + CPDF_Dictionary* pagesDict = + CreatePageTreeNode(std::move(allPages), this, 3); m_pOwnedRootDict.reset(new CPDF_Dictionary()); m_pOwnedRootDict->SetReferenceFor("Pages", this, pagesDict->GetObjNum()); m_pRootDict = m_pOwnedRootDict.get(); @@ -186,15 +190,14 @@ TEST_F(cpdf_document_test, UseCachedPageObjNumIfHaveNotPagesDict) { TestLinearized linearized(dict.get()); document.LoadLinearizedDoc(&linearized); ASSERT_EQ(page_count, document.GetPageCount()); - CPDF_Object* page_stub = new CPDF_Dictionary(); - const uint32_t obj_num = document.AddIndirectObject(page_stub); + CPDF_Object* page_stub = document.NewIndirect(); + const uint32_t obj_num = page_stub->GetObjNum(); const int test_page_num = 33; EXPECT_FALSE(document.IsPageLoaded(test_page_num)); EXPECT_EQ(nullptr, document.GetPage(test_page_num)); document.SetPageObjNum(test_page_num, obj_num); - EXPECT_TRUE(document.IsPageLoaded(test_page_num)); EXPECT_EQ(page_stub, document.GetPage(test_page_num)); } diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp index 529eda21fb..12b1e9f089 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp @@ -43,13 +43,14 @@ std::unique_ptr CPDF_IndirectObjectHolder::ParseIndirectObject( return nullptr; } -uint32_t CPDF_IndirectObjectHolder::AddIndirectObject(CPDF_Object* pObj) { +CPDF_Object* CPDF_IndirectObjectHolder::AddIndirectObject( + std::unique_ptr pObj) { CHECK(!pObj->m_ObjNum); - m_LastObjNum++; + CPDF_Object* pUnowned = pObj.get(); + pObj->m_ObjNum = ++m_LastObjNum; m_IndirectObjs[m_LastObjNum].release(); // TODO(tsepez): stop this leak. - m_IndirectObjs[m_LastObjNum].reset(pObj); - pObj->m_ObjNum = m_LastObjNum; - return m_LastObjNum; + m_IndirectObjs[m_LastObjNum] = std::move(pObj); + return pUnowned; } bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration( diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h index db6f4acbf4..428bfa05ec 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h @@ -11,6 +11,7 @@ #include #include "core/fxcrt/fx_system.h" +#include "third_party/base/ptr_util.h" class CPDF_Object; @@ -26,8 +27,17 @@ class CPDF_IndirectObjectHolder { CPDF_Object* GetOrParseIndirectObject(uint32_t objnum); void DeleteIndirectObject(uint32_t objnum); - // Take ownership of |pObj|. - uint32_t AddIndirectObject(CPDF_Object* pObj); + // Creates and adds a new object owned by the indirect object holder, + // and returns an unowned pointer to it. + template + T* NewIndirect(Args... args) { + return static_cast(AddIndirectObject(pdfium::MakeUnique(args...))); + } + + // Takes ownership of |pObj|, returns unowned pointer to it. + CPDF_Object* AddIndirectObject(std::unique_ptr pObj); + + // Always takes ownership of |pObj|, return true if higher generation number. bool ReplaceIndirectObjectIfHigherGeneration( uint32_t objnum, std::unique_ptr pObj); diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp index b40a8e373d..4145f248fa 100644 --- a/core/fpdfapi/parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp @@ -90,16 +90,17 @@ class PDFObjectsTest : public testing::Test { m_DirectObjs.emplace_back(objs[i]); // Indirect references to indirect objects. - m_ObjHolder.reset(new CPDF_IndirectObjectHolder()); - m_IndirectObjs = { - boolean_true_obj->Clone().release(), number_int_obj->Clone().release(), - str_spec_obj->Clone().release(), name_obj->Clone().release(), - m_ArrayObj->Clone().release(), m_DictObj->Clone().release(), - stream_obj->Clone().release()}; - for (size_t i = 0; i < m_IndirectObjs.size(); ++i) { - m_ObjHolder->AddIndirectObject(m_IndirectObjs[i]); - m_RefObjs.emplace_back(new CPDF_Reference( - m_ObjHolder.get(), m_IndirectObjs[i]->GetObjNum())); + m_ObjHolder = pdfium::MakeUnique(); + m_IndirectObjs = {m_ObjHolder->AddIndirectObject(boolean_true_obj->Clone()), + m_ObjHolder->AddIndirectObject(number_int_obj->Clone()), + m_ObjHolder->AddIndirectObject(str_spec_obj->Clone()), + m_ObjHolder->AddIndirectObject(name_obj->Clone()), + m_ObjHolder->AddIndirectObject(m_ArrayObj->Clone()), + m_ObjHolder->AddIndirectObject(m_DictObj->Clone()), + m_ObjHolder->AddIndirectObject(stream_obj->Clone())}; + for (CPDF_Object* pObj : m_IndirectObjs) { + m_RefObjs.emplace_back( + new CPDF_Reference(m_ObjHolder.get(), pObj->GetObjNum())); } } @@ -837,14 +838,13 @@ TEST(PDFObjectTest, CloneCheckLoop) { { CPDF_IndirectObjectHolder objects_holder; // Create an object with a reference loop. - CPDF_Dictionary* dict_obj = new CPDF_Dictionary(); - CPDF_Array* arr_obj = new CPDF_Array; - objects_holder.AddIndirectObject(dict_obj); - EXPECT_EQ(1u, dict_obj->GetObjNum()); - dict_obj->SetFor("arr", arr_obj); + CPDF_Dictionary* dict_obj = objects_holder.NewIndirect(); + std::unique_ptr arr_obj = pdfium::MakeUnique(); arr_obj->InsertAt( 0, new CPDF_Reference(&objects_holder, dict_obj->GetObjNum())); CPDF_Object* elem0 = arr_obj->GetObjectAt(0); + dict_obj->SetFor("arr", arr_obj.release()); + EXPECT_EQ(1u, dict_obj->GetObjNum()); ASSERT_TRUE(elem0); ASSERT_TRUE(elem0->IsReference()); EXPECT_EQ(1u, elem0->AsReference()->GetRefObjNum()); -- cgit v1.2.3