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

Review URL: https://chromiumcodereview.appspot.com/9817013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128206 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 41 insertions, 10 deletions

diff --git a/content/browser/renderer_host/java/java_bound_object.cc b/content/browser/renderer_host/java/java_bound_object.cc
index a9f7920..c0d1aa5 100644
--- a/content/browser/renderer_host/java/java_bound_object.cc
+++ b/content/browser/renderer_host/java/java_bound_object.cc
@@ -397,7 +397,10 @@ jobject CreateJavaArray(const JavaType& type, jsize length) {
   return NULL;
 }
 
-// Note that this only handles primitive types and strings.
+// Sets the specified element of the supplied array to the value of the
+// supplied jvalue. Requires that the type of the array matches that of the
+// jvalue. Handles only primitive types and strings. Note that in the case of a
+// string, the array takes a new reference to the string object.
 void SetArrayElement(jobject array,
                      const JavaType& type,
                      jsize index,
@@ -450,10 +453,22 @@ void SetArrayElement(jobject array,
   base::android::CheckException(env);
 }
 
+void ReleaseJavaValueIfRequired(JNIEnv* env,
+                                jvalue* value,
+                                const JavaType& type) {
+  if (type.type == JavaType::TypeString ||
+      type.type == JavaType::TypeObject ||
+      type.type == JavaType::TypeArray) {
+    env->DeleteLocalRef(value->l);
+    value->l = NULL;
+  }
+}
+
 jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
                                         const JavaType& target_type,
                                         bool coerce_to_string);
 
+// Returns a new local reference to a Java array.
 jobject CoerceJavaScriptObjectToArray(const NPVariant& variant,
                                       const JavaType& target_type) {
   DCHECK_EQ(JavaType::TypeArray, target_type.type);
@@ -498,11 +513,11 @@ jobject CoerceJavaScriptObjectToArray(const NPVariant& variant,
     return NULL;
   }
 
-  // Create the Java array. Note that we don't explicitly release the local
-  // ref to the result or any of its elements.
+  // Create the Java array.
   // TODO(steveblock): Handle failure to create the array.
   jobject result = CreateJavaArray(target_inner_type, length);
   NPVariant value_variant;
+  JNIEnv* env = AttachCurrentThread();
   for (jsize i = 0; i < length; ++i) {
     // It seems that getProperty() will set the variant to type void on failure,
     // but this doesn't seem to be documented, so do it explicitly here for
@@ -512,10 +527,17 @@ jobject CoerceJavaScriptObjectToArray(const NPVariant& variant,
     // value as JavaScript undefined.
     WebBindings::getProperty(0, object, WebBindings::getIntIdentifier(i),
                              &value_variant);
-    SetArrayElement(result, target_inner_type, i,
-                    CoerceJavaScriptValueToJavaValue(value_variant,
-                                                     target_inner_type,
-                                                     false));
+    jvalue element = CoerceJavaScriptValueToJavaValue(value_variant,
+                                                      target_inner_type,
+                                                      false);
+    SetArrayElement(result, target_inner_type, i, element);
+    // CoerceJavaScriptValueToJavaValue() creates new local references to
+    // strings, objects and arrays. Of these, only strings can occur here.
+    // SetArrayElement() causes the array to take its own reference to the
+    // string, so we can now release the local reference.
+    DCHECK_NE(JavaType::TypeObject, target_inner_type.type);
+    DCHECK_NE(JavaType::TypeArray, target_inner_type.type);
+    ReleaseJavaValueIfRequired(env, &element, target_inner_type);
     WebBindings::releaseVariantValue(&value_variant);
   }
 
@@ -644,15 +666,16 @@ jvalue CoerceJavaScriptNullOrUndefinedToJavaValue(const NPVariant& variant,
 // strings when required, rather than simply converting to NULL. This is used
 // to maintain current behaviour, which differs slightly depending upon whether
 // or not the coercion in question is for an array element.
+//
+// Note that the jvalue returned by this method may contain a new local
+// reference to an object (string, object or array). This must be released by
+// the caller.
 jvalue CoerceJavaScriptValueToJavaValue(const NPVariant& variant,
                                         const JavaType& target_type,
                                         bool coerce_to_string) {
   // Note that in all these conversions, the relevant field of the jvalue must
   // always be explicitly set, as jvalue does not initialize its fields.
 
-  // Some of these methods create new Java Strings. Note that we don't
-  // explicitly release the local ref to these new objects, as there's no simple
-  // way to do so.
   switch (variant.type) {
     case NPVariantType_Int32:
     case NPVariantType_Double:
@@ -745,6 +768,14 @@ bool JavaBoundObject::Invoke(const std::string& name, const NPVariant* args,
   // Call
   *result = CallJNIMethod(java_object_.obj(), method->return_type(),
                           method->id(), &parameters[0]);
+
+  // Now that we're done with the jvalue, release any local references created
+  // by CoerceJavaScriptValueToJavaValue().
+  JNIEnv* env = AttachCurrentThread();
+  for (size_t i = 0; i < arg_count; ++i) {
+    ReleaseJavaValueIfRequired(env, &parameters[i], method->parameter_type(i));
+  }
+
   return true;
 }