Remove CPDF_Object::Release() in favor of direct delete

Remove CPDF_Object::Release() in favor of direct delete

Follow-on once we prove Release always deletes in previous CL.

Committed: https://pdfium.googlesource.com/pdfium/+/4de3d095c9d9e961f93750cf1ebd489fd515be12

[Tom Sepez, 2016-10-01 01:43:17]

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

[commit-bot: I haz the power, 2016-10-01 01:43:19]

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-01 01:45:46]

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

[Tom Sepez, 2016-10-01 01:45:46]

This is where we want to go if the prior CL proves safety.

[Lei Zhang, 2016-10-01 01:50:37]

Yep, I figured and I'd like this to happen.

There's probably more opportunities to use std::unique_ptr, and avoid "if (foo)
{ delete foo; }".

[commit-bot: I haz the power, 2016-10-01 01:56:09]

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

[commit-bot: I haz the power, 2016-10-01 01:56:09]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-10-03 16:03:32]

On 2016/10/01 01:50:37, Lei Zhang wrote:
> Yep, I figured and I'd like this to happen.
> 
> There's probably more opportunities to use std::unique_ptr, and avoid "if
(foo)
> { delete foo; }".

yup, but once we start putting unique_ptr's as arguments, the CL gets big fast.
I'll just tidy up the locals where possible.

[Tom Sepez, 2016-10-03 16:44:23]

On 2016/10/03 16:03:32, Tom Sepez wrote:
> On 2016/10/01 01:50:37, Lei Zhang wrote:
> > Yep, I figured and I'd like this to happen.
> > 
> > There's probably more opportunities to use std::unique_ptr, and avoid "if
> (foo)
> > { delete foo; }".
> 
> yup, but once we start putting unique_ptr's as arguments, the CL gets big
fast.
> I'll just tidy up the locals where possible.
Nah, it gets messy, too, since we'd really want to pass these as args.  I'd
leave this as a mechanical chage.

[Lei Zhang, 2016-10-03 21:18:31]

https://codereview.chromium.org/2384883003/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2384883003/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:25: using ScopedArray =
std::unique_ptr<CPDF_Array>;
We can also just get rid of these.

[Tom Sepez, 2016-10-03 21:41:11]

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

[commit-bot: I haz the power, 2016-10-03 21:41:18]

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-03 21:41:31]

https://codereview.chromium.org/2384883003/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/2384883003/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:25: using ScopedArray =
std::unique_ptr<CPDF_Array>;
On 2016/10/03 21:18:30, Lei Zhang wrote:
> We can also just get rid of these.

Done.

[Lei Zhang, 2016-10-03 21:50:47]

lgtm

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_image.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_image.cpp:289: if (pDict) {
no if?

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_parser.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_parser.cpp:748: if (m_pTrailer)
no if

https://codereview.chromium.org/2384883003/diff/40001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine_unittest.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine_unittest.cpp:18: std::unique_ptr<CPDF_Dictionary>
dict_obj(new CPDF_Dictionary());
Want to start using MakeUnique?

[commit-bot: I haz the power, 2016-10-03 22:09:17]

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

[commit-bot: I haz the power, 2016-10-03 22:09:18]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-10-03 22:51:43]

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

[commit-bot: I haz the power, 2016-10-03 22:51:50]

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-03 22:55:00]

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

[commit-bot: I haz the power, 2016-10-03 22:55:01]

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

[Tom Sepez, 2016-10-05 21:03:03]

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_image.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_image.cpp:289: if (pDict) {
On 2016/10/03 21:50:47, Lei Zhang wrote:
> no if?

Done.

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_parser.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_parser.cpp:748: if (m_pTrailer)
On 2016/10/03 21:50:47, Lei Zhang wrote:
> no if

Done.

https://codereview.chromium.org/2384883003/diff/40001/core/fxge/dib/fx_dib_en...
File core/fxge/dib/fx_dib_engine_unittest.cpp (right):

https://codereview.chromium.org/2384883003/diff/40001/core/fxge/dib/fx_dib_en...
core/fxge/dib/fx_dib_engine_unittest.cpp:18: std::unique_ptr<CPDF_Dictionary>
dict_obj(new CPDF_Dictionary());
On 2016/10/03 21:50:47, Lei Zhang wrote:
> Want to start using MakeUnique?

Done here, other places will wait.

[Tom Sepez, 2016-10-05 21:03:12]

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

[commit-bot: I haz the power, 2016-10-05 21:03:15]

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-05 21:03:43]

Need to let the previous CL bake a bit to ensure check(0) doesn't trip on
canary, etc.

[Lei Zhang, 2016-10-05 21:09:25]

lgtm

https://codereview.chromium.org/2384883003/diff/120001/core/fpdfapi/parser/cp...
File core/fpdfapi/parser/cpdf_data_avail.cpp (right):

https://codereview.chromium.org/2384883003/diff/120001/core/fpdfapi/parser/cp...
core/fpdfapi/parser/cpdf_data_avail.cpp:1439: CPDF_Object* pPages =
GetObject(m_PagesObjNum, pHints, &bExist);
For later: unique_ptr.

[commit-bot: I haz the power, 2016-10-05 21:31:25]

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

[commit-bot: I haz the power, 2016-10-05 21:31:26]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-10-28 21:03:12]

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

[commit-bot: I haz the power, 2016-10-28 21:03:20]

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-28 21:08:26]

(Keeping this CL alive for now, CHECK() has hit dev, wait a day or so).

[commit-bot: I haz the power, 2016-10-28 21:18:29]

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

[commit-bot: I haz the power, 2016-10-28 21:18:30]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-03 20:20:23]

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

[commit-bot: I haz the power, 2016-11-03 20:20:26]

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-03 20:21:32]

Lei, permission to land?

[commit-bot: I haz the power, 2016-11-03 20:36:09]

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

[commit-bot: I haz the power, 2016-11-03 20:36:10]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-03 20:39:07]

On 2016/11/03 20:21:32, Tom Sepez wrote:
> Lei, permission to land?

(e.g. no faith in a 4 week old L_G_T_M)

[Lei Zhang, 2016-11-04 00:03:22]

Let's give it a go.

[Tom Sepez, 2016-11-04 00:04:40]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-04 00:04:40]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2384883003/#ps200001
(title: "rebase")

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

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 00:05:10]

Description was changed from

==========
Remove CPDF_Object::Release() in favor of direct delete

Follow-on once we prove Release always deletes in previous CL.
==========

to

==========
Remove CPDF_Object::Release() in favor of direct delete

Follow-on once we prove Release always deletes in previous CL.

Committed:
https://pdfium.googlesource.com/pdfium/+/4de3d095c9d9e961f93750cf1ebd489fd515...
==========

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

Committed patchset #11 (id:200001) as
https://pdfium.googlesource.com/pdfium/+/4de3d095c9d9e961f93750cf1ebd489fd515...

[dsinclair, 2016-11-04 15:25:08]

A revert of this CL (patchset #11 id:200001) has been created in
https://codereview.chromium.org/2478253002/ by dsinclair@chromium.org.

The reason for reverting is: Looks like it's blocking the roll.

https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium....