Add RetainedRef uses where needed.

Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind.

Please see the bug for more context.

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

[vmpstr, 2016-03-18 20:42:17]

Description was changed from

==========
Add RetainedRef uses where needed. Remove scoped_refptr unwrapping.

This patch adds RetainedRef uses where they are required. At the same
time this removes scoped_refptr unwrapping for general args. Note that
we still unwrap scoped_refptr for the first (object) parameter if
required. Please see the bug for more context.

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

to

==========
Add RetainedRef uses where needed. Remove scoped_refptr unwrapping.

This patch adds RetainedRef uses where they are required. At the same
time this removes scoped_refptr unwrapping for general args. Note that
we still unwrap scoped_refptr for the first (object) parameter if
the first parameter is the object on which the function will be called.

Please see the bug for more context.

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

[vmpstr, 2016-03-18 20:44:12]

Please take a look.

This depends on https://codereview.chromium.org/1815493002/. 

I plan on sending a PSA before landing this to let people know that passing a
scoped_refptr and expecting a raw pointer on the callee side will no longer work
after this lands.

[Nico, 2016-03-18 21:01:01]

I'm very confused by the nullptr changes. The rest looks good.

https://codereview.chromium.org/1815363002/diff/20001/base/bind_helpers.h
File base/bind_helpers.h (left):

https://codereview.chromium.org/1815363002/diff/20001/base/bind_helpers.h#old...
base/bind_helpers.h:438: template <typename T>
I'd land this bit in a different CL, so that it can be reverted independently of
the rest in case this breaks some obscure bot

https://codereview.chromium.org/1815363002/diff/20001/chrome/browser/printing...
File chrome/browser/printing/print_job_worker.cc (right):

https://codereview.chromium.org/1815363002/diff/20001/chrome/browser/printing...
chrome/browser/printing/print_job_worker.cc:380:
base::RetainedRef<PrintedPage>(nullptr)));
RetainedRef<>(nullptr)? Why can't this just pass nullptr directly?

https://codereview.chromium.org/1815363002/diff/20001/chrome/browser/printing...
chrome/browser/printing/print_job_worker.cc:434:
base::RetainedRef<PrintedPage>(nullptr)));
same question

https://codereview.chromium.org/1815363002/diff/20001/content/browser/tracing...
File content/browser/tracing/tracing_controller_impl.cc (right):

https://codereview.chromium.org/1815363002/diff/20001/content/browser/tracing...
content/browser/tracing/tracing_controller_impl.cc:416:
base::RetainedRef<TraceMessageFilter>(nullptr), status));
ditto

https://codereview.chromium.org/1815363002/diff/20001/content/child/fileapi/w...
File content/child/fileapi/webfilesystem_impl.cc (right):

https://codereview.chromium.org/1815363002/diff/20001/content/child/fileapi/w...
content/child/fileapi/webfilesystem_impl.cc:107:
base::RetainedRef<WaitableCallbackResults>(nullptr)));
ditto

[vmpstr, 2016-03-18 22:19:27]

Description was changed from

==========
Add RetainedRef uses where needed. Remove scoped_refptr unwrapping.

This patch adds RetainedRef uses where they are required. At the same
time this removes scoped_refptr unwrapping for general args. Note that
we still unwrap scoped_refptr for the first (object) parameter if
the first parameter is the object on which the function will be called.

Please see the bug for more context.

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

to

==========
Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind. 

Please see the bug for more context.

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

[vmpstr, 2016-03-18 22:20:23]

PTAL

https://codereview.chromium.org/1815363002/diff/20001/base/bind_helpers.h
File base/bind_helpers.h (left):

https://codereview.chromium.org/1815363002/diff/20001/base/bind_helpers.h#old...
base/bind_helpers.h:438: template <typename T>
On 2016/03/18 21:01:01, Nico wrote:
> I'd land this bit in a different CL, so that it can be reverted independently
of
> the rest in case this breaks some obscure bot

Good idea. I've separated this.

https://codereview.chromium.org/1815363002/diff/20001/chrome/browser/printing...
File chrome/browser/printing/print_job_worker.cc (right):

https://codereview.chromium.org/1815363002/diff/20001/chrome/browser/printing...
chrome/browser/printing/print_job_worker.cc:380:
base::RetainedRef<PrintedPage>(nullptr)));
On 2016/03/18 21:01:01, Nico wrote:
> RetainedRef<>(nullptr)? Why can't this just pass nullptr directly?

Oh oops. I meant to go back and see if nullptr worked. It does, thanks!

[Nico, 2016-03-18 22:23:22]

lgtm

[vmpstr, 2016-03-18 22:34:16]

vmpstr@chromium.org changed reviewers:
+ gavinp@chromium.org, jam@chromium.org, jochen@chromium.org,
pavely@chromium.org, piman@chromium.org, reillyg@chromium.org,
thestig@chromium.org

[vmpstr, 2016-03-18 22:34:17]

+jochen@ for components/ and content/
+reillyg@ for extensions/
+jam@ for ipc/
+gavinp@ for net/
+piman@ for ppapi/proxy/
+thestig@ for printing/
+pavely@ for sync/

Please take a look (I'll TBR the rest if most people are fine with this change)

[Reilly Grant (use Gerrit), 2016-03-18 22:42:02]

//chrome/browser/extensions and //extensions lgtm.

In all the cases I see in the code I looked at it seems like the function being
bound should just take a scoped_refptr<T> instead of T*. Should we go through
after this lands and clean up instances of base::RetainedRef? Is there any case
where it should continue to exist?

[pavely, 2016-03-18 22:53:56]

sync/internal_api/http_bridge_unittest.cc lgtm

[vmpstr, 2016-03-18 23:12:08]

On 2016/03/18 22:42:02, Reilly Grant wrote:
> //chrome/browser/extensions and //extensions lgtm.
> 
> In all the cases I see in the code I looked at it seems like the function
being
> bound should just take a scoped_refptr<T> instead of T*. Should we go through
> after this lands and clean up instances of base::RetainedRef? Is there any
case
> where it should continue to exist?

There are cases where the function should really take a T*, since it does not
participate in ownership of the given object. That being said, if there is a
function that takes a T* and then wraps it in a scoped_refptr, then that should
most certainly be cleaned up.

There was a bit of a discussion on this topic in crbug.com/589048 (comments 4
and 5), and we thought it would be nicer to allow functions that take a non
owning raw pointer to be bound to ref counted objects...

[piman, 2016-03-18 23:49:24]

lgtm

[tzik, 2016-03-19 10:13:45]

lgtm

[jam, 2016-03-21 14:47:47]

lgtm

[vmpstr, 2016-03-21 18:49:15]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2016-03-21 18:49:47]

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

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

Description was changed from

==========
Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind. 

Please see the bug for more context.

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

to

==========
Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind. 

Please see the bug for more context.

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

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

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-21 20:30:36]

Description was changed from

==========
Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind. 

Please see the bug for more context.

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

to

==========
Add RetainedRef uses where needed.

This patch adds RetainedRef uses where they are required. These
changes will be required when we remove automatic unwrapping of
scoped_refptrs in Bind.

Please see the bug for more context.

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

Committed: https://crrev.com/a34d11324e0767acd94d8754e3a117e072492cf4
Cr-Commit-Position: refs/heads/master@{#382371}
==========

[commit-bot: I haz the power, 2016-03-21 20:30:39]

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