New JNI bindings

jni/Utils.cpp

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 50 matching lines @@
 }
 
 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.

+  JniThrowException(env, "Unknown exception from libadblockplus");
 }
 
 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();
   }
 }