Add helper KeepRefUntilDone

Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of scope.

Committed: https://crrev.com/14f4144a82558ec4da2d4962ef02b23f44967b6a
Cr-Commit-Position: refs/heads/master@{#10847}

[perkj_webrtc, 2015-11-29 16:51:22]

perkj@webrtc.org changed reviewers:
+ magjed@webrtc.org, tommi@webrtc.org

[perkj_webrtc, 2015-11-29 16:51:22]

Please?

[mflodman, 2015-11-30 08:45:10]

Seen-by-coincidence-drive-by-nit

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
File webrtc/base/release_when_done_callback.h (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:2: *  Copyright 2004 The WebRTC Project
Authors. All rights reserved.
s/2004/2015

[tommi, 2015-11-30 08:45:51]

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest.cc
File webrtc/base/callback_unittest.cc (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest...
webrtc/base/callback_unittest.cc:119: cb2 = cb;
add EXPECT_EQ(2, t.RefCount()); ?

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
File webrtc/base/release_when_done_callback.h (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:29: Callback0<void>
ReleaseWhenDoneCallback(ObjectT* object) {
What's the difference between this implementation and something like:

ReleaseWhenDoneCallback(ObjectT* object) {
  object->AddRef();
  return rtc::Bind(&ObjectT::Release, object);
}

Actually, the name ReleaseWhenDoneCallback name kinda indicates that the
function will only Release and not AddRef.  What about KeepRefUntilDone?

[magjed_webrtc, 2015-11-30 09:22:18]

Update this place as well:
webrtc/common_video/video_frame_buffer.cc:
static void NoLongerUsedCallback(rtc::scoped_refptr<VideoFrameBuffer> dummy)

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
File webrtc/base/release_when_done_callback.h (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:22: void ReleaseRef(const
scoped_refptr<T>& object) {}
Should this be a static function?

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:29: Callback0<void>
ReleaseWhenDoneCallback(ObjectT* object) {
The argument type should be const scoped_refptr<ObjectT>&.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:29: Callback0<void>
ReleaseWhenDoneCallback(ObjectT* object) {
ditto: Should this be a static function?

[perkj_webrtc, 2015-11-30 12:54:20]

Please?

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest.cc
File webrtc/base/callback_unittest.cc (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest...
webrtc/base/callback_unittest.cc:119: cb2 = cb;
On 2015/11/30 08:45:51, tommi (webrtc) wrote:
> add EXPECT_EQ(2, t.RefCount()); ?

Actually no - it is still one. See callback. Each callback has |Helper| that is
ref counted itself.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
File webrtc/base/release_when_done_callback.h (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:2: *  Copyright 2004 The WebRTC Project
Authors. All rights reserved.
On 2015/11/30 08:45:10, mflodman wrote:
> s/2004/2015

Done.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:22: void ReleaseRef(const
scoped_refptr<T>& object) {}
On 2015/11/30 09:22:17, magjed_webrtc wrote:
> Should this be a static function?

Done.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:29: Callback0<void>
ReleaseWhenDoneCallback(ObjectT* object) {
On 2015/11/30 09:22:17, magjed_webrtc wrote:
> ditto: Should this be a static function?

Done.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/release_when_done...
webrtc/base/release_when_done_callback.h:29: Callback0<void>
ReleaseWhenDoneCallback(ObjectT* object) {
On 2015/11/30 08:45:51, tommi (webrtc) wrote:
> What's the difference between this implementation and something like:
> 
> ReleaseWhenDoneCallback(ObjectT* object) {
>   object->AddRef();
>   return rtc::Bind(&ObjectT::Release, object);
> }
> 
> Actually, the name ReleaseWhenDoneCallback name kinda indicates that the
> function will only Release and not AddRef.  What about KeepRefUntilDone?

Renamed.

The difference would be that the callback can in theory be executed mulitple
times.
ie 
cb = ReleaseWhenDoneCallback();
cb()  --> releases a ref
cb()  --> releases another ref...

[perkj_webrtc, 2015-11-30 12:54:33]

Patchset #2 (id:20001) has been deleted

[tommi, 2015-11-30 13:05:01]

lgtm.  I'm fine with the template version that you had before for
KeepRefUntilDone, but will leave it up to you.

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest.cc
File webrtc/base/callback_unittest.cc (right):

https://codereview.webrtc.org/1487493002/diff/1/webrtc/base/callback_unittest...
webrtc/base/callback_unittest.cc:119: cb2 = cb;
On 2015/11/30 12:54:19, perkj1 wrote:
> On 2015/11/30 08:45:51, tommi (webrtc) wrote:
> > add EXPECT_EQ(2, t.RefCount()); ?
> 
> Actually no - it is still one. See callback. Each callback has |Helper| that
is
> ref counted itself.

Brilliant

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/keep_ref_unti...
File webrtc/base/keep_ref_until_done.h (right):

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/keep_ref_unti...
webrtc/base/keep_ref_until_done.h:28: static inline Callback0<void>
KeepRefUntilDone(RefCountInterface* object) {
I actually kinda liked having the same compatibility as scoped_refptr does.

[magjed_webrtc, 2015-11-30 15:41:52]

lgtm

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/keep_ref_unti...
File webrtc/base/keep_ref_until_done.h (right):

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/keep_ref_unti...
webrtc/base/keep_ref_until_done.h:28: static inline Callback0<void>
KeepRefUntilDone(RefCountInterface* object) {
If you don't put this definition in a cc file, you might as well use a template
function. As long as it's static you shouldn't get any linker name conflicts.

[magjed_webrtc, 2015-11-30 15:44:34]

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/callback_unit...
File webrtc/base/callback_unittest.cc (right):

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/callback_unit...
webrtc/base/callback_unittest.cc:11: #include
<webrtc/base/keep_ref_until_done.h>
This is not sorted anymore. Did you pass presubmit?

[perkj_webrtc, 2015-11-30 16:40:09]

Description was changed from

==========
Add helper ReleaseWhenDoneCallback.
The callback keeps a reference to an object until the callback goes out of
scope.
==========

to

==========
Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of
scope.
==========

[perkj_webrtc, 2015-11-30 16:41:31]

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/callback_unit...
File webrtc/base/callback_unittest.cc (right):

https://codereview.webrtc.org/1487493002/diff/40001/webrtc/base/callback_unit...
webrtc/base/callback_unittest.cc:11: #include
<webrtc/base/keep_ref_until_done.h>
On 2015/11/30 15:44:34, magjed_webrtc wrote:
> This is not sorted anymore. Did you pass presubmit?

Done.

[magjed_webrtc, 2015-11-30 16:48:46]

lgtm

[perkj_webrtc, 2015-11-30 16:49:09]

The CQ bit was checked by perkj@webrtc.org

[perkj_webrtc, 2015-11-30 16:49:10]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1487493002/#ps60001 (title:
"Changed back to use template")

[commit-bot: I haz the power, 2015-11-30 16:49:13]

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

[commit-bot: I haz the power, 2015-11-30 18:39:36]

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

[commit-bot: I haz the power, 2015-11-30 18:39:37]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[perkj_webrtc, 2015-12-01 06:14:30]

The CQ bit was checked by perkj@webrtc.org

[commit-bot: I haz the power, 2015-12-01 06:14:37]

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

[commit-bot: I haz the power, 2015-12-01 06:15:48]

Description was changed from

==========
Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of
scope.
==========

to

==========
Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of
scope.
==========

[commit-bot: I haz the power, 2015-12-01 06:15:49]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-12-01 06:16:01]

Description was changed from

==========
Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of
scope.
==========

to

==========
Add helper KeepRefUntilDone.
The callback keeps a reference to an object until the callback goes out of
scope.

Committed: https://crrev.com/14f4144a82558ec4da2d4962ef02b23f44967b6a
Cr-Commit-Position: refs/heads/master@{#10847}
==========

[commit-bot: I haz the power, 2015-12-01 06:16:02]

Patchset 3 (id:??) landed as
https://crrev.com/14f4144a82558ec4da2d4962ef02b23f44967b6a
Cr-Commit-Position: refs/heads/master@{#10847}