Make AddIndirectObject() take a unique_ptr.

Make AddIndirectObject() take a unique_ptr.

Add convenience routines to create and add object in
one step.

Committed: https://pdfium.googlesource.com/pdfium/+/70c4afd5c3c5c44cd24f814423a23a6ef05bba02

[Tom Sepez, 2016-11-14 23:25:04]

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

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

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-14 23:27:39]

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

[commit-bot: I haz the power, 2016-11-14 23:27:40]

Dry run: Try jobs failed on following builders:
  win on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win/builds/2820)
  win_no_v8 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_no_v8/build...)
  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-11-14 23:31:53]

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

[commit-bot: I haz the power, 2016-11-14 23:31:57]

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-14 23:36:47]

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

[commit-bot: I haz the power, 2016-11-14 23:36:47]

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

[Tom Sepez, 2016-11-14 23:37:51]

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

[commit-bot: I haz the power, 2016-11-14 23:37:57]

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-11-14 23:40:00]

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

[commit-bot: I haz the power, 2016-11-14 23:40:07]

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-14 23:50:37]

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

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

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

[Tom Sepez, 2016-11-14 23:54:42]

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

[commit-bot: I haz the power, 2016-11-14 23:54:47]

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-15 00:03:32]

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

[commit-bot: I haz the power, 2016-11-15 00:03:32]

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

[Tom Sepez, 2016-11-15 17:53:10]

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

[commit-bot: I haz the power, 2016-11-15 17:53:16]

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-15 18:09:43]

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

[commit-bot: I haz the power, 2016-11-15 18:09:44]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-15 18:19:02]

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

[Tom Sepez, 2016-11-15 18:19:03]

Dan, for review.  

For context, this is a partial step, and things get cleaner once the
dictionary/arrays/streams required unique_ptrs too.  At that point the ugly
.releases() go away.

[dsinclair, 2016-11-15 18:41:07]

Description was changed from

==========
Make AddIndirectObject() take a unique_ptr.

Add convenience routines to create and add object in
one step.

BUG=
==========

to

==========
Make AddIndirectObject() take a unique_ptr.

Add convenience routines to create and add object in
one step.
==========

