Update CPDF_IndirectObjectHolder APIs for unique objects

Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
Add convenience functions to return an unowned reference to
a newly-created indirect object.

Committed: https://pdfium.googlesource.com/pdfium/+/3ba098595ae56b64eacc0c25ab76b89a4d78d920

[Tom Sepez, 2016-10-14 21:28:29]

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

[commit-bot: I haz the power, 2016-10-14 21:28:37]

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-10-14 21:32:56]

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

[commit-bot: I haz the power, 2016-10-14 21:32:57]

Dry run: Try jobs failed on following builders:
  win_xfa_clang on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_clang/b...)

[Tom Sepez, 2016-10-14 21:56:08]

Description was changed from

==========
Update CPDF_IndirectObjectHolder APIs for unique objects
==========

to

==========
Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
==========

[Tom Sepez, 2016-10-14 21:56:08]

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

[Tom Sepez, 2016-10-14 21:56:20]

Lei, for review.

[Tom Sepez, 2016-10-14 21:57:12]

Description was changed from

==========
Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
==========

to

==========
Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
Add convenience functions to return an unowned reference to
a newly-created indirect object.
==========

[Tom Sepez, 2016-10-14 22:02:21]

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

[commit-bot: I haz the power, 2016-10-14 22:02:24]

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-10-14 22:49:21]

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

[commit-bot: I haz the power, 2016-10-14 22:49:22]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-10-14 23:18:05]

Still reading.

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cfd...
File core/fpdfapi/parser/cfdf_document.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cfd...
core/fpdfapi/parser/cfdf_document.cpp:72: UniqueObject(pObj));  // Owner?
There's a few of these "// Owner?" comments. Should they be converted to TODOs?

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_array.h (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_array.h:57: void
AddReference(CPDF_IndirectObjectHolder* pDoc, const CPDF_Object* objnum);
s/objnum/pObj/ ?

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_array.h:57: void
AddReference(CPDF_IndirectObjectHolder* pDoc, const CPDF_Object* objnum);
Is there any advantage to adding this, versus making the caller call
AddReference(pDoc, pObj->GetObjNum()); ? Convenience?

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_indirect_object_holder.cpp:77: uint8_t* pData,
Parameters are not used?

[Lei Zhang, 2016-10-14 23:24:39]

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_document.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_document.cpp:832: CPDF_Object* pUnowned =
AddIndirectObject(std::move(pFontDict));
Can we do CPDF_Dictionary* pFontDict = AddIndirectDictionary(m_pByteStringPool),
and then set the |pFontDict| values?

[Tom Sepez, 2016-10-14 23:42:27]

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

[commit-bot: I haz the power, 2016-10-14 23:42:28]

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

[Tom Sepez, 2016-10-14 23:42:29]

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cfd...
File core/fpdfapi/parser/cfdf_document.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cfd...
core/fpdfapi/parser/cfdf_document.cpp:72: UniqueObject(pObj));  // Owner?
On 2016/10/14 23:18:04, Lei Zhang wrote:
> There's a few of these "// Owner?" comments. Should they be converted to
TODOs?

Done.

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_array.h (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_array.h:57: void
AddReference(CPDF_IndirectObjectHolder* pDoc, const CPDF_Object* objnum);
On 2016/10/14 23:18:04, Lei Zhang wrote:
> s/objnum/pObj/ ?

Done.

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_array.h:57: void
AddReference(CPDF_IndirectObjectHolder* pDoc, const CPDF_Object* objnum);
On 2016/10/14 23:18:04, Lei Zhang wrote:
> Is there any advantage to adding this, versus making the caller call
> AddReference(pDoc, pObj->GetObjNum()); ? Convenience?

Convenience, also probably codesize as we replace N scattered calls to this with
one consolidated one.

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_document.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_document.cpp:832: CPDF_Object* pUnowned =
AddIndirectObject(std::move(pFontDict));
On 2016/10/14 23:24:38, Lei Zhang wrote:
> Can we do CPDF_Dictionary* pFontDict =
AddIndirectDictionary(m_pByteStringPool),
> and then set the |pFontDict| values?

Done.

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/2419173002/diff/60001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_indirect_object_holder.cpp:77: uint8_t* pData,
On 2016/10/14 23:18:04, Lei Zhang wrote:
> Parameters are not used?

Argh.  I have some template foo coming to avoid this.  Fixed.

[Lei Zhang, 2016-10-14 23:45:25]

lgtm

[Tom Sepez, 2016-10-14 23:49:59]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2016-10-14 23:50:04]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-10-14 23:50:10]

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-10-14 23:59:13]

Description was changed from

==========
Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
Add convenience functions to return an unowned reference to
a newly-created indirect object.
==========

to

==========
Update CPDF_IndirectObjectHolder APIs for unique objects.

Doing so highlights a few places where ownership is dubious.
Add convenience functions to return an unowned reference to
a newly-created indirect object.

Committed:
https://pdfium.googlesource.com/pdfium/+/3ba098595ae56b64eacc0c25ab76b89a4d78...
==========

[commit-bot: I haz the power, 2016-10-14 23:59:14]

Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/3ba098595ae56b64eacc0c25ab76b89a4d78...