base: Add RetainedRef for unwrapping scoped_refptrs.

base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048
Committed: https://crrev.com/1d492be0508e1638a37ac6a156d16e9f724da69c
Cr-Commit-Position: refs/heads/master@{#382076}

[vmpstr, 2016-03-17 21:14:56]

Description was changed from

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org
BUG=589048
==========

to

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048
==========

[vmpstr, 2016-03-17 21:14:56]

vmpstr@chromium.org changed reviewers:
+ thakis@chromium.org

[vmpstr, 2016-03-17 21:18:48]

Please take a look. This is a work in progress proposal from the bug. The
overall effort is targeting removing implicit scoped_refptr creation.

[tzik, 2016-03-18 03:18:48]

lgtm

[Nico, 2016-03-18 14:16:36]

The overall plan sounds good. The patch looks fine too, but it's lacking a
comment update, see below.

Do you have a rough idea how frequently Bind() holds on to a refptr only to pass
it as a raw pointer? Intuitively I'd expect this to be rare, but I don't feel
very confident about this prediction.

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h
File base/bind_helpers.h (right):

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h#newcode37
base/bind_helpers.h:37: // something else that expect a Callback with a void
return.
explain this new qualifier here...

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h#newcode133
base/bind_helpers.h:133: //
...and here

[vmpstr, 2016-03-18 18:02:46]

PTAL

> Do you have a rough idea how frequently Bind() holds on to
> a refptr only to pass it as a raw pointer? Intuitively I'd
> expect this to be rare, but I don't feel very confident
> about this prediction.

I've preemptively started converting some of the callsites and so far I've
changed 106 files (some files have multiple instances of this happening). I'm
not sure how that compares to general Bind usage, but it feels like it's a
fairly common pattern.

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h
File base/bind_helpers.h (right):

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h#newcode37
base/bind_helpers.h:37: // something else that expect a Callback with a void
return.
On 2016/03/18 14:16:36, Nico wrote:
> explain this new qualifier here...

Done.

https://codereview.chromium.org/1815493002/diff/1/base/bind_helpers.h#newcode133
base/bind_helpers.h:133: //
On 2016/03/18 14:16:36, Nico wrote:
> ...and here

Done.

[Nico, 2016-03-18 18:04:48]

lgtm

[vmpstr, 2016-03-18 18:07:00]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2016-03-18 18:07:01]

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

[commit-bot: I haz the power, 2016-03-18 18:07:54]

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

[commit-bot: I haz the power, 2016-03-18 20:46:48]

Description was changed from

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048
==========

to

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048
==========

[commit-bot: I haz the power, 2016-03-18 20:46:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-18 20:48:47]

Description was changed from

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048
==========

to

==========
base: Add RetainedRef for unwrapping scoped_refptrs.

This patch adds a RetainedRef call in order to be used in Bind with
scoped_refptrs that are to be unwrapped before calling the function.

This is an effort to eliminate implicit scoped_refptr construction from
Bind. Currently, scoped_refptr are always unwrapped into their
underlying T*. This means that if the function does actually expect
a scoped_refptr, one will be constructed implicitly.

The plan is to stop unwrapping scoped_refptrs altogether. If the behavior
is still desired, then the object has to be wrapped in a RetainedRef
wrapper. For more details, please see crbug.com/589048

R=tzik@chromium.org, danakj@chromium.org, thakis@chromium.org
BUG=589048

Committed: https://crrev.com/1d492be0508e1638a37ac6a156d16e9f724da69c
Cr-Commit-Position: refs/heads/master@{#382076}
==========

[commit-bot: I haz the power, 2016-03-18 20:48:48]

Patchset 2 (id:??) landed as
https://crrev.com/1d492be0508e1638a37ac6a156d16e9f724da69c
Cr-Commit-Position: refs/heads/master@{#382076}