From 1d023881cd53485303c0fcc0b5878e700dc470fd Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 13 Oct 2016 16:36:20 -0700 Subject: Make CPDF_Object containers hold objects via unique pointers. This tweaks the implementation while leaving the API the same. The API change is more disruptive, so break this part off first. Review-Url: https://codereview.chromium.org/2385293002 --- core/fpdfapi/parser/cpdf_array.cpp | 44 ++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 25 deletions(-) (limited to 'core/fpdfapi/parser/cpdf_array.cpp') diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index 1aec3e06f3..807f62d7f3 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp @@ -20,11 +20,11 @@ CPDF_Array::CPDF_Array() {} CPDF_Array::~CPDF_Array() { // Mark the object as deleted so that it will not be deleted again - // in case of cyclic references. + // in case of cyclic references, then break cycles. m_ObjNum = kInvalidObjNum; for (auto& it : m_Objects) { - if (it && it->GetObjNum() != kInvalidObjNum) - it->Release(); + if (it && it->GetObjNum() == kInvalidObjNum) + it.release(); } } @@ -53,10 +53,11 @@ CPDF_Object* CPDF_Array::CloneNonCyclic( 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)); + for (const auto& pObj : m_Objects) { + if (!pdfium::ContainsKey(*pVisited, pObj.get())) { + pCopy->m_Objects.push_back( + UniqueObject(pObj->CloneNonCyclic(bDirect, pVisited))); + } } return pCopy; } @@ -86,7 +87,7 @@ CFX_Matrix CPDF_Array::GetMatrix() { CPDF_Object* CPDF_Array::GetObjectAt(size_t i) const { if (i >= m_Objects.size()) return nullptr; - return m_Objects[i]; + return m_Objects[i].get(); } CPDF_Object* CPDF_Array::GetDirectObjectAt(size_t i) const { @@ -139,11 +140,8 @@ 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) { - if (CPDF_Object* p = m_Objects[i + j]) - p->Release(); - } - m_Objects.erase(m_Objects.begin() + i, m_Objects.begin() + i + nCount); + auto it = m_Objects.begin() + i; + m_Objects.erase(it, it + nCount); } void CPDF_Array::ConvertToIndirectObjectAt(size_t i, @@ -151,12 +149,11 @@ void CPDF_Array::ConvertToIndirectObjectAt(size_t i, if (i >= m_Objects.size()) return; - CPDF_Object* pObj = m_Objects[i]; - if (!pObj || pObj->IsReference()) + if (!m_Objects[i] || m_Objects[i]->IsReference()) return; - uint32_t dwObjNum = pHolder->AddIndirectObject(pObj); - m_Objects[i] = new CPDF_Reference(pHolder, dwObjNum); + uint32_t dwObjNum = pHolder->AddIndirectObject(m_Objects[i].release()); + m_Objects[i] = UniqueObject(new CPDF_Reference(pHolder, dwObjNum)); } void CPDF_Array::SetAt(size_t i, CPDF_Object* pObj) { @@ -166,10 +163,7 @@ void CPDF_Array::SetAt(size_t i, CPDF_Object* pObj) { ASSERT(false); return; } - if (CPDF_Object* pOld = m_Objects[i]) - pOld->Release(); - - m_Objects[i] = pObj; + m_Objects[i] = UniqueObject(pObj); } void CPDF_Array::InsertAt(size_t index, CPDF_Object* pObj) { @@ -177,18 +171,18 @@ void CPDF_Array::InsertAt(size_t index, CPDF_Object* pObj) { CHECK(!pObj || pObj->IsInline()); if (index >= m_Objects.size()) { // Allocate space first. - m_Objects.resize(index + 1, nullptr); - m_Objects[index] = pObj; + m_Objects.resize(index + 1); + m_Objects[index] = UniqueObject(pObj); } else { // Directly insert. - m_Objects.insert(m_Objects.begin() + index, pObj); + m_Objects.insert(m_Objects.begin() + index, UniqueObject(pObj)); } } void CPDF_Array::Add(CPDF_Object* pObj) { ASSERT(IsArray()); CHECK(!pObj || pObj->IsInline()); - m_Objects.push_back(pObj); + m_Objects.push_back(UniqueObject(pObj)); } void CPDF_Array::AddName(const CFX_ByteString& str) { -- cgit v1.2.3