[dsinclair, 2016-11-15 18:55:51]

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/edit/cpdf...
File core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/edit/cpdf...
core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp:137: CFX_ByteString name =
RealizeResource(pStream->GetObjNum(), "XObject");
Why does this one need the extra ->GetObjNum when the others don't?

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/page/cpdf...
File core/fpdfapi/page/cpdf_image.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/page/cpdf...
core/fpdfapi/page/cpdf_image.cpp:231: pCS->AddReference(m_pDocument,
pCTS->GetObjNum());
ditto

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
File core/fpdfapi/parser/cpdf_document.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_document.cpp:881: auto pBaseDict =
pdfium::MakeUnique<CPDF_Dictionary>(m_pByteStringPool);
Why not NewIndirect here?

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_document.cpp:1013: auto pBaseDict =
pdfium::MakeUnique<CPDF_Dictionary>(m_pByteStringPool);
ditto

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
File core/fpdfapi/parser/cpdf_indirect_object_holder.h (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_indirect_object_holder.h:30: // Creates and adds a new
ojbect, returns unowned pointer to it.
Returns unowned pointer is a bit confusing. Ouowned by the indirect object
holder, or unowned by the caller?

Maybe // Creates and adds a new object, the returned object is owned by the
indirect object holder?

https://codereview.chromium.org/2489283003/diff/160001/fpdfsdk/cpdfsdk_baanno...
File fpdfsdk/cpdfsdk_baannot.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/fpdfsdk/cpdfsdk_baanno...
fpdfsdk/cpdfsdk_baannot.cpp:355: CPDF_Document* pDoc =
m_pPageView->GetPDFDocument();  // OWNERSHIP ???
not sure what the comment means?

[Tom Sepez, 2016-11-15 19:15:17]

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/edit/cpdf...
File core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/edit/cpdf...
core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp:137: CFX_ByteString name =
RealizeResource(pStream->GetObjNum(), "XObject");
On 2016/11/15 18:55:51, dsinclair wrote:
> Why does this one need the extra ->GetObjNum when the others don't?

which other ones?  RealizeResource() only takes object numbers now.

$ git grep RealizeResource
core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp:CFX_ByteString
CPDF_PageContentGenerator::RealizeResource(
core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp:  CFX_ByteString name =
RealizeResource(dwObjNum, "XObject");
core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp:  CFX_ByteString name =
RealizeResource(pStream->GetObjNum(), "XObject");
core/fpdfapi/edit/cpdf_pagecontentgenerator.h:  CFX_ByteString
RealizeResource(uint32_t dwResourceObjNum,

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/page/cpdf...
File core/fpdfapi/page/cpdf_image.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/page/cpdf...
core/fpdfapi/page/cpdf_image.cpp:231: pCS->AddReference(m_pDocument,
pCTS->GetObjNum());
On 2016/11/15 18:55:51, dsinclair wrote:
> ditto

Done.

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
File core/fpdfapi/parser/cpdf_document.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_document.cpp:881: auto pBaseDict =
pdfium::MakeUnique<CPDF_Dictionary>(m_pByteStringPool);
On 2016/11/15 18:55:51, dsinclair wrote:
> Why not NewIndirect here?

Ok, I guess we always add it (no return before line 923).  Done

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_document.cpp:1013: auto pBaseDict =
pdfium::MakeUnique<CPDF_Dictionary>(m_pByteStringPool);
On 2016/11/15 18:55:51, dsinclair wrote:
> ditto

Done.

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
File core/fpdfapi/parser/cpdf_indirect_object_holder.h (right):

https://codereview.chromium.org/2489283003/diff/160001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_indirect_object_holder.h:30: // Creates and adds a new
ojbect, returns unowned pointer to it.
On 2016/11/15 18:55:51, dsinclair wrote:
> Returns unowned pointer is a bit confusing. Ouowned by the indirect object
> holder, or unowned by the caller?
> 
> Maybe // Creates and adds a new object, the returned object is owned by the
> indirect object holder?

Done.

https://codereview.chromium.org/2489283003/diff/160001/fpdfsdk/cpdfsdk_baanno...
File fpdfsdk/cpdfsdk_baannot.cpp (right):

https://codereview.chromium.org/2489283003/diff/160001/fpdfsdk/cpdfsdk_baanno...
fpdfsdk/cpdfsdk_baannot.cpp:355: CPDF_Document* pDoc =
m_pPageView->GetPDFDocument();  // OWNERSHIP ???
On 2016/11/15 18:55:51, dsinclair wrote:
> not sure what the comment means?

It was a reminder for me to check this again. Removed, and updated the logic a
bit to not clone unless inline.

[Tom Sepez, 2016-11-15 19:15:29]

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

[commit-bot: I haz the power, 2016-11-15 19:15:31]

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

[dsinclair, 2016-11-15 19:21:34]

lgtm

[Tom Sepez, 2016-11-15 19:24:05]

The CQ bit was unchecked by tsepez@chromium.org

[Tom Sepez, 2016-11-15 19:24:14]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-11-15 19:24:22]

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-15 19:33:47]

Description was changed from

==========
Make AddIndirectObject() take a unique_ptr.

Add convenience routines to create and add object in
one step.
==========

to

==========
Make AddIndirectObject() take a unique_ptr.

Add convenience routines to create and add object in
one step.

Committed:
https://pdfium.googlesource.com/pdfium/+/70c4afd5c3c5c44cd24f814423a23a6ef05b...
==========

[commit-bot: I haz the power, 2016-11-15 19:33:49]

Committed patchset #10 (id:180001) as
https://pdfium.googlesource.com/pdfium/+/70c4afd5c3c5c44cd24f814423a23a6ef05b...