New JNI bindings

jni/Utils.cpp

Index: jni/Utils.cpp
diff --git a/jni/Utils.cpp b/jni/Utils.cpp
index b4c7cb60e982458591b1c75958a25b3f9c13912c..9ebf1fcb9d8b443293bd67e7a7f406026867c68e 100644
--- a/jni/Utils.cpp
+++ b/jni/Utils.cpp
@@ -15,6 +15,8 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <string>
+
 #include "Utils.h"
 #include "Debug.h"
 
@@ -36,3 +38,69 @@ const std::string GetString(JNIEnv *pEnv, jstring str)
 
   return value;
 }
+
+std::string JniJavaToStdString(JNIEnv* env, jstring str)
+{
+  if (!str)
+  {
+    return std::string();
+  }
+
+  const char* cStr = env->GetStringUTFChars(str, 0);
+  std::string ret(cStr);
+  env->ReleaseStringUTFChars(str, cStr);
+
+  return ret;
+}
+
+jobject NewJniArrayList(JNIEnv* env)
+{
+  jclass clazz = env->FindClass("java/util/ArrayList");
+  jmethodID ctor = env->GetMethodID(clazz, "<init>", "()V");
+  return env->NewObject(clazz, ctor);
+}
+
+void JniAddObjectToList(JNIEnv* env, jobject list, jobject value)
+{
+  jmethodID add = env->GetMethodID(env->GetObjectClass(list), "add", "(Ljava/lang/Object;)Z");
+  env->CallBooleanMethod(list, add, value);
+}
+
+void JniThrowException(JNIEnv* env, const std::string& message)
+{
+  jclass clazz = env->FindClass(PKG("AdblockPlusException"));

[Felix Dahlke, 2014/03/28 17:28:41]

We're only using AdblockPlusException for native exceptions (which should only
come from libadblockplus), right? I'd vote for a more telling name, maybe
("LibAdblockPlusException") or ("NativeAdblockPlusException"), not sure. What do
you think?

[René Jeschke, 2014/03/31 09:10:38]

I would also like to use the "AdblockPlusException" for all exceptions thrown by
the Java code. This way it's easier to say if it's a system error or something
we don't like. So I would like to keep the name.

[Felix Dahlke, 2014/03/31 10:43:23]

Alright.

[René Jeschke, 2014/04/11 12:25:53]

Done.

+  env->ThrowNew(clazz, message.c_str());
+}
+
+void JniThrowException(JNIEnv* env, const std::exception& e)
+{
+  JniThrowException(env, e.what());
+}
+
+void JniThrowException(JNIEnv* env)
+{
+  JniThrowException(env, "Unknown exception from libAdblockPlus");

[Felix Dahlke, 2014/03/28 17:28:41]

We usually call it "libadblockplus".

[René Jeschke, 2014/04/11 12:25:53]

Done.

+}
+
+JNIEnvAcquire::JNIEnvAcquire(JavaVM* javaVM)
+  : javaVM(javaVM), jniEnv(0), attachmentStatus(0)
+{
+  attachmentStatus = javaVM->GetEnv((void **)&jniEnv, ABP_JNI_VERSION);
+  if (attachmentStatus == JNI_EDETACHED)
+  {
+    if (javaVM->AttachCurrentThread(&jniEnv, 0))
+    {
+      // This one is FATAL, we can't recover from this (because without a JVM we're dead), so
+      // throwing a runtime_exception in a ctor can be tolerated here IMHO

[Felix Dahlke, 2014/03/28 17:28:41]

Throwing a std::runtime_error from a constructor is perfectly fine IMO, doesn't
need reasoning - std::runtime_error is really just a general purpose exception
base class, and exceptions are how error handling with constructors is supposed
to work.

You seem to think differently?

[René Jeschke, 2014/03/31 09:10:38]

When using 'new' and exceptions from the ctor you get an allocated but not
initialized object. So I really don't like throwing exceptions in a ctor and
prefer static factory methods that include cleanup on failure.

[Felix Dahlke, 2014/03/31 10:43:23]

Are we talking about C++ constructors? I'm absolutely sure that no memory is
getting allocated if the constructor fails.

[René Jeschke, 2014/04/11 12:25:53]

Done.

+      throw std::runtime_error("Failed to get JNI environment");
+    }
+  }
+}
+
+JNIEnvAcquire::~JNIEnvAcquire()
+{
+  if (attachmentStatus == JNI_EDETACHED)
+  {
+    javaVM->DetachCurrentThread();
+  }
+}