Add a Retained Pointer smart class.

Add a Retained Pointer smart class.

It's going to be hard to fix some XFA object leaks without
this. Put this in fxcrt/ as we should be able to make the
FX strings take advantage of this.

BUG=pdfium:55
Committed: https://pdfium.googlesource.com/pdfium/+/602aebc11a615971800d38f8d427a4e4f95c1134

[Tom Sepez, 2016-03-15 21:47:03]

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

[Tom Sepez, 2016-03-15 21:47:03]

Dan, for review.

[Tom Sepez, 2016-03-15 21:59:37]

Description was changed from

==========
Add a Retained Pointer smart class.

It's going to be hard to fix some XFA object leaks without
this. Put this in fxcrt/ as we should be able to make the
FX strings take advantage of this.

BUG=pdfium:55
==========

to

==========
Add a Retained Pointer smart class.

It's going to be hard to fix some XFA object leaks without
this. Put this in fxcrt/ as we should be able to make the
FX strings take advantage of this.

BUG=pdfium:55
==========

[Tom Sepez, 2016-03-15 21:59:37]

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

[dsinclair, 2016-03-16 13:47:45]

https://codereview.chromium.org/1805683002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1805683002/diff/40001/BUILD.gn#newcode541
BUILD.gn:541: "core/include/fxcrt/cfx_retain_ptr.h",
Can we put this in core/fxcrt/include?

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
File core/include/fxcrt/cfx_retain_ptr.h (right):

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
core/include/fxcrt/cfx_retain_ptr.h:28: void Reset(T* obj = NULL) {
nit: nullptr

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
core/include/fxcrt/cfx_retain_ptr.h:55: protected:
Why not private?

[Tom Sepez, 2016-03-16 19:12:56]

https://codereview.chromium.org/1805683002/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1805683002/diff/40001/BUILD.gn#newcode541
BUILD.gn:541: "core/include/fxcrt/cfx_retain_ptr.h",
On 2016/03/16 13:47:45, dsinclair wrote:
> Can we put this in core/fxcrt/include?

Done.

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
File core/include/fxcrt/cfx_retain_ptr.h (right):

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
core/include/fxcrt/cfx_retain_ptr.h:28: void Reset(T* obj = NULL) {
On 2016/03/16 13:47:45, dsinclair wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1805683002/diff/40001/core/include/fxcrt/cfx_...
core/include/fxcrt/cfx_retain_ptr.h:55: protected:
On 2016/03/16 13:47:45, dsinclair wrote:
> Why not private?

Done.

[dsinclair, 2016-03-17 01:30:29]

lgtm

Sorry, thought I'd lgtm'd this one.

[dsinclair, 2016-03-22 20:33:18]

Were you still planning on landing this one?

[Tom Sepez, 2016-03-22 20:38:06]

On 2016/03/22 20:33:18, dsinclair wrote:
> Were you still planning on landing this one?

Once Lei gets back and has a look.

[Lei Zhang, 2016-03-29 18:36:36]

lgtm, but needs rebasing.

https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
File core/fxcrt/include/cfx_retain_ptr.h (right):

https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
core/fxcrt/include/cfx_retain_ptr.h:28: void Reset(T* obj = nullptr) {
Would it be slightly cheaper to do this instead to avoid creating |tmp| ?

if (obj)
  obj->Retain();
m_pObj.reset(obj)

[Tom Sepez, 2016-03-29 21:32:14]

On 2016/03/29 18:36:36, Lei Zhang wrote:
> lgtm, but needs rebasing.
> 
>
https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
> File core/fxcrt/include/cfx_retain_ptr.h (right):
> 
>
https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
> core/fxcrt/include/cfx_retain_ptr.h:28: void Reset(T* obj = nullptr) {
> Would it be slightly cheaper to do this instead to avoid creating |tmp| ?
> 
> if (obj)
>   obj->Retain();
> m_pObj.reset(obj)

You're back !!!  Thanks.

[Tom Sepez, 2016-03-29 21:37:23]

https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
File core/fxcrt/include/cfx_retain_ptr.h (right):

https://codereview.chromium.org/1805683002/diff/80001/core/fxcrt/include/cfx_...
core/fxcrt/include/cfx_retain_ptr.h:28: void Reset(T* obj = nullptr) {
On 2016/03/29 18:36:36, Lei Zhang wrote:
> Would it be slightly cheaper to do this instead to avoid creating |tmp| ?
> 
> if (obj)
>   obj->Retain();
> m_pObj.reset(obj)

Done.

[Tom Sepez, 2016-03-29 21:37:31]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-03-29 21:37:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/1805683002/#ps100001
(title: "Rebase, improve Reset().")

[commit-bot: I haz the power, 2016-03-29 21:37:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1805683002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1805683002/100001

[commit-bot: I haz the power, 2016-03-29 22:04:24]

Description was changed from

==========
Add a Retained Pointer smart class.

It's going to be hard to fix some XFA object leaks without
this. Put this in fxcrt/ as we should be able to make the
FX strings take advantage of this.

BUG=pdfium:55
==========

to

==========
Add a Retained Pointer smart class.

It's going to be hard to fix some XFA object leaks without
this. Put this in fxcrt/ as we should be able to make the
FX strings take advantage of this.

BUG=pdfium:55

Committed:
https://pdfium.googlesource.com/pdfium/+/602aebc11a615971800d38f8d427a4e4f95c...
==========

[commit-bot: I haz the power, 2016-03-29 22:04:25]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/602aebc11a615971800d38f8d427a4e4f95c...