Make class lookup lazy in jni_generator when using lazy method lookup.

base/android/jni_android.cc

Index: base/android/jni_android.cc
diff --git a/base/android/jni_android.cc b/base/android/jni_android.cc
index 6f080fb6e3b112d6d12feedc2d5d03ca5edd4386..8a6479b092d17a16c8527148f77e194615690fa6 100644
--- a/base/android/jni_android.cc
+++ b/base/android/jni_android.cc
@@ -8,6 +8,7 @@
 
 #include "base/android/build_info.h"
 #include "base/android/jni_string.h"
+#include "base/android/jni_utils.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 
@@ -21,6 +22,9 @@ JavaVM* g_jvm = NULL;
 // that may still be running at shutdown. There is no harm in doing this.
 base::LazyInstance<base::android::ScopedJavaGlobalRef<jobject> >::Leaky
     g_application_context = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<base::android::ScopedJavaGlobalRef<jobject> >::Leaky
+    g_class_loader = LAZY_INSTANCE_INITIALIZER;
+jmethodID g_class_loader_load_class_method_id = 0;
 
 std::string GetJavaExceptionInfo(JNIEnv* env, jthrowable java_throwable) {
   ScopedJavaLocalRef<jclass> throwable_clazz =
@@ -118,17 +122,66 @@ void InitApplicationContext(JNIEnv* env, const JavaRef<jobject>& context) {
   g_application_context.Get().Reset(context);
 }
 
+void InitFallbackClassLoader(JNIEnv* env) {

[rmcilroy, 2014/08/18 15:16:41]

Maybe call this InitReplacementClassLoader (since the classloader used here
replaces all classloaders.  Also, I think it might be better to just pass the
classloader as an argument to this function, with the caller itself using the
JniUtils GetClassLoader call to get the correct classloader - that way you don't
need the warning on the function description that this needs to be done after
registering the JNI for base, WDYT?

[mkosiba (inactive), 2014/08/19 09:50:15]

Sounds good. As I was commenting earlier - it seemed to me that having a method
in base:: ask the caller to give it the result of calling another method in
base:: seemed a bit ugly. It does clarify when it is appropriate to call the
method though so I guess I'll just go with that.

+  DCHECK(g_class_loader.Get().is_null());
+
+  ScopedJavaLocalRef<jclass> class_loader_clazz =
+      GetClass(env, "java/lang/ClassLoader");
+  g_class_loader_load_class_method_id =
+      env->GetMethodID(class_loader_clazz.obj(),
+                       "loadClass",
+                       "(Ljava/lang/String;)Ljava/lang/Class;");
+  ScopedJavaLocalRef<jobject> class_loader = GetClassLoader(env);
+  DCHECK(!class_loader.is_null());
+  g_class_loader.Get().Reset(class_loader);
+}
+
 const jobject GetApplicationContext() {
   DCHECK(!g_application_context.Get().is_null());
   return g_application_context.Get().obj();
 }
 
 ScopedJavaLocalRef<jclass> GetClass(JNIEnv* env, const char* class_name) {
-  jclass clazz = env->FindClass(class_name);
+  jclass clazz;
+  if (!g_class_loader.Get().is_null()) {
+    clazz = static_cast<jclass>(
+        env->CallObjectMethod(g_class_loader.Get().obj(),
+                              g_class_loader_load_class_method_id,
+                              ConvertUTF8ToJavaString(env, class_name).obj()));
+  } else {
+    clazz = env->FindClass(class_name);
+  }
   CHECK(!ClearException(env) && clazz) << "Failed to find class " << class_name;
   return ScopedJavaLocalRef<jclass>(env, clazz);
 }
 
+jclass LazyGetClass(
+    JNIEnv* env,
+    const char* class_name,
+    base::subtle::AtomicWord* atomic_class_id) {
+  COMPILE_ASSERT(sizeof(subtle::AtomicWord) >= sizeof(jclass),
+                 AtomicWord_SmallerThan_jMethodID);
+  subtle::AtomicWord value = base::subtle::Acquire_Load(atomic_class_id);
+  if (value)
+    return reinterpret_cast<jclass>(value);
+  ScopedJavaGlobalRef<jclass> clazz;
+  clazz.Reset(GetClass(env, class_name));
+  subtle::AtomicWord null_aw = reinterpret_cast<subtle::AtomicWord>(NULL);
+  subtle::AtomicWord old = base::subtle::Release_CompareAndSwap(
+      atomic_class_id,
+      null_aw,
+      reinterpret_cast<subtle::AtomicWord>(clazz.obj()));
+  if (old == null_aw) {
+    // We intentionally release the global ref as that makes it easy to store
+    // the ref in |atomic_class_id|.

[rmcilroy, 2014/08/18 15:16:41]

Maybe mention that this is so that we can store it as a raw pointer in
|atomic_class_id| and to avoid the destructor being called on the JavaRef?

[mkosiba (inactive), 2014/08/19 09:50:15]

Done.

+    return clazz.Release();
+  } else {
+    subtle::AtomicWord value = base::subtle::NoBarrier_Load(atomic_class_id);

[Torne, 2014/08/18 14:54:40]

Can't we just return old here?

[rmcilroy, 2014/08/18 15:16:41]

Hmm, I'm not sure about the semantics of a NoBarrier_Load after a
Release_CompareAndSwap.  The definition of Release operations is "all previous
memory stores are globally visible, and all previous memory loads have been
satisfied, but following memory reads are not prevented from being speculated to
before the barrier".  

I think this should still be OK since the two operations are on the same memory
location, so the NoBarrier_Load shouldn't be speculated above the
Release_CompareAndSwap, but these things are subtle.

Would it not be OK just to return the old value here?

[mkosiba (inactive), 2014/08/19 09:50:15]

Yeath, using old should be fine here.

+    CHECK(value);
+    return reinterpret_cast<jclass>(value);
+  }
+}
+
 template<MethodID::Type type>
 jmethodID MethodID::Get(JNIEnv* env,
                         jclass clazz,