[Android] Introduce a Java wrapper for TemplateUrlService.

base/android/jni_map.cc

+// Copyright (c) 2013 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.
+
+#include "base/android/jni_map.h"
+
+#include <map>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/logging.h"
+
+namespace base {
+namespace android {
+
+// Converts a std::map<std::string, string16> to a java HashMap<String,String>.

[nyquist, 2013/02/20 07:33:09]

Nit: insert space between Java generics types.

[Yaron, 2013/02/21 21:39:15]

Done.

+ScopedJavaLocalRef<jobject> ConvertMapToJavaMap(
+    JNIEnv* env,
+    const std::map<std::string, string16>& cmap) {
+  ScopedJavaLocalRef<jclass> map_clazz = GetClass(env, "java/util/HashMap");

[bulach, 2013/02/20 14:41:25]

a kitten has died.... :D

any chance of generating the JNI for this? we fully support system classes... if
there's any issues with it, create a tiny wrapper in java.... this sort of
boiler plate tend to be viral, and before we know it, it's spread all over the
place! it'd be great to set a better example, wdyt?

also, why is it necessary to pass a map to/from java?
wouldn't it be simpler, and perhaps even more efficient, to just expose either a
@CalledByNative or a native "setKeyValue(String, String)" wherever this is
needed?

I mean, from the java side, it'd call N times "native setKeyValue(String,
String)" then call "doSomething()" once...
from native, it'd call "Java_setKeyValue(..)" then "doSomething()"...

wdyt?

[Yaron, 2013/02/21 21:39:15]

That seems suboptimal to me. I feel like what you're really trying to do is pass
a map of data from native to Java or vice-versa and we should support. Exposing
it as a series of setKeyValue calls is getting away from having a data structure
that can be marshalled along the boundary. I'd also like to avoid duplicating
the data and/or having two possible owners.

In the end, I stuck with a map and converted most bindings to JNI-generated
ones. The one caveat is that inner-classes generated from a jar-file don't work
'cause $ is treating as an environment variable. This required a minor tweak to
jni_generator to support interfaces.

+  jmethodID map_constructor = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, map_clazz.obj(), "<init>", "()V");
+  jmethodID map_put = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, map_clazz.obj(), "put",
+      "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+
+  jobject jmap = env->NewObject(map_clazz.obj(), map_constructor);
+  DCHECK(jmap);
+  CheckException(env);
+
+  for (std::map<std::string, string16>::const_iterator i = cmap.begin();
+       i != cmap.end(); i++) {
+    ScopedJavaLocalRef<jstring> key = ConvertUTF8ToJavaString(env, i->first);
+    ScopedJavaLocalRef<jstring> value = ConvertUTF16ToJavaString(env,
+                                                                 i->second);
+    // Assign the result to a ScopedJavaLocalRef so we don't leak the local ref.
+    ScopedJavaLocalRef<jobject> result(env,
+        env->CallObjectMethod(jmap, map_put, key.obj(), value.obj()));
+    CheckException(env);
+  }
+  return ScopedJavaLocalRef<jobject>(env, jmap);
+}
+
+std::map<std::string, string16> ConvertJavaMapToMap(JNIEnv* env,
+                                                    jobject jmap) {
+  std::map<std::string, string16> cmap;
+
+  // Get a Set of all items.
+  ScopedJavaLocalRef<jclass> map_clazz = GetClass(env, "java/util/HashMap");
+  jmethodID map_entryset = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, map_clazz.obj(), "entrySet", "()Ljava/util/Set;");
+  ScopedJavaLocalRef<jobject> jset(env,
+      env->CallObjectMethod(jmap, map_entryset));
+  DCHECK(!jset.is_null());
+  CheckException(env);
+
+  // Get an iterator for this set.
+  ScopedJavaLocalRef<jclass> set_clazz = GetClass(env, "java/util/Set");
+  jmethodID set_iterator = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, set_clazz.obj(), "iterator", "()Ljava/util/Iterator;");
+  ScopedJavaLocalRef<jobject> iter(env,
+      env->CallObjectMethod(jset.obj(), set_iterator));
+  DCHECK(!iter.is_null());
+  CheckException(env);
+
+  // Loop over them.

[nyquist, 2013/02/20 07:33:09]

Nit: Technically you don't start looping yet, you're only getting jmethodIDs.

[Yaron, 2013/02/21 21:39:15]

Dropped most comments as with bindings, the readability is improved

+  ScopedJavaLocalRef<jclass> iter_clazz = GetClass(env, "java/util/Iterator");
+  jmethodID iter_hasnext = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, iter_clazz.obj(), "hasNext", "()Z");
+  jmethodID iter_next = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, iter_clazz.obj(), "next", "()Ljava/lang/Object;");
+
+  ScopedJavaLocalRef<jclass> map_entry_clazz =
+      GetClass(env, "java/util/Map$Entry");
+  jmethodID map_entry_getkey = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, map_entry_clazz.obj(), "getKey", "()Ljava/lang/Object;");
+  jmethodID map_entry_getvalue = MethodID::Get<MethodID::TYPE_INSTANCE>(
+      env, map_entry_clazz.obj(), "getValue", "()Ljava/lang/Object;");
+
+  // Dump items into a std::map.
+  while (env->CallBooleanMethod(iter.obj(), iter_hasnext)) {
+    CheckException(env);
+    ScopedJavaLocalRef<jobject> map_entry(env,
+        env->CallObjectMethod(iter.obj(), iter_next));
+    DCHECK(env->IsInstanceOf(map_entry.obj(), map_entry_clazz.obj()));
+    ScopedJavaLocalRef<jstring> jkey(env,
+        static_cast<jstring>(env->CallObjectMethod(map_entry.obj(),
+                                                   map_entry_getkey)));
+    ScopedJavaLocalRef<jstring> jvalue(env,
+        static_cast<jstring>(env->CallObjectMethod(map_entry.obj(),
+                                                   map_entry_getvalue)));
+    // For profiles, jvalue can be null if not found in the contacts database.

[nyquist, 2013/02/20 07:33:09]

Which contacts database? What does this have to do with profiles?

[Yaron, 2013/02/21 21:39:15]

dropped. Copied costant from the original implementation that became a shared
function.

+    if (!jkey.is_null() && !jvalue.is_null() &&
+        env->GetStringLength(jvalue.obj())) {
+      cmap[ConvertJavaStringToUTF8(jkey)] = ConvertJavaStringToUTF16(jvalue);
+    }
+  }
+  return cmap;
+}
+
+
+}  // namespace android
+}  // namespace base