Android: lazy initialization for method id.

chrome/browser/history/android/sqlite_cursor.cc

Index: chrome/browser/history/android/sqlite_cursor.cc
diff --git a/chrome/browser/history/android/sqlite_cursor.cc b/chrome/browser/history/android/sqlite_cursor.cc
index 786e157c7e090978b5666162fbdaaa71adb2b1d2..51ee4b1c02a517f4955290cdb073e8f630ae3788 100644
--- a/chrome/browser/history/android/sqlite_cursor.cc
+++ b/chrome/browser/history/android/sqlite_cursor.cc
@@ -16,8 +16,7 @@
 using base::android::ConvertUTF8ToJavaString;
 using base::android::GetClass;
 using base::android::HasClass;
-using base::android::HasMethod;
-using base::android::GetMethodID;
+using base::android::MethodID;
 using base::android::ScopedJavaLocalRef;
 using content::BrowserThread;
 
@@ -62,12 +61,14 @@ ScopedJavaLocalRef<jobject> SQLiteCursor::NewJavaSqliteCursor(
   }
 
   ScopedJavaLocalRef<jclass> sclass = GetClass(env, kSQLiteCursorClassPath);
-  if (!HasMethod(env, sclass, "<init>", "(I)V")) {
+  jmethodID method_id = MethodID::Get<
+      MethodID::METHODTYPE_NORMAL, MethodID::EXCEPTIONCHECK_NO>(

[joth, 2012/10/04 17:59:59]

For the non-lazy API I'm ambivalent about switching to template API. I see the
consistency argument, but the EXCEPTIONCHECK_YES is 90% irrelevant noise, and in
the odd place where it's important like here I completely missed the fact this
method may be chucking a NULL pointer back at us. (And I even was going to ask
whether the if(!method_id) below is spurious)

TL;DR: GetFooOrNull() is much easier for my eyes to spot, than the difference
between _YES vs _NO in a param I trained myself to ignore.

[bulach, 2012/10/04 18:58:17]

well, a few steps back :)
aren't your eyes ignoring just because this patch is massive?

I mean, going forward, both reviewers and writers should:
- not write custom JNI in the first place! :)
(I do intend to get rid of these at some point..)

- if they do, they should be really careful.

I'd rather keep a single consistent API, otherwise we'll end up with that
multitude of methods with ambiguous documentation quite quickly... wdyt?

[nyquist, 2012/10/04 19:05:48]

So, I kind of agree with joth that it might be hard to remember that by passing
the EXCEPTIONCHECK_NO template argument, the method can return null. Maybe the
argument should be clearer?
For example, change it to something like:
ALLOW_NULL_RESULT / DISALLOW_NULL_RESULT ?
Or use any type of good English language tricks to clarify the meaning of the
enum value, and what implications it has?

+          env, sclass.obj(), "<init>", "(I)V");
+  if (!method_id) {
     LOG(ERROR) << "Can not find " << kSQLiteCursorClassPath << " Constructor";
     return ScopedJavaLocalRef<jobject>();
   }
 
-  jmethodID method_id = GetMethodID(env, sclass, "<init>", "(I)V");
   SQLiteCursor* cursor = new SQLiteCursor(column_names, statement, service,
                                           favicon_service);
   ScopedJavaLocalRef<jobject> obj(env,