jni_generator: Pass object parameters as JavaParamRef.

jni_generator: Pass object parameters as JavaParamRef.

Pass all object parameters to JNI functions in JavaParamRef<> wrappers.
This new type behaves like ScopedJavaLocalRef, but does not free the
local reference when it goes out of scope, since the JVM does not allow
method parameters to be freed and this causes a warning.

This CL only changes the implementation of the generator and the
signatures of JNI functions which take JavaParamRef arguments; the
minimum required to allow it to compile and work. An implicit cast from
JavaParamRef<T> to T is defined, to allow function bodies to remain
unaltered. These will be migrated over future CLs and the cast removed.

BUG=506850
Committed: https://crrev.com/89cc5d913e36029273a916f27d154967af3900d6
Cr-Commit-Position: refs/heads/master@{#347379}

[Torne, 2015-08-28 11:21:26]

torne@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[Torne, 2015-08-28 11:21:26]

PS#1 contains the "interesting" change: introducing JavaParamRef and updating
the JNI generator to use it.

PS#2 is a 100% scripted rewrite of all the affected function signatures and does
not change any function bodies whatsoever.

This increases the binary size of libchrome.so by 68 bytes.

[Torne, 2015-08-28 11:46:18]

D'oh. More references in files that are only build with chromecast=1. :(

[rmcilroy, 2015-08-28 12:16:04]

Some suggestions for the jni_generator code.

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
File base/android/jni_generator/jni_generator.py (right):

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1016: def
GetJNIFirstParamForDeclaration(self, native):
I think you could avoid this extra method if you had a WrapCTypeForDeclaration
(which could also be used by JavaDataTypeToCForDeclaration). 

def GetJNIFirstParam(self, native, for_declaration):
  if native.type == 'method':
    type = 'jobject'
  elif native.type == 'function':
    if native.static:
      type = 'jclass'
    else:
      type = 'jobject'
  if (for_declaration):
    type = WrapJavaTypeForDeclaration(type)
  return [type + ' jcaller']

WDYT?

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1038: def
GetJNIFirstParamForCall(self, native):
nit - move this down to be above GetNativeStub where it is used.

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1113: param_ref_template =
Template('JavaParamRef<${TYPE}>(env, ${NAME})')
nit - could you create a helper for this (which could also be used by
GetJNIFirstParamForCall

[Torne, 2015-08-28 12:36:18]

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
File base/android/jni_generator/jni_generator.py (right):

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1016: def
GetJNIFirstParamForDeclaration(self, native):
On 2015/08/28 12:16:04, rmcilroy wrote:
> I think you could avoid this extra method if you had a WrapCTypeForDeclaration
> (which could also be used by JavaDataTypeToCForDeclaration). 
> 
> def GetJNIFirstParam(self, native, for_declaration):
>   if native.type == 'method':
>     type = 'jobject'
>   elif native.type == 'function':
>     if native.static:
>       type = 'jclass'
>     else:
>       type = 'jobject'
>   if (for_declaration):
>     type = WrapJavaTypeForDeclaration(type)
>   return [type + ' jcaller']
> 
> WDYT?

Done.

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1038: def
GetJNIFirstParamForCall(self, native):
On 2015/08/28 12:16:04, rmcilroy wrote:
> nit - move this down to be above GetNativeStub where it is used.

Done.

https://codereview.chromium.org/1312153003/diff/1/base/android/jni_generator/...
base/android/jni_generator/jni_generator.py:1113: param_ref_template =
Template('JavaParamRef<${TYPE}>(env, ${NAME})')
On 2015/08/28 12:16:04, rmcilroy wrote:
> nit - could you create a helper for this (which could also be used by
> GetJNIFirstParamForCall

Done.

[rmcilroy, 2015-08-28 14:09:50]

lgtm, thanks!

[Torne, 2015-08-28 14:12:59]

torne@chromium.org changed reviewers:
+ ben@chromium.org

[Torne, 2015-08-28 14:13:00]

Ben, would you be able to stamp this as a toplevel OWNER? The changes outside of
base/android (reviewed by Ross) are all entirely scripted updates of function
signatures with no changes to bodies (and run through 'git cl format'), and the
impact on the compiled code is negligible (almost all functions compile
identically and total size only increases by 68 bytes), so it should be safe.

[Torne, 2015-09-03 14:29:51]

torne@chromium.org changed reviewers:
+ cpu@chromium.org

[Torne, 2015-09-03 14:29:52]

+cpu - any chance of a toplevel OWNER approval? This is becoming very hard to
rebase :(

[Torne, 2015-09-03 16:04:40]

argh linux_android_rel_ng failure seems to be some issue with the recipe; will
try to find out what's going on there. Doesn't appear to be related to the
patch, which probably shouldn't be able to break a python test script running on
the host ;)

[cpu_(ooo_6.6-7.5), 2015-09-03 21:17:51]

rubberstamp lgtm on the mechanical changes.

[Torne, 2015-09-04 09:41:03]

The CQ bit was checked by torne@chromium.org

[Torne, 2015-09-04 09:41:03]

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

[commit-bot: I haz the power, 2015-09-04 09:41:35]

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

[commit-bot: I haz the power, 2015-09-04 11:16:42]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-09-04 11:17:16]

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