Explicitly release JNI local references in the Java Bridge

Explicitly release JNI local references in the Java Bridge

In order to pass arguments to the methods of injected Java objects, the Java
Bridge converts JavaScript values to Java values. This involves creating JNI
jvalue objects, which may contain new local references to strings, objects and
arrays. Currently, these local references are not explicitly released, so the
VM may run out of JNI local references.

This change addresses this by explicitly releasing these local references as
soon as we're done with them.

BUG=112819
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=128206

[joth, 2012-03-22 12:42:12]

Good find!
LGTM.  couple minor suggestions.

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
File content/browser/renderer_host/java/java_bound_object.cc (right):

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
content/browser/renderer_host/java/java_bound_object.cc:529:
AttachCurrentThread()->DeleteLocalRef(element.l);
nit: Grab the JNIEnv* outside the loop.

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
content/browser/renderer_host/java/java_bound_object.cc:769: }
to make it a bit more self documenting, how about we extract a function

  void ReleaseJavaValueOfType(JNIEnv* env, jvalue* value, const JavaType& type)
{
    if (type == String | Object | Array) {
      env->DeleteLocalRef(v->l);
      v->l = NULL;
    }
  }

can call it here and in CoerceJavaScriptObjectToArray

[Steve Block, 2012-03-22 13:03:23]

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
File content/browser/renderer_host/java/java_bound_object.cc (right):

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
content/browser/renderer_host/java/java_bound_object.cc:529:
AttachCurrentThread()->DeleteLocalRef(element.l);
On 2012/03/22 12:42:12, joth wrote:
> nit: Grab the JNIEnv* outside the loop.

Done.

http://codereview.chromium.org/9817013/diff/1/content/browser/renderer_host/j...
content/browser/renderer_host/java/java_bound_object.cc:769: }
On 2012/03/22 12:42:12, joth wrote:
> to make it a bit more self documenting, how about we extract a function
> 
>   void ReleaseJavaValueOfType(JNIEnv* env, jvalue* value, const JavaType&
type)
> {
>     if (type == String | Object | Array) {
>       env->DeleteLocalRef(v->l);
>       v->l = NULL;
>     }
>   }
> 
> can call it here and in CoerceJavaScriptObjectToArray

Done.

[joth, 2012-03-22 13:20:48]

lgtm

http://codereview.chromium.org/9817013/diff/4001/content/browser/renderer_hos...
File content/browser/renderer_host/java/java_bound_object.cc (right):

http://codereview.chromium.org/9817013/diff/4001/content/browser/renderer_hos...
content/browser/renderer_host/java/java_bound_object.cc:460: void
ReleaseJavaValueIfRequired(JNIEnv* env, jvalue* value,
nit: one param per line.
(if you want, you could group value & type together as they are very closely
coupled)