Add support for calling the java Callback from native.

Add support for calling the java Callback from native.

Attempting to reduce the boilerplate interfaces that need
to be added just to pass back single params from native to
java.

BUG=None

Committed: https://crrev.com/7efe979a9f4ca0755a535bde1cc4d5a5492f4277
Cr-Commit-Position: refs/heads/master@{#395218}

[Ted C, 2016-05-19 23:29:45]

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

[Ted C, 2016-05-19 23:29:46]

PTAL

Personally, conflicted.  I saw people adding more of these interfaces recently
and took a quick stab at how to reduce it.  I was thinking this would reduce it
more than it did, so throwing this out there to get initial thoughts.

[Michael van Ouwerkerk, 2016-05-20 09:01:41]

Drive-by notes :-)

This seems like a nice cleanup. Aren't there more places that would benefit from
this, like JavaScriptCallback?

[Yaron, 2016-05-20 14:27:25]

lgtm

When I saw the CL I was concerned about possible memory leaks because it would
be too easy to be sloppy and get wrong. Then one callback holding onto something
which holds onto an activity or something and we're in trouble.

Looking at the actual details and specifically the native side just being a
static caller and having no ownership of the callback, this seems like a fine
generic utility

[nyquist, 2016-05-20 15:59:09]

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/Callback.java (right):

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
base/android/java/src/org/chromium/base/Callback.java:14: public abstract class
Callback<T> {
Do you think it makes sense to add a sanity check Java test for this?

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
base/android/java/src/org/chromium/base/Callback.java:22: private void
onResultFromNative(Object result) {
Would it be helpful to be able to pass primitives from native? Or are they
already automatically autoboxed?

I guess it'd be more just to ensure that developers use this new nicety instead
of still having to fiddle.

[Ted C, 2016-05-20 16:20:04]

On 2016/05/20 09:01:41, Michael van Ouwerkerk wrote:
> Drive-by notes :-)
> 
> This seems like a nice cleanup. Aren't there more places that would benefit
from
> this, like JavaScriptCallback?

Oh, I'm sure there are.  I was going for low hanging fruit as a means of
example.

That one is meaty enough that it definitely should be done in a separate change
IMO.

I was really trying to gauge initial interest, which doesn't seem to be
immediately disgusted.

[Ted C, 2016-05-20 16:21:23]

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
File base/android/java/src/org/chromium/base/Callback.java (right):

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
base/android/java/src/org/chromium/base/Callback.java:14: public abstract class
Callback<T> {
On 2016/05/20 15:59:08, nyquist wrote:
> Do you think it makes sense to add a sanity check Java test for this?

I honestly can't think of anything useful this could test (purely from java).

https://codereview.chromium.org/1996193003/diff/20001/base/android/java/src/o...
base/android/java/src/org/chromium/base/Callback.java:22: private void
onResultFromNative(Object result) {
On 2016/05/20 15:59:09, nyquist wrote:
> Would it be helpful to be able to pass primitives from native? Or are they
> already automatically autoboxed?

From the almighty stack overflow, it doesn't look like it would be autoboxed.

http://stackoverflow.com/questions/2495742/jni-new-primitive-types

Nor anything in the jni spec about types:
http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html

> 
> I guess it'd be more just to ensure that developers use this new nicety
instead
> of still having to fiddle.

Added a version for boolean, which is the only one I saw immediately.  This sets
a precedence so other people can add the other primitives as needed.

Migrated the one last callback in SnippetsBridge that was using a boolean to
this instead.

[nyquist, 2016-05-20 16:23:54]

lgtm. Agree that adding the one boolean-version will show the intent of possibly
adding more later.

[Ted C, 2016-05-20 16:27:56]

The CQ bit was checked by tedchoc@chromium.org

[Ted C, 2016-05-20 16:27:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1996193003/#ps40001
(title: "Add support for calling callbacks with boolean primitives")

[commit-bot: I haz the power, 2016-05-20 16:28:14]

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

[commit-bot: I haz the power, 2016-05-20 16:32:19]

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

[commit-bot: I haz the power, 2016-05-20 16:32:20]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Ted C, 2016-05-20 16:38:03]

tedchoc@chromium.org changed reviewers:
+ thestig@chromium.org

[Ted C, 2016-05-20 16:38:04]

+thestig for base gn/gyp files owners

[Ted C, 2016-05-20 16:38:12]

The CQ bit was checked by tedchoc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-20 16:38:32]

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

[commit-bot: I haz the power, 2016-05-20 18:26:34]

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

[commit-bot: I haz the power, 2016-05-20 18:26:35]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-05-20 21:02:03]

lgtm

[Ted C, 2016-05-20 22:01:48]

The CQ bit was checked by tedchoc@chromium.org

[Ted C, 2016-05-20 22:01:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
nyquist@chromium.org, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1996193003/#ps60001
(title: "Fix base.gypi missing trailing '")

[commit-bot: I haz the power, 2016-05-20 22:02:23]

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

[commit-bot: I haz the power, 2016-05-20 22:11:04]

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

[commit-bot: I haz the power, 2016-05-20 22:11:06]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ted C, 2016-05-20 23:03:45]

The CQ bit was checked by tedchoc@chromium.org

[commit-bot: I haz the power, 2016-05-20 23:04:23]

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

[commit-bot: I haz the power, 2016-05-20 23:12:37]

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

[commit-bot: I haz the power, 2016-05-20 23:12:38]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Lei Zhang, 2016-05-20 23:15:31]

On 2016/05/20 23:12:38, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Need to rebase?

[Ted C, 2016-05-20 23:32:53]

The CQ bit was checked by tedchoc@chromium.org

[Ted C, 2016-05-20 23:32:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, nyquist@chromium.org, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1996193003/#ps80001
(title: "Rebased")

[Ted C, 2016-05-20 23:33:03]

On 2016/05/20 23:15:31, Lei Zhang wrote:
> On 2016/05/20 23:12:38, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
> 
> Need to rebase?

Indeed :-)

[commit-bot: I haz the power, 2016-05-20 23:33:33]

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

[commit-bot: I haz the power, 2016-05-21 00:36:01]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-05-21 00:37:45]

Description was changed from

==========
Add support for calling the java Callback from native.

Attempting to reduce the boilerplate interfaces that need
to be added just to pass back single params from native to
java.

BUG=None
==========

to

==========
Add support for calling the java Callback from native.

Attempting to reduce the boilerplate interfaces that need
to be added just to pass back single params from native to
java.

BUG=None

Committed: https://crrev.com/7efe979a9f4ca0755a535bde1cc4d5a5492f4277
Cr-Commit-Position: refs/heads/master@{#395218}
==========

[commit-bot: I haz the power, 2016-05-21 00:37:46]

Patchset 5 (id:??) landed as
https://crrev.com/7efe979a9f4ca0755a535bde1cc4d5a5492f4277
Cr-Commit-Position: refs/heads/master@{#395218}