Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.

Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.

Committed: https://pdfium.googlesource.com/pdfium/+/931d087b991a986cc8bfa72131fe8eda0d987fc4

[Tom Sepez, 2016-11-04 19:25:29]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[Tom Sepez, 2016-11-04 19:25:29]

Lei, follow-on set up by previous cl.

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_da...
File core/fpdfapi/parser/cpdf_data_avail.cpp (left):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_da...
core/fpdfapi/parser/cpdf_data_avail.cpp:1770: if (pPageDict) {
RIOIHG() does this check for us.

[Tom Sepez, 2016-11-04 20:00:35]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 20:00:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 20:15:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 20:15:52]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-04 22:04:37]

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
File core/fpdfapi/parser/cfdf_document.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
core/fpdfapi/parser/cfdf_document.cpp:72: auto pObj =
pdfium::WrapUnique<CPDF_Object>(
Would love it if CPDF_SyntaxParser::GetObject() can return an unique_ptr.

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ar...
File core/fpdfapi/parser/cpdf_array_unittest.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ar...
core/fpdfapi/parser/cpdf_array_unittest.cpp:111: CPDF_Number* obj = new
CPDF_Number(elems[i][j]);
Make this unique_ptr?

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ob...
File core/fpdfapi/parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ob...
core/fpdfapi/parser/cpdf_object_unittest.cpp:728:
arr1->AddReference(holder.get(), indirect_objs[i]->GetObjNum());
Didn't we just give away |indirect_objs[i]| above?

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_pa...
File core/fpdfapi/parser/cpdf_parser.cpp (left):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_pa...
core/fpdfapi/parser/cpdf_parser.cpp:959: // Takes ownership of object (std::move
someday).
Today is someday. :)

[Tom Sepez, 2016-11-04 22:44:03]

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
File core/fpdfapi/parser/cfdf_document.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
core/fpdfapi/parser/cfdf_document.cpp:72: auto pObj =
pdfium::WrapUnique<CPDF_Object>(
On 2016/11/04 22:04:36, Lei Zhang wrote:
> Would love it if CPDF_SyntaxParser::GetObject() can return an unique_ptr.

To do as a followup. This is an interim step. The ripples spread pretty far when
that change is made.

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ar...
File core/fpdfapi/parser/cpdf_array_unittest.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ar...
core/fpdfapi/parser/cpdf_array_unittest.cpp:111: CPDF_Number* obj = new
CPDF_Number(elems[i][j]);
On 2016/11/04 22:04:36, Lei Zhang wrote:
> Make this unique_ptr?

Done.

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ob...
File core/fpdfapi/parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cpdf_ob...
core/fpdfapi/parser/cpdf_object_unittest.cpp:728:
arr1->AddReference(holder.get(), indirect_objs[i]->GetObjNum());
On 2016/11/04 22:04:36, Lei Zhang wrote:
> Didn't we just give away |indirect_objs[i]| above?

No, that's why WrapUnique() has to be viewed with suspicion, in that it can make
a unique_ptr to things it doesn't really own. indirect_objs[i] is to be treated
like an unowned reference.

WrapUnique should go away in a subsequent refactoring.

[Tom Sepez, 2016-11-04 22:44:26]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 22:44:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Lei Zhang, 2016-11-04 22:48:53]

lgtm

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
File core/fpdfapi/parser/cfdf_document.cpp (right):

https://codereview.chromium.org/2477003002/diff/1/core/fpdfapi/parser/cfdf_do...
core/fpdfapi/parser/cfdf_document.cpp:72: auto pObj =
pdfium::WrapUnique<CPDF_Object>(
On 2016/11/04 22:44:03, Tom Sepez wrote:
> On 2016/11/04 22:04:36, Lei Zhang wrote:
> > Would love it if CPDF_SyntaxParser::GetObject() can return an unique_ptr.
> 
> To do as a followup. This is an interim step. The ripples spread pretty far
when
> that change is made.

Sure. I wasn't suggesting doing it here.

[Tom Sepez, 2016-11-04 22:50:39]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2016-11-04 22:50:42]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-11-04 22:50:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 23:00:05]

Description was changed from

==========
Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.
==========

to

==========
Pass object to ReplaceIndirectObjectIfHigherGeneration() by unique_ptr.

Committed:
https://pdfium.googlesource.com/pdfium/+/931d087b991a986cc8bfa72131fe8eda0d98...
==========

[commit-bot: I haz the power, 2016-11-04 23:00:06]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/931d087b991a986cc8bfa72131fe8eda0d98...