android: Don't pass ScopedJavaGlobalRef pointers.

android: Don't pass ScopedJavaGlobalRef pointers.

Various code was passing pointers to heap-allocated ScopedJavaGlobalRef
objects in order to pass Java objects between threads. Having
heap-allocated smart pointer types is very weird and not a desirable
pattern.

base::Bind handles rvalues correctly (they become owned by the callback)
and so the nicer alternative is to simply pass temporary
ScopedJavaGlobalRef parameters, which will automatically be deleted when
the callback is deleted. The receiving function can take a const
reference to JavaRef as its parameter type, which matches how direct
function calls typically take Java object parameters and makes this look
much more consistent.

ScopedJavaGlobalRef is movable, so using std::mode is also an option,
but all the cases in this CL simply construct temporaries.

BUG=519562
Committed: https://crrev.com/d2b4c5dac132d311691be5818b44f2abdf238749
Cr-Commit-Position: refs/heads/master@{#435593}

[Torne, 2016-11-28 18:09:14]

torne@chromium.org changed reviewers:
+ nyquist@chromium.org, tedchoc@chromium.org

[Torne, 2016-11-28 18:09:14]

Hi Tommy, Ted,

This fixes all the "simple" cases where ScopedJavaGlobalRef is being passed
around as a heap-allocated pointer. There's a couple of remaining places where
they're allocated on the heap that are doing something more complex that I'll
fix separately.

Ted: Can you review content and also just take a quick look at the
android_webview changes (though I own that).
Tommy: can you review chrome and dom_distiller?

[Ted C, 2016-11-28 18:44:22]

lgtm

[sgurun-gerrit only, 2016-11-30 17:22:22]

On 2016/11/28 18:44:22, Ted C wrote:
> lgtm

I guess some of this code may have been written before scopedjavaglobalref has a
copy constructor? Actually there was this CL

https://codereview.chromium.org/11971007/


not needed by lgtm from me too.

[Torne, 2016-11-30 17:28:00]

On 2016/11/30 17:22:22, sgurun wrote:
> On 2016/11/28 18:44:22, Ted C wrote:
> > lgtm
> 
> I guess some of this code may have been written before scopedjavaglobalref has
a
> copy constructor? Actually there was this CL
> 
> https://codereview.chromium.org/11971007/
> 
> 
> not needed by lgtm from me too.

Yes, probably. Since it now also has a move constructor it would be an
improvement to go back to the callsites in that CL and change them to just pass
a temporary, to eliminate the copy. I'll do that later.

[nyquist, 2016-11-30 22:30:28]

lgtm

[Torne, 2016-12-01 10:41:03]

The CQ bit was checked by torne@chromium.org

[commit-bot: I haz the power, 2016-12-01 10:41:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-01 11:18:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480588863240090,
"parent_rev": "75d6d1c74be87fcfcdc5acc4221f850716e8c36f", "commit_rev":
"3dd416af29c2d8f7e5a01aea9d7a436405d94533"}

[commit-bot: I haz the power, 2016-12-01 11:18:43]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-01 11:20:27]

Description was changed from

==========
android: Don't pass ScopedJavaGlobalRef pointers.

Various code was passing pointers to heap-allocated ScopedJavaGlobalRef
objects in order to pass Java objects between threads. Having
heap-allocated smart pointer types is very weird and not a desirable
pattern.

base::Bind handles rvalues correctly (they become owned by the callback)
and so the nicer alternative is to simply pass temporary
ScopedJavaGlobalRef parameters, which will automatically be deleted when
the callback is deleted. The receiving function can take a const
reference to JavaRef as its parameter type, which matches how direct
function calls typically take Java object parameters and makes this look
much more consistent.

ScopedJavaGlobalRef is movable, so using std::mode is also an option,
but all the cases in this CL simply construct temporaries.

BUG=519562
==========

to

==========
android: Don't pass ScopedJavaGlobalRef pointers.

Various code was passing pointers to heap-allocated ScopedJavaGlobalRef
objects in order to pass Java objects between threads. Having
heap-allocated smart pointer types is very weird and not a desirable
pattern.

base::Bind handles rvalues correctly (they become owned by the callback)
and so the nicer alternative is to simply pass temporary
ScopedJavaGlobalRef parameters, which will automatically be deleted when
the callback is deleted. The receiving function can take a const
reference to JavaRef as its parameter type, which matches how direct
function calls typically take Java object parameters and makes this look
much more consistent.

ScopedJavaGlobalRef is movable, so using std::mode is also an option,
but all the cases in this CL simply construct temporaries.

BUG=519562

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

[commit-bot: I haz the power, 2016-12-01 11:20:27]

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