Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.

Review-Url: https://codereview.chromium.org/2477003002

8 files changed, 24 insertions, 26 deletions

diff --git a/core/fpdfapi/parser/cfdf_document.cpp b/core/fpdfapi/parser/cfdf_document.cpp
index e66bfeb12b..a0f3e1490d 100644
--- a/core/fpdfapi/parser/cfdf_document.cpp
+++ b/core/fpdfapi/parser/cfdf_document.cpp
@@ -69,11 +69,12 @@ void CFDF_Document::ParseStream(IFX_SeekableReadStream* pFile, bool bOwnFile) {
       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 800afb0f9a..b1a4605666 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 @@ TEST(cpdf_array, Clone) {
     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 eadbf1e828..c4ed95e17f 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -1763,13 +1763,11 @@ CPDF_Dictionary* CPDF_DataAvail::GetPage(int index) {
   // 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 720fe18606..b2cb08c08a 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -53,18 +53,17 @@ uint32_t CPDF_IndirectObjectHolder::AddIndirectObject(CPDF_Object* pObj) {
 
 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 83a31535c4..fcd77215eb 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -28,8 +28,9 @@ class CPDF_IndirectObjectHolder {
 
   // 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 a5efc1423e..717c2b7887 100644
--- a/core/fpdfapi/parser/cpdf_number.h
+++ b/core/fpdfapi/parser/cpdf_number.h
@@ -17,6 +17,7 @@ class CPDF_Number : public CPDF_Object {
   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 @@ class CPDF_Number : public CPDF_Object {
   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 64dc8c63ec..1bcf6164be 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -722,9 +722,8 @@ TEST(PDFArrayTest, AddReferenceAndGetObjectAt) {
   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 91f157c269..c8c07bd0b7 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -956,9 +956,8 @@ bool CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, bool bMainXRef) {
     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;
     }
   }