Upstream Android JNI code, allowing us to use more ScopedJava references.

base/android/jni_android.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef BASE_ANDROID_JNI_ANDROID_H_
 #define BASE_ANDROID_JNI_ANDROID_H_
 
 #include <jni.h>
 #include <sys/types.h>
 
 #include "base/android/scoped_java_ref.h"
 
 namespace base {
 namespace android {
 
 // Attach the current thread to the VM (if necessary) and return the JNIEnv*.
 JNIEnv* AttachCurrentThread();
 
 // Detach the current thread from VM if it is attached.
 void DetachFromVM();
 
 // Initializes the global JVM. It is not necessarily called before
 // InitApplicationContext().
 void InitVM(JavaVM* vm);
 
 // Initializes the global application context object. The |context| can be any
 // valid reference to the application context. Internally holds a global ref to
 // the context. InitVM and InitApplicationContext maybe called in either order.
-// TODO: We might combine InitVM() and InitApplicationContext() into one method.
 void InitApplicationContext(const JavaRef<jobject>& context);
 
 // Gets a global ref to the application context set with
 // InitApplicationContext(). Ownership is retained by the function - the caller
 // must NOT release it.
-jobject GetApplicationContext();
+const jobject GetApplicationContext();
+
+// Finds the class named |class_name| and returns it.
+// Use this method instead of invoking directly the JNI FindClass method (to
+// prevent leaking local references).
+// This method triggers a fatal assertion if the class could not be found.
+// Use HasClass if you need to check whether the class exists.
+ScopedJavaLocalRef<jclass> GetClass(JNIEnv* env, const char* class_name);
+
+// Returns true iff the class |class_name| could be found.
+bool HasClass(JNIEnv* env, const char* class_name);
+
+// Returns the method ID for the method with the specified name and signature.
+// This method triggers a fatal assertion if the method could not be found.
+// Use HasMethod if you need to check whether a method exists.
+jmethodID GetMethodID(JNIEnv* env,
+                      const JavaRef<jclass>& clazz,
+                      const char* method_name,
+                      const char* jni_signature);
+
+// Returns the method ID for the static method with the specified name and
+// signature.
+// This method triggers a fatal assertion if the method could not be found.
+// Use HasMethod if you need to check whether a method exists.
+jmethodID GetStaticMethodID(JNIEnv* env,
+                            const JavaRef<jclass>& clazz,
+                            const char* method_name,
+                            const char* jni_signature);
+
+// Returns true iff |clazz| has a method with the specified name and signature.
+bool HasMethod(JNIEnv* env,
+               const JavaRef<jclass>& clazz,
+               const char* method_name,
+               const char* jni_signature);
 
 // Gets the method ID from the class name. Clears the pending Java exception
 // and returns NULL if the method is not found. Caches results. Note that
 // GetMethodID() below avoids a class lookup, but does not cache results.
 // Strings passed to this function are held in the cache and MUST remain valid
 // beyond the duration of all future calls to this function, across all
 // threads. In practice, this means that the function should only be used with
 // string constants.
 jmethodID GetMethodIDFromClassName(JNIEnv* env,
                                    const char* class_name,
                                    const char* method,
                                    const char* jni_signature);
 
-// Get the method ID for a method. Will clear the pending Java
-// exception and return 0 if the method is not found.
-jmethodID GetMethodID(JNIEnv* env,
-                      jclass clazz,
-                      const char* const method,
-                      const char* const jni_signature);
-
-// Get the method ID for a class static method. Will clear the pending Java
-// exception and return 0 if the method is not found.
-jmethodID GetStaticMethodID(JNIEnv* env,
-                            jclass clazz,
-                            const char* const method,
-                            const char* const jni_signature);
-
-// Gets the field ID for a class field. Clears the pending Java exception and
-// returns NULL if the field is not found.
+// Gets the field ID for a class field.
+// This method triggers a fatal assertion if the field could not be found.
 jfieldID GetFieldID(JNIEnv* env,
-                    jclass clazz,
-                    const char* field,
+                    const JavaRef<jclass>& clazz,
+                    const char* field_name,
                     const char* jni_signature);
 
-// Returns true if an exception is pending in the provided JNIEnv*. If an
-// exception is pending, this function prints and then clears it.
-bool CheckException(JNIEnv* env);
+// Returns true if |clazz| as a field with the given name and signature.
+bool HasField(JNIEnv* env,
+              const JavaRef<jclass>& clazz,

[joth, 2012/02/08 18:28:31]

odd to pass an explicit env when we could use clazz.env() instead.
Is this for thread correctness?

[Steve Block, 2012/02/08 18:41:55]

I think this was an oversight in the original review. I can't think of a case
where these methods are used with an JNIEnv from a different thread to that of
the clazz.

[Jay Civelli, 2012/02/08 18:49:56]

I believe there were actually some tests running into that scenario somehow.
Some code might create a global ref to a class in one thread, and then try to
get fields or methods later on from another one.

[Steve Block, 2012/02/08 19:02:43]

Hmm, if we want to support that use-case, we should add a comment to make it
very clear that env must be valid for the calling thread, but that clazz.env()
need not be.

However, in general, this seems like it could lead to confusion. I wonder if
it's better to remove the env param here, and the minority of call sites that
need this edge-case behaviour can call JNI methods directly. This could be a
TODO for the purposes of this patch.

[Jay Civelli, 2012/02/08 19:20:47]

Yeah, I dislike that case as well.
My main concern is that that contract is hard to enforce (that the
HasMethod/GetMethod methods are called on the same thread the class was
retrieved).
Someone could change some code and not realize that they are now breaking the
contract.
Passing the JNIEnv makes it less error prone in that way. 

[Peter Beverloo, 2012/02/09 15:25:29]

I added a TODO as Steve suggested, and attributed it to Jay. Jay, does this work
for you?

+              const char* field_name,
+              const char* jni_signature);
+
+// Gets the field ID for a static class field.
+// This method triggers a fatal assertion if the field could not be found.
+jfieldID GetStaticFieldID(JNIEnv* env,
+                          const JavaRef<jclass>& clazz,
+                          const char* field_name,
+                          const char* jni_signature);
+
+// Returns true if an exception is pending in the provided JNIEnv*.
+bool HasException(JNIEnv* env);
+
+// If an exception is pending in the provided JNIEnv*, this function clears it
+// and returns true.
+bool ClearException(JNIEnv* env);
+
+// This function will call CHECK() macro if there's any pending exception.
+void CheckException(JNIEnv* env);
 
 }  // namespace android
 }  // namespace base
 
 #endif  // BASE_ANDROID_JNI_ANDROID_H_