Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.
Review-Url: https://codereview.chromium.org/2477003002
diff --git a/core/fpdfapi/parser/cfdf_document.cpp b/core/fpdfapi/parser/cfdf_document.cpp
index e66bfeb..a0f3e14 100644
--- a/core/fpdfapi/parser/cfdf_document.cpp
+++ b/core/fpdfapi/parser/cfdf_document.cpp
@@ -69,11 +69,12 @@
if (word != "obj")
break;
- CPDF_Object* pObj = parser.GetObject(this, objnum, 0, true);
+ auto pObj = pdfium::WrapUnique<CPDF_Object>(
+ parser.GetObject(this, objnum, 0, true));
if (!pObj)
break;
- ReplaceIndirectObjectIfHigherGeneration(objnum, pObj);
+ ReplaceIndirectObjectIfHigherGeneration(objnum, std::move(pObj));
word = parser.GetNextWord(nullptr);
if (word != "endobj")
break;
diff --git a/core/fpdfapi/parser/cpdf_array_unittest.cpp b/core/fpdfapi/parser/cpdf_array_unittest.cpp
index 800afb0..b1a4605 100644
--- a/core/fpdfapi/parser/cpdf_array_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_array_unittest.cpp
@@ -3,12 +3,13 @@
// found in the LICENSE file.
#include "core/fpdfapi/parser/cpdf_array.h"
-#include "core/fpdfapi/parser/cpdf_number.h"
-#include "core/fpdfapi/parser/cpdf_reference.h"
#include <memory>
+#include "core/fpdfapi/parser/cpdf_number.h"
+#include "core/fpdfapi/parser/cpdf_reference.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/base/ptr_util.h"
TEST(cpdf_array, RemoveAt) {
{
@@ -107,10 +108,11 @@
for (size_t i = 0; i < kNumOfRows; ++i) {
CPDF_Array* arr_elem = new CPDF_Array;
for (size_t j = 0; j < kNumOfRowElems; ++j) {
- CPDF_Number* obj = new CPDF_Number(elems[i][j]);
+ std::unique_ptr<CPDF_Number> obj(new CPDF_Number(elems[i][j]));
// Starts object number from 1.
int obj_num = i * kNumOfRowElems + j + 1;
- obj_holder->ReplaceIndirectObjectIfHigherGeneration(obj_num, obj);
+ obj_holder->ReplaceIndirectObjectIfHigherGeneration(obj_num,
+ std::move(obj));
arr_elem->InsertAt(j, new CPDF_Reference(obj_holder.get(), obj_num));
}
arr->InsertAt(i, arr_elem);
diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp
index eadbf1e..c4ed95e 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -1763,13 +1763,11 @@
// We should say to the document, which object is the page.
m_pDocument->SetPageObjNum(index, dwObjNum);
// Page object already can be parsed in document.
- CPDF_Object* pPageDict = m_pDocument->GetIndirectObject(dwObjNum);
- if (!pPageDict) {
+ if (!m_pDocument->GetIndirectObject(dwObjNum)) {
m_syntaxParser.InitParser(m_pFileRead, (uint32_t)szPageStartPos);
- pPageDict = ParseIndirectObjectAt(0, dwObjNum, m_pDocument);
- if (pPageDict) {
- m_pDocument->ReplaceIndirectObjectIfHigherGeneration(dwObjNum, pPageDict);
- }
+ m_pDocument->ReplaceIndirectObjectIfHigherGeneration(
+ dwObjNum, pdfium::WrapUnique<CPDF_Object>(
+ ParseIndirectObjectAt(0, dwObjNum, m_pDocument)));
}
return m_pDocument->GetPage(index);
}
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
index 720fe18..b2cb08c 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -53,18 +53,17 @@
bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration(
uint32_t objnum,
- CPDF_Object* pObj) {
+ std::unique_ptr<CPDF_Object> pObj) {
ASSERT(objnum);
if (!pObj)
return false;
CPDF_Object* pOldObj = GetIndirectObject(objnum);
- if (pOldObj && pObj->GetGenNum() <= pOldObj->GetGenNum()) {
- delete pObj;
+ if (pOldObj && pObj->GetGenNum() <= pOldObj->GetGenNum())
return false;
- }
+
pObj->m_ObjNum = objnum;
- m_IndirectObjs[objnum].reset(pObj);
+ m_IndirectObjs[objnum] = std::move(pObj);
m_LastObjNum = std::max(m_LastObjNum, objnum);
return true;
}
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
index 83a3153..fcd7721 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -28,8 +28,9 @@
// Take ownership of |pObj|.
uint32_t AddIndirectObject(CPDF_Object* pObj);
- bool ReplaceIndirectObjectIfHigherGeneration(uint32_t objnum,
- CPDF_Object* pObj);
+ bool ReplaceIndirectObjectIfHigherGeneration(
+ uint32_t objnum,
+ std::unique_ptr<CPDF_Object> pObj);
uint32_t GetLastObjNum() const { return m_LastObjNum; }
void SetLastObjNum(uint32_t objnum) { m_LastObjNum = objnum; }
diff --git a/core/fpdfapi/parser/cpdf_number.h b/core/fpdfapi/parser/cpdf_number.h
index a5efc14..717c2b7 100644
--- a/core/fpdfapi/parser/cpdf_number.h
+++ b/core/fpdfapi/parser/cpdf_number.h
@@ -17,6 +17,7 @@
explicit CPDF_Number(int value);
explicit CPDF_Number(FX_FLOAT value);
explicit CPDF_Number(const CFX_ByteStringC& str);
+ ~CPDF_Number() override;
// CPDF_Object.
Type GetType() const override;
@@ -32,8 +33,6 @@
bool IsInteger() const { return m_bInteger; }
protected:
- ~CPDF_Number() override;
-
bool m_bInteger;
union {
int m_Integer;
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index 64dc8c6..1bcf616 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -722,9 +722,8 @@
std::unique_ptr<CPDF_Array> 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.
- holder->ReplaceIndirectObjectIfHigherGeneration(obj_nums[i],
- indirect_objs[i]);
+ holder->ReplaceIndirectObjectIfHigherGeneration(
+ obj_nums[i], pdfium::WrapUnique<CPDF_Object>(indirect_objs[i]));
arr->AddReference(holder.get(), obj_nums[i]);
arr1->AddReference(holder.get(), indirect_objs[i]->GetObjNum());
}
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index 91f157c..c8c07bd 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -956,9 +956,8 @@
CPDF_Dictionary* pRootDict = m_pDocument->GetRoot();
if (pRootDict && pRootDict->GetObjNum() == objnum)
return false;
- // Takes ownership of object (std::move someday).
if (!m_pDocument->ReplaceIndirectObjectIfHigherGeneration(
- objnum, pObject.release())) {
+ objnum, std::move(pObject))) {
return false;
}
}
