New JNI bindings

jni/Utils.h

 /*
  * 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 10 matching lines @@
 #include <string>
 #include <algorithm>
 #include <jni.h>
 #include <stdexcept>
 
 #include <AdblockPlus.h>
 #include <AdblockPlus/tr1_memory.h>
 
 #include "v8/v8stdint.h"
 
-#define PKG(x) "org/adblockplus/android/api/" x

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

PKG is fine, but I'd personally vote for "PACKAGE" since we tend to not
abbreviate stuff much. But it's up to you.

More importantly, I'm wondering if we need some kind of prefix for all macros,
Mozilla does it. "ABP_" comes to mind. What do you think?

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

I prefer PKG because it's only 3 chars. As there are only 2 macros that uses
this short naming schema (TYP & PKG) and as both are intended to be used
'inline' with/for string creation I really would like to keep them as short as
possible.

As for the "ABP_" extension: There are currently no name clashes and we're also
not exporting/reusing any of the code elsewhere, so we should be fine without
the prefix IMO.

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

OK, fine with having no prefix. Also fine with PKG if you prefer it.

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

Done.

+#define PKG(x) "org/adblockplus/libadblockplus/" x
 #define TYP(x) "L" PKG(x) ";"

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

Correct me if I'm wrong, but it seems the proper name for the "Lfoo/Bar;" thing
seems to be "name", that's how I understand it here:

http://docs.oracle.com/javase/6/docs/api/java/lang/Class.html#getName%28%29

Based on that, I think we should call this macro NAME.

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

That's not quite correct. The name of the class consists of the class name and
package name(s) joined by dots, so "foo.Bar". "Lfoo/Bar;" is a type signature as
is I, B, Z, J, ...

I'll explain what the macros do and why I called them PKG and TYP:

PKG("Bar") will return "foo/Bar", so it prefixes the given simple class name
with a fixed package and returns a FindClass compatible class name.

TYP("Bar") will return "Lfoo/Bar;", so it prefixes the package and wraps
everything in a type definition for method signatures.

First I was thinking about naming them PACKAGE and TYPE, but decided against
this for several reasons:

1. TYPE will definitely clash with something ... definitely
2. TYPE is 4 letters, PACKAGE is 7 letters, I hate asymmetry 
3. As because of #1 I must change TYPE to anything and didn't want to make the
macro longer, I came up with PKG and TYP

I would really like to keep both macros unchanged but it's up to you to make the
final decision.

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

Na it's not, we should both be able to understand the code. I can live with TYP,
your call.

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

Done.

 
 #define ABP_JNI_VERSION JNI_VERSION_1_6
 
 void JniThrowException(JNIEnv* env, const std::string& message);
 
 void JniThrowException(JNIEnv* env, const std::exception& e);
 
 void JniThrowException(JNIEnv* env);
 
 class JNIEnvAcquire
@@ skipping 18 matching lines @@
   int attachmentStatus;
 };
 
 template<typename T>
 class JniGlobalReference
 {
 public:
   JniGlobalReference(JNIEnv* env, T reference)
   {
     env->GetJavaVM(&javaVM);
-    reference = static_cast<T>(env->NewGlobalRef(static_cast<jobject>(reference)));
+    this->reference = static_cast<T>(env->NewGlobalRef(static_cast<jobject>(reference)));
   }
 
   ~JniGlobalReference()
   {
     JNIEnvAcquire env(javaVM);
-
     env->DeleteGlobalRef(static_cast<jobject>(reference));
   }
 
   JniGlobalReference(const JniGlobalReference& other);
   JniGlobalReference& operator=(const JniGlobalReference& other);
 
-  T get()

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

All functions should start with upper case letters.

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

Done.

+  T Get()
   {
     return reference;
   }
 
   typedef std::tr1::shared_ptr<JniGlobalReference<T> > Ptr;
 
 private:
   T reference;
   JavaVM* javaVM;
 };
@@ skipping 51 matching lines @@
   return env->NewObject(clazz, method, JniPtrToLong(new AdblockPlus::FilterPtr(filter)));
 }
 
 inline jobject NewJniSubscription(JNIEnv* env, const AdblockPlus::SubscriptionPtr& subscription)
 {
   jclass clazz = env->FindClass(PKG("Subscription"));
   jmethodID method = env->GetMethodID(clazz, "<init>", "(J)V");
   return env->NewObject(clazz, method, JniPtrToLong(new AdblockPlus::SubscriptionPtr(subscription)));
 }
 
-#define TRY try

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

I believe we can live without this one :D

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

It is just for symmetry reasons as:
try
{
  // ....
}
CATCH_AND_THROW(...)

looks awkward compared to having TRY on top.

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

Using "TRY" suggests that some magic is going on here, while "try" makes it
clear that "CATCH_AND_THROW" just generates a normal catch block. I'd rather
have things look a bit asymmetric but be more clear personally.

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

Done.

-
 #define CATCH_AND_THROW(jEnv) \

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

How about "CATCH_AND_RETHROW"?

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

Rethrow would imply that I would catch the C++ exception and rethrow it after
doing something, but I am transforming a C++ exception into a Java exception and
throwing this one into the JVM, so I vote for _THROW.

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

Fair enough.

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

Done.

   catch (const std::exception& except) \
   { \
     JniThrowException(jEnv, except); \
   } \
   catch (...) \
   { \
     JniThrowException(jEnv); \
   }
 
 #define CATCH_THROW_AND_RETURN(jEnv, retVal) \
@@ skipping 14 matching lines @@
 T TrimString(T text)
 {
   // Via http://stackoverflow.com/questions/216823/whats-the-best-way-to-trim-stdstring
   T trimmed(text);
   trimmed.erase(trimmed.begin(), std::find_if(trimmed.begin(), trimmed.end(), std::not1(std::ptr_fun<int, int>(std::isspace))));
   trimmed.erase(std::find_if(trimmed.rbegin(), trimmed.rend(), std::not1(std::ptr_fun<int, int>(std::isspace))).base(), trimmed.end());
   return trimmed;
 }
 
 #endif