From 335cf093231c984a23cb9ea113148ea1f19621ba Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 9 Nov 2016 13:28:26 -0800 Subject: Return unique_ptr from CPDF_Object::Clone(). Because that's what clone does. There are numerous release() calls that will go away as more code is converted. Review-Url: https://codereview.chromium.org/2484033002 --- core/fpdfapi/edit/fpdf_edit_create.cpp | 12 ++++---- core/fpdfapi/page/cpdf_contentmarkitem.cpp | 2 +- core/fpdfapi/page/cpdf_streamcontentparser.cpp | 2 +- core/fpdfapi/parser/cpdf_array.cpp | 17 ++++++------ core/fpdfapi/parser/cpdf_array.h | 6 ++-- core/fpdfapi/parser/cpdf_array_unittest.cpp | 6 ++-- core/fpdfapi/parser/cpdf_boolean.cpp | 5 ++-- core/fpdfapi/parser/cpdf_boolean.h | 2 +- core/fpdfapi/parser/cpdf_dictionary.cpp | 12 ++++---- core/fpdfapi/parser/cpdf_dictionary.h | 6 ++-- core/fpdfapi/parser/cpdf_name.cpp | 5 ++-- core/fpdfapi/parser/cpdf_name.h | 2 +- core/fpdfapi/parser/cpdf_null.cpp | 5 ++-- core/fpdfapi/parser/cpdf_null.h | 2 +- core/fpdfapi/parser/cpdf_number.cpp | 6 ++-- core/fpdfapi/parser/cpdf_number.h | 4 +-- core/fpdfapi/parser/cpdf_object.cpp | 7 +++-- core/fpdfapi/parser/cpdf_object.h | 11 ++++---- core/fpdfapi/parser/cpdf_object_unittest.cpp | 32 ++++++++++++---------- core/fpdfapi/parser/cpdf_parser.cpp | 12 ++++---- core/fpdfapi/parser/cpdf_reference.cpp | 7 +++-- core/fpdfapi/parser/cpdf_reference.h | 6 ++-- core/fpdfapi/parser/cpdf_stream.cpp | 12 ++++---- core/fpdfapi/parser/cpdf_stream.h | 6 ++-- core/fpdfapi/parser/cpdf_string.cpp | 5 ++-- core/fpdfapi/parser/cpdf_string.h | 4 +-- core/fpdfdoc/cpdf_formfield.cpp | 8 +++--- core/fpdfdoc/cpdf_interform.cpp | 13 +++++---- core/fpdfdoc/cpvt_generateap.cpp | 6 ++-- fpdfsdk/fpdf_flatten.cpp | 5 ++-- fpdfsdk/fpdfppo.cpp | 38 ++++++++++---------------- 31 files changed, 136 insertions(+), 130 deletions(-) diff --git a/core/fpdfapi/edit/fpdf_edit_create.cpp b/core/fpdfapi/edit/fpdf_edit_create.cpp index a578a0a114..c823ab01e1 100644 --- a/core/fpdfapi/edit/fpdf_edit_create.cpp +++ b/core/fpdfapi/edit/fpdf_edit_create.cpp @@ -404,7 +404,7 @@ class CPDF_FlateEncoder { void CPDF_FlateEncoder::CloneDict() { if (!m_bCloned) { - m_pDict = ToDictionary(m_pDict->Clone()); + m_pDict = ToDictionary(m_pDict->Clone().release()); ASSERT(m_pDict); m_bCloned = true; } @@ -425,7 +425,7 @@ CPDF_FlateEncoder::CPDF_FlateEncoder(CPDF_Stream* pStream, bool bFlateEncode) destAcc.LoadAllData(pStream); m_dwSize = destAcc.GetSize(); m_pData = (uint8_t*)destAcc.DetachData(); - m_pDict = ToDictionary(pStream->GetDict()->Clone()); + m_pDict = ToDictionary(pStream->GetDict()->Clone().release()); m_pDict->RemoveFor("Filter"); m_bNewData = true; m_bCloned = true; @@ -441,7 +441,7 @@ CPDF_FlateEncoder::CPDF_FlateEncoder(CPDF_Stream* pStream, bool bFlateEncode) m_bCloned = true; // TODO(thestig): Move to Init() and check return value. ::FlateEncode(m_Acc.GetData(), m_Acc.GetSize(), &m_pData, &m_dwSize); - m_pDict = ToDictionary(pStream->GetDict()->Clone()); + m_pDict = ToDictionary(pStream->GetDict()->Clone().release()); m_pDict->SetIntegerFor("Length", m_dwSize); m_pDict->SetNameFor("Filter", "FlateDecode"); m_pDict->RemoveFor("DecodeParms"); @@ -1931,7 +1931,7 @@ void CPDF_Creator::InitID(bool bDefault) { m_pIDArray.reset(new CPDF_Array); CPDF_Object* pID1 = pOldIDArray ? pOldIDArray->GetObjectAt(0) : nullptr; if (pID1) { - m_pIDArray->Add(pID1->Clone()); + m_pIDArray->Add(pID1->Clone().release()); } else { std::vector buffer = PDF_GenerateFileID((uint32_t)(uintptr_t) this, m_dwLastObjNum); @@ -1945,7 +1945,7 @@ void CPDF_Creator::InitID(bool bDefault) { if (pOldIDArray) { CPDF_Object* pID2 = pOldIDArray->GetObjectAt(1); if ((m_dwFlags & FPDFCREATE_INCREMENTAL) && m_pEncryptDict && pID2) { - m_pIDArray->Add(pID2->Clone()); + m_pIDArray->Add(pID2->Clone().release()); return; } std::vector buffer = @@ -1954,7 +1954,7 @@ void CPDF_Creator::InitID(bool bDefault) { m_pIDArray->Add(new CPDF_String(bsBuffer, true)); return; } - m_pIDArray->Add(m_pIDArray->GetObjectAt(0)->Clone()); + m_pIDArray->Add(m_pIDArray->GetObjectAt(0)->Clone().release()); if (m_pEncryptDict && !pOldIDArray && m_pParser && bNewId) { if (m_pEncryptDict->GetStringFor("Filter") == "Standard") { CFX_ByteString user_pass = m_pParser->GetPassword(); diff --git a/core/fpdfapi/page/cpdf_contentmarkitem.cpp b/core/fpdfapi/page/cpdf_contentmarkitem.cpp index dffeada707..48a9679251 100644 --- a/core/fpdfapi/page/cpdf_contentmarkitem.cpp +++ b/core/fpdfapi/page/cpdf_contentmarkitem.cpp @@ -16,7 +16,7 @@ CPDF_ContentMarkItem::CPDF_ContentMarkItem(const CPDF_ContentMarkItem& that) m_ParamType(that.m_ParamType), m_pPropertiesDict(that.m_pPropertiesDict) { if (that.m_pDirectDict) - m_pDirectDict.reset(that.m_pDirectDict->Clone()->AsDictionary()); + m_pDirectDict = ToDictionary(that.m_pDirectDict->Clone()); } CPDF_ContentMarkItem::~CPDF_ContentMarkItem() {} diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index cd77c0b633..e7d23c0522 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -667,7 +667,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() { if (name != "DeviceRGB" && name != "DeviceGray" && name != "DeviceCMYK") { pCSObj = FindResourceObj("ColorSpace", name); if (pCSObj && pCSObj->IsInline()) { - pCSObj = pCSObj->Clone(); + pCSObj = pCSObj->Clone().release(); pDict->SetFor("ColorSpace", pCSObj); } } diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index 4000bbc980..af9b544f0a 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp @@ -44,21 +44,22 @@ const CPDF_Array* CPDF_Array::AsArray() const { return this; } -CPDF_Object* CPDF_Array::Clone() const { +std::unique_ptr CPDF_Array::Clone() const { return CloneObjectNonCyclic(false); } -CPDF_Object* CPDF_Array::CloneNonCyclic( +std::unique_ptr CPDF_Array::CloneNonCyclic( bool bDirect, std::set* pVisited) const { pVisited->insert(this); - CPDF_Array* pCopy = new CPDF_Array(); - for (size_t i = 0; i < GetCount(); i++) { - CPDF_Object* value = m_Objects[i]; - if (!pdfium::ContainsKey(*pVisited, value)) - pCopy->m_Objects.push_back(value->CloneNonCyclic(bDirect, pVisited)); + auto pCopy = pdfium::MakeUnique(); + for (CPDF_Object* value : m_Objects) { + if (!pdfium::ContainsKey(*pVisited, value)) { + pCopy->m_Objects.push_back( + value->CloneNonCyclic(bDirect, pVisited).release()); + } } - return pCopy; + return std::move(pCopy); } CFX_FloatRect CPDF_Array::GetRect() { diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 9deb478809..5a9b10cb3c 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h @@ -23,9 +23,9 @@ class CPDF_Array : public CPDF_Object { CPDF_Array(); ~CPDF_Array() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; bool IsArray() const override; CPDF_Array* AsArray() override; const CPDF_Array* AsArray() const override; @@ -62,7 +62,7 @@ class CPDF_Array : public CPDF_Object { const_iterator end() const { return m_Objects.end(); } protected: - CPDF_Object* CloneNonCyclic( + std::unique_ptr CloneNonCyclic( bool bDirect, std::set* pVisited) const override; diff --git a/core/fpdfapi/parser/cpdf_array_unittest.cpp b/core/fpdfapi/parser/cpdf_array_unittest.cpp index b1a4605666..acb1bd89a9 100644 --- a/core/fpdfapi/parser/cpdf_array_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_array_unittest.cpp @@ -87,7 +87,7 @@ TEST(cpdf_array, Clone) { std::unique_ptr 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 arr2(arr->Clone()->AsArray()); + std::unique_ptr arr2 = ToArray(arr->Clone()); EXPECT_EQ(arr->GetCount(), arr2->GetCount()); for (size_t i = 0; i < FX_ArraySize(elems); ++i) { // Clone() always create new objects. @@ -120,10 +120,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 arr1(arr->Clone()->AsArray()); + std::unique_ptr arr1 = ToArray(arr->Clone()); EXPECT_EQ(arr->GetCount(), arr1->GetCount()); // Dereferencing reference objects creates new copies of direct objects. - std::unique_ptr arr2(arr->CloneDirectObject()->AsArray()); + std::unique_ptr arr2 = ToArray(arr->CloneDirectObject()); EXPECT_EQ(arr->GetCount(), arr2->GetCount()); for (size_t i = 0; i < kNumOfRows; ++i) { CPDF_Array* arr_elem = arr->GetObjectAt(i)->AsArray(); diff --git a/core/fpdfapi/parser/cpdf_boolean.cpp b/core/fpdfapi/parser/cpdf_boolean.cpp index 416b6ff4bc..0204fd9eb0 100644 --- a/core/fpdfapi/parser/cpdf_boolean.cpp +++ b/core/fpdfapi/parser/cpdf_boolean.cpp @@ -5,6 +5,7 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com #include "core/fpdfapi/parser/cpdf_boolean.h" +#include "third_party/base/ptr_util.h" CPDF_Boolean::CPDF_Boolean() : m_bValue(false) {} @@ -16,8 +17,8 @@ CPDF_Object::Type CPDF_Boolean::GetType() const { return BOOLEAN; } -CPDF_Object* CPDF_Boolean::Clone() const { - return new CPDF_Boolean(m_bValue); +std::unique_ptr CPDF_Boolean::Clone() const { + return pdfium::MakeUnique(m_bValue); } CFX_ByteString CPDF_Boolean::GetString() const { diff --git a/core/fpdfapi/parser/cpdf_boolean.h b/core/fpdfapi/parser/cpdf_boolean.h index bc864a6ab8..808f9ee6f0 100644 --- a/core/fpdfapi/parser/cpdf_boolean.h +++ b/core/fpdfapi/parser/cpdf_boolean.h @@ -19,7 +19,7 @@ class CPDF_Boolean : public CPDF_Object { // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CFX_ByteString GetString() const override; int GetInteger() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp index 37efbbc34a..403c29fa0d 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/parser/cpdf_dictionary.cpp @@ -57,23 +57,23 @@ const CPDF_Dictionary* CPDF_Dictionary::AsDictionary() const { return this; } -CPDF_Object* CPDF_Dictionary::Clone() const { +std::unique_ptr CPDF_Dictionary::Clone() const { return CloneObjectNonCyclic(false); } -CPDF_Object* CPDF_Dictionary::CloneNonCyclic( +std::unique_ptr CPDF_Dictionary::CloneNonCyclic( bool bDirect, std::set* pVisited) const { pVisited->insert(this); - CPDF_Dictionary* pCopy = new CPDF_Dictionary(m_pPool); + auto pCopy = pdfium::MakeUnique(m_pPool); for (const auto& it : *this) { CPDF_Object* value = it.second; if (!pdfium::ContainsKey(*pVisited, value)) { - pCopy->m_Map.insert( - std::make_pair(it.first, value->CloneNonCyclic(bDirect, pVisited))); + pCopy->m_Map.insert(std::make_pair( + it.first, value->CloneNonCyclic(bDirect, pVisited).release())); } } - return pCopy; + return std::move(pCopy); } CPDF_Object* CPDF_Dictionary::GetObjectFor(const CFX_ByteString& key) const { diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index 4ef2f96ce7..fffe03463c 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h @@ -28,9 +28,9 @@ class CPDF_Dictionary : public CPDF_Object { explicit CPDF_Dictionary(const CFX_WeakPtr& pPool); ~CPDF_Dictionary() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CPDF_Dictionary* GetDict() const override; bool IsDictionary() const override; CPDF_Dictionary* AsDictionary() override; @@ -90,7 +90,7 @@ class CPDF_Dictionary : public CPDF_Object { protected: CFX_ByteString MaybeIntern(const CFX_ByteString& str); - CPDF_Object* CloneNonCyclic( + std::unique_ptr CloneNonCyclic( bool bDirect, std::set* visited) const override; diff --git a/core/fpdfapi/parser/cpdf_name.cpp b/core/fpdfapi/parser/cpdf_name.cpp index 5cc8479c3b..bb46425117 100644 --- a/core/fpdfapi/parser/cpdf_name.cpp +++ b/core/fpdfapi/parser/cpdf_name.cpp @@ -7,6 +7,7 @@ #include "core/fpdfapi/parser/cpdf_name.h" #include "core/fpdfapi/parser/fpdf_parser_decode.h" +#include "third_party/base/ptr_util.h" CPDF_Name::CPDF_Name(const CFX_ByteString& str) : m_Name(str) {} @@ -16,8 +17,8 @@ CPDF_Object::Type CPDF_Name::GetType() const { return NAME; } -CPDF_Object* CPDF_Name::Clone() const { - return new CPDF_Name(m_Name); +std::unique_ptr CPDF_Name::Clone() const { + return pdfium::MakeUnique(m_Name); } CFX_ByteString CPDF_Name::GetString() const { diff --git a/core/fpdfapi/parser/cpdf_name.h b/core/fpdfapi/parser/cpdf_name.h index aea1de4f83..ee50595ed9 100644 --- a/core/fpdfapi/parser/cpdf_name.h +++ b/core/fpdfapi/parser/cpdf_name.h @@ -16,7 +16,7 @@ class CPDF_Name : public CPDF_Object { // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CFX_ByteString GetString() const override; CFX_WideString GetUnicodeText() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfapi/parser/cpdf_null.cpp b/core/fpdfapi/parser/cpdf_null.cpp index dd23101aa8..41478d7b4c 100644 --- a/core/fpdfapi/parser/cpdf_null.cpp +++ b/core/fpdfapi/parser/cpdf_null.cpp @@ -5,6 +5,7 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com #include "core/fpdfapi/parser/cpdf_null.h" +#include "third_party/base/ptr_util.h" CPDF_Null::CPDF_Null() {} @@ -12,6 +13,6 @@ CPDF_Object::Type CPDF_Null::GetType() const { return NULLOBJ; } -CPDF_Object* CPDF_Null::Clone() const { - return new CPDF_Null; +std::unique_ptr CPDF_Null::Clone() const { + return pdfium::MakeUnique(); } diff --git a/core/fpdfapi/parser/cpdf_null.h b/core/fpdfapi/parser/cpdf_null.h index 2b8b053b99..df985b92c0 100644 --- a/core/fpdfapi/parser/cpdf_null.h +++ b/core/fpdfapi/parser/cpdf_null.h @@ -15,7 +15,7 @@ class CPDF_Null : public CPDF_Object { // CPDF_Object. Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; }; #endif // CORE_FPDFAPI_PARSER_CPDF_NULL_H_ diff --git a/core/fpdfapi/parser/cpdf_number.cpp b/core/fpdfapi/parser/cpdf_number.cpp index 3ae629ede9..24feb2a2e0 100644 --- a/core/fpdfapi/parser/cpdf_number.cpp +++ b/core/fpdfapi/parser/cpdf_number.cpp @@ -5,6 +5,7 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com #include "core/fpdfapi/parser/cpdf_number.h" +#include "third_party/base/ptr_util.h" CPDF_Number::CPDF_Number() : m_bInteger(true), m_Integer(0) {} @@ -21,8 +22,9 @@ CPDF_Object::Type CPDF_Number::GetType() const { return NUMBER; } -CPDF_Object* CPDF_Number::Clone() const { - return m_bInteger ? new CPDF_Number(m_Integer) : new CPDF_Number(m_Float); +std::unique_ptr CPDF_Number::Clone() const { + return m_bInteger ? pdfium::MakeUnique(m_Integer) + : pdfium::MakeUnique(m_Float); } FX_FLOAT CPDF_Number::GetNumber() const { diff --git a/core/fpdfapi/parser/cpdf_number.h b/core/fpdfapi/parser/cpdf_number.h index 717c2b7887..0a8f187244 100644 --- a/core/fpdfapi/parser/cpdf_number.h +++ b/core/fpdfapi/parser/cpdf_number.h @@ -19,9 +19,9 @@ class CPDF_Number : public CPDF_Object { explicit CPDF_Number(const CFX_ByteStringC& str); ~CPDF_Number() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CFX_ByteString GetString() const override; FX_FLOAT GetNumber() const override; int GetInteger() const override; diff --git a/core/fpdfapi/parser/cpdf_object.cpp b/core/fpdfapi/parser/cpdf_object.cpp index e9c215ce19..acda334c9e 100644 --- a/core/fpdfapi/parser/cpdf_object.cpp +++ b/core/fpdfapi/parser/cpdf_object.cpp @@ -22,16 +22,17 @@ CPDF_Object* CPDF_Object::GetDirect() const { return const_cast(this); } -CPDF_Object* CPDF_Object::CloneObjectNonCyclic(bool bDirect) const { +std::unique_ptr CPDF_Object::CloneObjectNonCyclic( + bool bDirect) const { std::set visited_objs; return CloneNonCyclic(bDirect, &visited_objs); } -CPDF_Object* CPDF_Object::CloneDirectObject() const { +std::unique_ptr CPDF_Object::CloneDirectObject() const { return CloneObjectNonCyclic(true); } -CPDF_Object* CPDF_Object::CloneNonCyclic( +std::unique_ptr CPDF_Object::CloneNonCyclic( bool bDirect, std::set* pVisited) const { return Clone(); diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h index 8f6491ec72..c24b40a789 100644 --- a/core/fpdfapi/parser/cpdf_object.h +++ b/core/fpdfapi/parser/cpdf_object.h @@ -46,12 +46,13 @@ class CPDF_Object { bool IsInline() const { return m_ObjNum == 0; } // Create a deep copy of the object. - virtual CPDF_Object* Clone() const = 0; + virtual std::unique_ptr Clone() const = 0; + // Create a deep copy of the object except any reference object be // copied to the object it points to directly. - virtual CPDF_Object* CloneDirectObject() const; - virtual CPDF_Object* GetDirect() const; + virtual std::unique_ptr CloneDirectObject() const; + virtual CPDF_Object* GetDirect() const; virtual CFX_ByteString GetString() const; virtual CFX_WideString GetUnicodeText() const; virtual FX_FLOAT GetNumber() const; @@ -97,7 +98,7 @@ class CPDF_Object { CPDF_Object() : m_ObjNum(0), m_GenNum(0) {} - CPDF_Object* CloneObjectNonCyclic(bool bDirect) const; + std::unique_ptr CloneObjectNonCyclic(bool bDirect) const; // Create a deep copy of the object with the option to either // copy a reference object or directly copy the object it refers to @@ -105,7 +106,7 @@ class CPDF_Object { // Also check cyclic reference against |pVisited|, no copy if it is found. // Complex objects should implement their own CloneNonCyclic() // function to properly check for possible loop. - virtual CPDF_Object* CloneNonCyclic( + virtual std::unique_ptr CloneNonCyclic( bool bDirect, std::set* pVisited) const; diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp index 1bcf6164be..b40a8e373d 100644 --- a/core/fpdfapi/parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp @@ -91,10 +91,11 @@ class PDFObjectsTest : public testing::Test { // Indirect references to indirect objects. m_ObjHolder.reset(new CPDF_IndirectObjectHolder()); - m_IndirectObjs = {boolean_true_obj->Clone(), number_int_obj->Clone(), - str_spec_obj->Clone(), name_obj->Clone(), - m_ArrayObj->Clone(), m_DictObj->Clone(), - stream_obj->Clone()}; + 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( @@ -268,13 +269,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 obj(m_DirectObjs[i]->Clone()); + std::unique_ptr 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 obj(it->Clone()); + std::unique_ptr obj = it->Clone(); EXPECT_TRUE(Equal(it.get(), obj.get())); } } @@ -751,11 +752,12 @@ TEST(PDFArrayTest, CloneDirectObject) { ASSERT_TRUE(obj); EXPECT_TRUE(obj->IsReference()); - CPDF_Object* cloned_array_object = array->CloneDirectObject(); + std::unique_ptr cloned_array_object = array->CloneDirectObject(); ASSERT_TRUE(cloned_array_object); ASSERT_TRUE(cloned_array_object->IsArray()); - std::unique_ptr cloned_array(cloned_array_object->AsArray()); + std::unique_ptr cloned_array = + ToArray(std::move(cloned_array_object)); ASSERT_EQ(1U, cloned_array->GetCount()); CPDF_Object* cloned_obj = cloned_array->GetObjectAt(0); EXPECT_FALSE(cloned_obj); @@ -785,12 +787,12 @@ TEST(PDFDictionaryTest, CloneDirectObject) { ASSERT_TRUE(obj); EXPECT_TRUE(obj->IsReference()); - CPDF_Object* cloned_dict_object = dict->CloneDirectObject(); + std::unique_ptr cloned_dict_object = dict->CloneDirectObject(); ASSERT_TRUE(cloned_dict_object); ASSERT_TRUE(cloned_dict_object->IsDictionary()); - std::unique_ptr cloned_dict( - cloned_dict_object->AsDictionary()); + std::unique_ptr cloned_dict = + ToDictionary(std::move(cloned_dict_object)); ASSERT_EQ(1U, cloned_dict->GetCount()); CPDF_Object* cloned_obj = cloned_dict->GetObjectFor("foo"); EXPECT_FALSE(cloned_obj); @@ -805,7 +807,7 @@ TEST(PDFObjectTest, CloneCheckLoop) { arr_obj->InsertAt(0, dict_obj); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr cloned_array(arr_obj->Clone()->AsArray()); + std::unique_ptr cloned_array = ToArray(arr_obj->Clone()); // Cloned object should be the same as the original. ASSERT_TRUE(cloned_array); EXPECT_EQ(1u, cloned_array->GetCount()); @@ -823,7 +825,7 @@ TEST(PDFObjectTest, CloneCheckLoop) { dict_obj->SetFor("stream", stream_obj.get()); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr cloned_stream(stream_obj->Clone()->AsStream()); + std::unique_ptr cloned_stream = ToStream(stream_obj->Clone()); // Cloned object should be the same as the original. ASSERT_TRUE(cloned_stream); CPDF_Object* cloned_dict = cloned_stream->GetDict(); @@ -849,8 +851,8 @@ TEST(PDFObjectTest, CloneCheckLoop) { EXPECT_EQ(dict_obj, elem0->AsReference()->GetDirect()); // Clone this object to see whether stack overflow will be triggered. - std::unique_ptr cloned_dict( - ToDictionary(dict_obj->CloneDirectObject())); + std::unique_ptr 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"); diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index ed20cf73f4..2d96834964 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp @@ -737,8 +737,7 @@ bool CPDF_Parser::RebuildCrossRef() { CPDF_Object* pRoot = pDict->GetObjectFor("Root"); if (pRoot && pRoot->GetDict() && pRoot->GetDict()->GetObjectFor("Pages")) { - m_pTrailer = - ToDictionary(pdfium::WrapUnique(pDict->Clone())); + m_pTrailer = ToDictionary(pDict->Clone()); } } } @@ -811,14 +810,14 @@ bool CPDF_Parser::RebuildCrossRef() { m_pTrailer->SetReferenceFor(key, m_pDocument, dwObjNum); } else { - m_pTrailer->SetFor(key, pElement->Clone()); + m_pTrailer->SetFor(key, + pElement->Clone().release()); } } } } else { if (pObj->IsStream()) { - m_pTrailer = - ToDictionary(pdfium::WrapUnique(pTrailer->Clone())); + m_pTrailer = ToDictionary(pTrailer->Clone()); } else { m_pTrailer = ToDictionary(std::move(pObj)); } @@ -959,8 +958,7 @@ bool CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, bool bMainXRef) { if (size < 0) return false; - std::unique_ptr pNewTrailer = - ToDictionary(pdfium::WrapUnique(pDict->Clone())); + std::unique_ptr pNewTrailer = ToDictionary(pDict->Clone()); if (bMainXRef) { m_pTrailer = std::move(pNewTrailer); ShrinkObjectMap(size); diff --git a/core/fpdfapi/parser/cpdf_reference.cpp b/core/fpdfapi/parser/cpdf_reference.cpp index a9cdf54855..8f44aa0200 100644 --- a/core/fpdfapi/parser/cpdf_reference.cpp +++ b/core/fpdfapi/parser/cpdf_reference.cpp @@ -7,6 +7,7 @@ #include "core/fpdfapi/parser/cpdf_reference.h" #include "core/fpdfapi/parser/cpdf_indirect_object_holder.h" +#include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" CPDF_Reference::CPDF_Reference(CPDF_IndirectObjectHolder* pDoc, int objnum) @@ -50,11 +51,11 @@ const CPDF_Reference* CPDF_Reference::AsReference() const { return this; } -CPDF_Object* CPDF_Reference::Clone() const { +std::unique_ptr CPDF_Reference::Clone() const { return CloneObjectNonCyclic(false); } -CPDF_Object* CPDF_Reference::CloneNonCyclic( +std::unique_ptr CPDF_Reference::CloneNonCyclic( bool bDirect, std::set* pVisited) const { pVisited->insert(this); @@ -64,7 +65,7 @@ CPDF_Object* CPDF_Reference::CloneNonCyclic( ? pDirect->CloneNonCyclic(true, pVisited) : nullptr; } - return new CPDF_Reference(m_pObjList, m_RefObjNum); + return pdfium::MakeUnique(m_pObjList, m_RefObjNum); } CPDF_Object* CPDF_Reference::SafeGetDirect() const { diff --git a/core/fpdfapi/parser/cpdf_reference.h b/core/fpdfapi/parser/cpdf_reference.h index 20516ae567..93bab00334 100644 --- a/core/fpdfapi/parser/cpdf_reference.h +++ b/core/fpdfapi/parser/cpdf_reference.h @@ -18,9 +18,9 @@ class CPDF_Reference : public CPDF_Object { CPDF_Reference(CPDF_IndirectObjectHolder* pDoc, int objnum); ~CPDF_Reference() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CPDF_Object* GetDirect() const override; CFX_ByteString GetString() const override; FX_FLOAT GetNumber() const override; @@ -36,7 +36,7 @@ class CPDF_Reference : public CPDF_Object { void SetRef(CPDF_IndirectObjectHolder* pDoc, uint32_t objnum); protected: - CPDF_Object* CloneNonCyclic( + std::unique_ptr CloneNonCyclic( bool bDirect, std::set* pVisited) const override; CPDF_Object* SafeGetDirect() const; diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp index 11ef1d2dc1..e8ee022940 100644 --- a/core/fpdfapi/parser/cpdf_stream.cpp +++ b/core/fpdfapi/parser/cpdf_stream.cpp @@ -68,11 +68,11 @@ void CPDF_Stream::InitStreamFromFile(IFX_SeekableReadStream* pFile, m_pDict->SetIntegerFor("Length", m_dwSize); } -CPDF_Object* CPDF_Stream::Clone() const { +std::unique_ptr CPDF_Stream::Clone() const { return CloneObjectNonCyclic(false); } -CPDF_Object* CPDF_Stream::CloneNonCyclic( +std::unique_ptr CPDF_Stream::CloneNonCyclic( bool bDirect, std::set* pVisited) const { pVisited->insert(this); @@ -81,11 +81,11 @@ CPDF_Object* CPDF_Stream::CloneNonCyclic( uint32_t streamSize = acc.GetSize(); CPDF_Dictionary* pDict = GetDict(); if (pDict && !pdfium::ContainsKey(*pVisited, pDict)) { - pDict = ToDictionary( - static_cast(pDict)->CloneNonCyclic(bDirect, pVisited)); + pDict = ToDictionary(static_cast(pDict) + ->CloneNonCyclic(bDirect, pVisited) + .release()); } - - return new CPDF_Stream(acc.DetachData(), streamSize, pDict); + return pdfium::MakeUnique(acc.DetachData(), streamSize, pDict); } void CPDF_Stream::SetData(const uint8_t* pData, uint32_t size) { diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h index f0ba31924e..ddf7cc5b69 100644 --- a/core/fpdfapi/parser/cpdf_stream.h +++ b/core/fpdfapi/parser/cpdf_stream.h @@ -22,9 +22,9 @@ class CPDF_Stream : public CPDF_Object { CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict); ~CPDF_Stream() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CPDF_Dictionary* GetDict() const override; CFX_WideString GetUnicodeText() const override; bool IsStream() const override; @@ -48,7 +48,7 @@ class CPDF_Stream : public CPDF_Object { bool IsMemoryBased() const { return m_bMemoryBased; } protected: - CPDF_Object* CloneNonCyclic( + std::unique_ptr CloneNonCyclic( bool bDirect, std::set* pVisited) const override; diff --git a/core/fpdfapi/parser/cpdf_string.cpp b/core/fpdfapi/parser/cpdf_string.cpp index f4fa956280..2116c200fe 100644 --- a/core/fpdfapi/parser/cpdf_string.cpp +++ b/core/fpdfapi/parser/cpdf_string.cpp @@ -7,6 +7,7 @@ #include "core/fpdfapi/parser/cpdf_string.h" #include "core/fpdfapi/parser/fpdf_parser_decode.h" +#include "third_party/base/ptr_util.h" CPDF_String::CPDF_String() : m_bHex(false) {} @@ -23,8 +24,8 @@ CPDF_Object::Type CPDF_String::GetType() const { return STRING; } -CPDF_Object* CPDF_String::Clone() const { - return new CPDF_String(m_String, m_bHex); +std::unique_ptr CPDF_String::Clone() const { + return pdfium::MakeUnique(m_String, m_bHex); } CFX_ByteString CPDF_String::GetString() const { diff --git a/core/fpdfapi/parser/cpdf_string.h b/core/fpdfapi/parser/cpdf_string.h index 49834c03f2..1e73e8699f 100644 --- a/core/fpdfapi/parser/cpdf_string.h +++ b/core/fpdfapi/parser/cpdf_string.h @@ -18,9 +18,9 @@ class CPDF_String : public CPDF_Object { explicit CPDF_String(const CFX_WideString& str); ~CPDF_String() override; - // CPDF_Object. + // CPDF_Object: Type GetType() const override; - CPDF_Object* Clone() const override; + std::unique_ptr Clone() const override; CFX_ByteString GetString() const override; CFX_WideString GetUnicodeText() const override; void SetString(const CFX_ByteString& str) override; diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp index 8170b7fcbf..4999f2d6b8 100644 --- a/core/fpdfdoc/cpdf_formfield.cpp +++ b/core/fpdfdoc/cpdf_formfield.cpp @@ -219,14 +219,14 @@ bool CPDF_FormField::ResetField(bool bNotify) { return false; if (pDV) { - CPDF_Object* pClone = pDV->Clone(); + std::unique_ptr pClone = pDV->Clone(); if (!pClone) return false; - m_pDict->SetFor("V", pClone); + m_pDict->SetFor("V", pClone.release()); if (pRV) { - CPDF_Object* pCloneR = pDV->Clone(); - m_pDict->SetFor("RV", pCloneR); + std::unique_ptr pCloneR = pDV->Clone(); + m_pDict->SetFor("RV", pCloneR.release()); } } else { m_pDict->RemoveFor("V"); diff --git a/core/fpdfdoc/cpdf_interform.cpp b/core/fpdfdoc/cpdf_interform.cpp index f709c57961..2a02e890c3 100644 --- a/core/fpdfdoc/cpdf_interform.cpp +++ b/core/fpdfdoc/cpdf_interform.cpp @@ -1105,22 +1105,22 @@ CPDF_FormField* CPDF_InterForm::AddTerminalField(CPDF_Dictionary* pFieldDict) { if (pFieldDict->KeyExist("FT")) { CPDF_Object* pFTValue = pFieldDict->GetDirectObjectFor("FT"); if (pFTValue) - pParent->SetFor("FT", pFTValue->Clone()); + pParent->SetFor("FT", pFTValue->Clone().release()); } if (pFieldDict->KeyExist("Ff")) { CPDF_Object* pFfValue = pFieldDict->GetDirectObjectFor("Ff"); if (pFfValue) - pParent->SetFor("Ff", pFfValue->Clone()); + pParent->SetFor("Ff", pFfValue->Clone().release()); } } pField = new CPDF_FormField(this, pParent); CPDF_Object* pTObj = pDict->GetObjectFor("T"); if (ToReference(pTObj)) { - CPDF_Object* pClone = pTObj->CloneDirectObject(); + std::unique_ptr pClone = pTObj->CloneDirectObject(); if (pClone) - pDict->SetFor("T", pClone); + pDict->SetFor("T", pClone.release()); else pDict->SetNameFor("T", ""); } @@ -1251,7 +1251,7 @@ CFDF_Document* CPDF_InterForm::ExportToFDF( } else { CPDF_Object* pV = FPDF_GetFieldAttr(pField->m_pDict, "V"); if (pV) - pFieldDict->SetFor("V", pV->CloneDirectObject()); + pFieldDict->SetFor("V", pV->CloneDirectObject().release()); } pFields->Add(pFieldDict); } @@ -1304,7 +1304,8 @@ void CPDF_InterForm::FDF_ImportField(CPDF_Dictionary* pFieldDict, if ((eType == CPDF_FormField::ListBox || eType == CPDF_FormField::ComboBox) && pFieldDict->KeyExist("Opt")) { pField->m_pDict->SetFor( - "Opt", pFieldDict->GetDirectObjectFor("Opt")->CloneDirectObject()); + "Opt", + pFieldDict->GetDirectObjectFor("Opt")->CloneDirectObject().release()); } if (bNotify && m_pFormNotify) { diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp index 66abde63a8..93ef9dc366 100644 --- a/core/fpdfdoc/cpvt_generateap.cpp +++ b/core/fpdfdoc/cpvt_generateap.cpp @@ -186,7 +186,8 @@ bool GenerateWidgetAP(CPDF_Document* pDoc, pStreamResFontList->SetReferenceFor(sFontName, pDoc, pFontDict->GetObjNum()); } else { - pStreamDict->SetFor("Resources", pFormDict->GetDictFor("DR")->Clone()); + pStreamDict->SetFor("Resources", + pFormDict->GetDictFor("DR")->Clone().release()); pStreamResList = pStreamDict->GetDictFor("Resources"); } } @@ -437,7 +438,8 @@ bool GenerateWidgetAP(CPDF_Document* pDoc, pStreamResFontList->SetReferenceFor(sFontName, pDoc, pFontDict->GetObjNum()); } else { - pStreamDict->SetFor("Resources", pFormDict->GetDictFor("DR")->Clone()); + pStreamDict->SetFor("Resources", + pFormDict->GetDictFor("DR")->Clone().release()); pStreamResList = pStreamDict->GetDictFor("Resources"); } } diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp index 6cffbe0b31..f39a50aad4 100644 --- a/fpdfsdk/fpdf_flatten.cpp +++ b/fpdfsdk/fpdf_flatten.cpp @@ -408,8 +408,9 @@ DLLEXPORT int STDCALL FPDFPage_Flatten(FPDF_PAGE page, int nFlag) { CPDF_Object* pObj = pAPStream; if (pObj->IsInline()) { - pObj = pObj->Clone(); - pDocument->AddIndirectObject(pObj); + std::unique_ptr pNew = pObj->Clone(); + pObj = pNew.get(); + pDocument->AddIndirectObject(pNew.release()); } CPDF_Dictionary* pObjDic = pObj->GetDict(); diff --git a/fpdfsdk/fpdfppo.cpp b/fpdfsdk/fpdfppo.cpp index ccfd141db2..f8b96de1f3 100644 --- a/fpdfsdk/fpdfppo.cpp +++ b/fpdfsdk/fpdfppo.cpp @@ -109,7 +109,7 @@ bool CPDF_PageOrganizer::ExportPage(CPDF_Document* pSrcPDFDoc, if (cbSrcKeyStr.Compare(("Type")) && cbSrcKeyStr.Compare(("Parent"))) { if (pCurPageDict->KeyExist(cbSrcKeyStr)) pCurPageDict->RemoveFor(cbSrcKeyStr); - pCurPageDict->SetFor(cbSrcKeyStr, pObj->Clone()); + pCurPageDict->SetFor(cbSrcKeyStr, pObj->Clone().release()); } } @@ -123,7 +123,7 @@ bool CPDF_PageOrganizer::ExportPage(CPDF_Document* pSrcPDFDoc, // if not exists,we take the letter size. pInheritable = PageDictGetInheritableTag(pSrcPageDict, "CropBox"); if (pInheritable) { - pCurPageDict->SetFor("MediaBox", pInheritable->Clone()); + pCurPageDict->SetFor("MediaBox", pInheritable->Clone().release()); } else { // Make the default size to be letter size (8.5'x11') CPDF_Array* pArray = new CPDF_Array; @@ -134,7 +134,7 @@ bool CPDF_PageOrganizer::ExportPage(CPDF_Document* pSrcPDFDoc, pCurPageDict->SetFor("MediaBox", pArray); } } else { - pCurPageDict->SetFor("MediaBox", pInheritable->Clone()); + pCurPageDict->SetFor("MediaBox", pInheritable->Clone().release()); } } // 2 Resources //required @@ -142,27 +142,25 @@ bool CPDF_PageOrganizer::ExportPage(CPDF_Document* pSrcPDFDoc, pInheritable = PageDictGetInheritableTag(pSrcPageDict, "Resources"); if (!pInheritable) return false; - pCurPageDict->SetFor("Resources", pInheritable->Clone()); + pCurPageDict->SetFor("Resources", pInheritable->Clone().release()); } // 3 CropBox //Optional if (!pCurPageDict->KeyExist("CropBox")) { pInheritable = PageDictGetInheritableTag(pSrcPageDict, "CropBox"); if (pInheritable) - pCurPageDict->SetFor("CropBox", pInheritable->Clone()); + pCurPageDict->SetFor("CropBox", pInheritable->Clone().release()); } // 4 Rotate //Optional if (!pCurPageDict->KeyExist("Rotate")) { pInheritable = PageDictGetInheritableTag(pSrcPageDict, "Rotate"); if (pInheritable) - pCurPageDict->SetFor("Rotate", pInheritable->Clone()); + pCurPageDict->SetFor("Rotate", pInheritable->Clone().release()); } // Update the reference uint32_t dwOldPageObj = pSrcPageDict->GetObjNum(); uint32_t dwNewPageObj = pCurPageDict->GetObjNum(); - (*pObjNumberMap)[dwOldPageObj] = dwNewPageObj; - UpdateReference(pCurPageDict, pDestPDFDoc, pObjNumberMap.get()); ++curpage; } @@ -277,29 +275,22 @@ uint32_t CPDF_PageOrganizer::GetNewObjId(CPDF_Document* pDoc, if (!pDirect) return 0; - CPDF_Object* pClone = pDirect->Clone(); - if (!pClone) - return 0; - + std::unique_ptr pClone = pDirect->Clone(); if (CPDF_Dictionary* pDictClone = pClone->AsDictionary()) { if (pDictClone->KeyExist("Type")) { CFX_ByteString strType = pDictClone->GetStringFor("Type"); - if (!FXSYS_stricmp(strType.c_str(), "Pages")) { - delete pDictClone; + if (!FXSYS_stricmp(strType.c_str(), "Pages")) return 4; - } - if (!FXSYS_stricmp(strType.c_str(), "Page")) { - delete pDictClone; + if (!FXSYS_stricmp(strType.c_str(), "Page")) return 0; - } } } - dwNewObjNum = pDoc->AddIndirectObject(pClone); + dwNewObjNum = pDoc->AddIndirectObject(pClone.get()); (*pObjNumberMap)[dwObjnum] = dwNewObjNum; - if (!UpdateReference(pClone, pDoc, pObjNumberMap)) { - delete pClone; + if (!UpdateReference(pClone.get(), pDoc, pObjNumberMap)) return 0; - } + + pClone.release(); // TODO(tsepez): figure out ownership. return dwNewObjNum; } @@ -400,6 +391,7 @@ DLLEXPORT FPDF_BOOL STDCALL FPDF_CopyViewerPreferences(FPDF_DOCUMENT dest_doc, if (!pDstDict) return false; - pDstDict->SetFor("ViewerPreferences", pSrcDict->CloneDirectObject()); + pDstDict->SetFor("ViewerPreferences", + pSrcDict->CloneDirectObject().release()); return true; } -- cgit v1.2.3