rtc::Bind: Capture method objects as scoped_refptr if they are ref counted

rtc::Bind: Capture method objects as scoped_refptr if they are ref counted

R=tommi@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/d3de9c548d1121e7c4787a4b81fd66be714abc04

[magjed_webrtc, 2015-08-18 17:07:39]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-18 17:07:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1300523004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1300523004/1

[commit-bot: I haz the power, 2015-08-18 17:20:10]

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

[commit-bot: I haz the power, 2015-08-18 17:20:10]

Dry run: Try jobs failed on following builders:
  linux_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/8891)

[magjed_webrtc, 2015-08-18 17:45:54]

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

[magjed_webrtc, 2015-08-18 17:45:55]

This is the simplest implementation that worked. The failing bots are unrelated
to this CL. is_base_of implementation is copy-pasted. std::is_base_of is defined
in C++11 and in std::tr1 namespace for C++03. Do you think this will be
landable?

[tommi, 2015-08-18 19:53:19]

On 2015/08/18 17:45:55, magjed_webrtc wrote:
> This is the simplest implementation that worked. The failing bots are
unrelated
> to this CL. is_base_of implementation is copy-pasted. std::is_base_of is
defined
> in C++11 and in std::tr1 namespace for C++03. Do you think this will be
> landable?

Yes I think so. Some documentation would be good.

[magjed_webrtc, 2015-08-19 11:42:54]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 11:43:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1300523004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1300523004/20001

[magjed_webrtc, 2015-08-19 11:52:23]

I can use webrtc/base/template_util.h for doing the hardcore template magic.
Please take a look.

[tommi (sloooow) - chröme, 2015-08-19 13:13:32]

tommi@chromium.org changed reviewers:
+ tommi@chromium.org

[tommi (sloooow) - chröme, 2015-08-19 13:13:32]

great - one question

https://codereview.webrtc.org/1300523004/diff/20001/webrtc/base/bind.h
File webrtc/base/bind.h (right):

https://codereview.webrtc.org/1300523004/diff/20001/webrtc/base/bind.h#newcode86
webrtc/base/bind.h:86: static const bool value = is_convertible<T*,
RefCountInterface*>::value;
is there a way to detect presence of an AddRef() method instead of
RefCountInterface?

[commit-bot: I haz the power, 2015-08-19 13:43:05]

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

[commit-bot: I haz the power, 2015-08-19 13:43:06]

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

[magjed_webrtc, 2015-08-19 20:54:56]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-19 20:55:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1300523004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1300523004/40001

[commit-bot: I haz the power, 2015-08-19 20:56:55]

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

[commit-bot: I haz the power, 2015-08-19 20:56:55]

Dry run: Try jobs failed on following builders:
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/5143)
  win_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/9072)

[magjed_webrtc, 2015-08-20 09:40:56]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-20 09:41:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1300523004/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1300523004/70001

[magjed_webrtc, 2015-08-20 09:43:52]

Patchset #4 (id:10002) has been deleted

[magjed_webrtc, 2015-08-20 09:48:30]

Patchset #3 (id:40001) has been deleted

[magjed_webrtc, 2015-08-20 10:14:53]

PTAL. It's working as you want now, i.e. using rtc::Bind(T, ...) on any type T
that implemens AddRef() and Release() will be captured as scoped_refptr<T>.

[tommi, 2015-08-20 12:35:41]

one question, member variable naming, but otherwise looks great.

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind.h
File webrtc/base/bind.h (right):

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind.h#newcode3
webrtc/base/bind.h:3: // DO NOT EDIT BY HAND!!!
just checking - changes in this file are the result of running the script,
right? :)

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind.h#newcode58
webrtc/base/bind.h:58: //     bar = nullptr;
good example

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest.cc
File webrtc/base/bind_unittest.cc (right):

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:47: LifeTimeCheck(bool* hasDied) :
hasDied_(hasDied), isOkToDie(false) {}
has_died

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:56: bool* const hasDied_;
has_died_

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:57: bool isOkToDie;
is_ok_to_die_ (or ok_to_die_?)

[magjed_webrtc, 2015-08-20 13:05:12]

Patchset #5 (id:110001) has been deleted

[magjed_webrtc, 2015-08-20 13:05:20]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-20 13:05:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1300523004/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1300523004/130001

[magjed_webrtc, 2015-08-20 13:05:34]

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind.h
File webrtc/base/bind.h (right):

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind.h#newcode3
webrtc/base/bind.h:3: // DO NOT EDIT BY HAND!!!
On 2015/08/20 12:35:41, tommi (webrtc) wrote:
> just checking - changes in this file are the result of running the script,
> right? :)

yep :)

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest.cc
File webrtc/base/bind_unittest.cc (right):

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:47: LifeTimeCheck(bool* hasDied) :
hasDied_(hasDied), isOkToDie(false) {}
On 2015/08/20 12:35:41, tommi (webrtc) wrote:
> has_died

Done.

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:56: bool* const hasDied_;
On 2015/08/20 12:35:41, tommi (webrtc) wrote:
> has_died_

Done.

https://codereview.webrtc.org/1300523004/diff/90001/webrtc/base/bind_unittest...
webrtc/base/bind_unittest.cc:57: bool isOkToDie;
On 2015/08/20 12:35:41, tommi (webrtc) wrote:
> is_ok_to_die_ (or ok_to_die_?)

Done.

[tommi, 2015-08-20 13:38:33]

lgtm

[magjed_webrtc, 2015-08-20 13:57:37]

The CQ bit was unchecked by magjed@webrtc.org

[magjed_webrtc, 2015-08-20 14:04:12]

Committed patchset #5 (id:130001) manually as
d3de9c548d1121e7c4787a4b81fd66be714abc04 (presubmit successful).