Bug 1186530 - Fix compile error when using native methods with ref parameters; r=snorp
authorJim Chen <nchen@mozilla.com>
Wed, 29 Jul 2015 15:11:15 -0400
changeset 286927 866e22abf3714ee8af9a8de4b2c9206b8be6ae6e
parent 286926 d9f1a6f4ede03c4d08125623614a2e529477eae5
child 286928 0229401cfa6940f5f659059386dbcda986873024
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1186530
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1186530 - Fix compile error when using native methods with ref parameters; r=snorp We use Ref::From() inside TypeAdapter<Ref>::ToNative to convert a raw JNI ref argument to a Ref argument for the C++ function. However, that generates a compile error, unless we make TypeAdapter<Ref> a friend of Ref, because we intentionally made Ref's copy constructor private and returning from TypeAdapter<Ref>::ToNative requires the copy constructor.
widget/android/jni/Accessors.h
widget/android/jni/Refs.h
widget/android/jni/Types.h
widget/android/jni/Utils.cpp
--- a/widget/android/jni/Accessors.h
+++ b/widget/android/jni/Accessors.h
@@ -1,14 +1,13 @@
 #ifndef mozilla_jni_Accessors_h__
 #define mozilla_jni_Accessors_h__
 
 #include <jni.h>
 
-#include "mozilla/Attributes.h"
 #include "mozilla/jni/Refs.h"
 #include "mozilla/jni/Types.h"
 #include "AndroidBridge.h"
 
 namespace mozilla {
 namespace jni{
 
 namespace {
@@ -201,22 +200,16 @@ public:
 template<class Traits>
 class Field : public Accessor
 {
     typedef Accessor Base;
     typedef class Traits::Owner Owner;
     typedef typename Traits::ReturnType GetterType;
     typedef typename Traits::SetterType SetterType;
 
-    template<typename T> struct RemoveRef { typedef T Type; };
-    template<typename T> struct RemoveRef<const T&> { typedef T Type; };
-
-    // Setter type without any const/& added
-    typedef typename RemoveRef<SetterType>::Type SetterBaseType;
-
 private:
 
     static jfieldID sID;
 
     static JNIEnv* BeginAccess()
     {
         JNIEnv* const env = Base::BeginAccess<Traits>();
 
@@ -257,23 +250,23 @@ public:
         return result;
     }
 
     static void Set(const Owner* cls, nsresult* rv, SetterType val)
     {
         JNIEnv* const env = BeginAccess();
 
         if (Traits::isStatic) {
-            (env->*TypeAdapter<SetterBaseType>::StaticSet)(
+            (env->*TypeAdapter<SetterType>::StaticSet)(
                     Owner::sClassRef, sID,
-                    TypeAdapter<SetterBaseType>::FromNative(env, val));
+                    TypeAdapter<SetterType>::FromNative(env, val));
         } else {
-            (env->*TypeAdapter<SetterBaseType>::Set)(
+            (env->*TypeAdapter<SetterType>::Set)(
                     cls->mInstance, sID,
-                    TypeAdapter<SetterBaseType>::FromNative(env, val));
+                    TypeAdapter<SetterType>::FromNative(env, val));
         }
 
         EndAccess(env, rv);
     }
 };
 
 // Define sID member.
 template<class T> jfieldID Field<T>::sID;
--- a/widget/android/jni/Refs.h
+++ b/widget/android/jni/Refs.h
@@ -26,16 +26,22 @@ template<class Cls> class GlobalRef;
 
 // Type used for a reference parameter. Default is a wrapped object
 // reference, but ParamImpl can be specialized to define custom behavior,
 // e.g. a StringParam class that automatically converts nsAString& and
 // nsACString& to a jstring.
 template<class Cls> struct ParamImpl { typedef Ref<Cls> Type; };
 template<class Cls> using Param = typename ParamImpl<Cls>::Type;
 
+namespace detail {
+
+template<class Cls> struct TypeAdapter;
+
+} // namespace detail
+
 
 // How exception during a JNI call should be treated.
 enum class ExceptionMode
 {
     // Abort on unhandled excepion (default).
     ABORT,
     // Ignore the exception and return to caller.
     IGNORE,
@@ -201,16 +207,17 @@ public:
 
 // Wrapped object reference (e.g. jobject, jclass, etc...)
 template<class Cls>
 class Ref : public RefBase<Cls, jobject>
 {
     template<class C, typename T> friend class RefBase;
     friend class jni::LocalRef<Cls>;
     friend class jni::GlobalRef<Cls>;
+    friend class detail::TypeAdapter<Ref<Cls>>;
 
     typedef RefBase<Cls, jobject> Base;
 
 protected:
     // Protected jobject constructor because outside code should be using
     // Ref::From. Using Ref::From makes it very easy to see which code is using
     // raw JNI types for future refactoring.
     Ref(jobject instance) : Base(instance) {}
@@ -234,16 +241,17 @@ RefBase<Cls, JNIType>::operator Ref<Obje
 
 template<typename T>
 class Ref<TypedObject<T>>
         : public RefBase<TypedObject<T>, T>
 {
     friend class RefBase<TypedObject<T>, T>;
     friend class jni::LocalRef<TypedObject<T>>;
     friend class jni::GlobalRef<TypedObject<T>>;
+    friend class detail::TypeAdapter<Ref<TypedObject<T>>>;
 
     typedef RefBase<TypedObject<T>, T> Base;
 
 protected:
     Ref(jobject instance) : Base(instance) {}
 
     Ref(const Ref& ref) : Base(ref.mInstance) {}
 
@@ -486,16 +494,17 @@ public:
 
 // Ref specialization for jstring.
 template<>
 class Ref<String> : public RefBase<String, jstring>
 {
     friend class RefBase<String, jstring>;
     friend class jni::LocalRef<String>;
     friend class jni::GlobalRef<String>;
+    friend class detail::TypeAdapter<Ref<String>>;
 
     typedef RefBase<TypedObject<jstring>, jstring> Base;
 
 protected:
     Ref(jobject instance) : Base(instance) {}
 
     Ref(const Ref& ref) : Base(ref.mInstance) {}
 
--- a/widget/android/jni/Types.h
+++ b/widget/android/jni/Types.h
@@ -1,21 +1,20 @@
 #ifndef mozilla_jni_Types_h__
 #define mozilla_jni_Types_h__
 
 #include <jni.h>
 
 #include "mozilla/jni/Refs.h"
-#include "AndroidBridge.h"
 
 namespace mozilla {
 namespace jni {
-namespace {
+namespace detail {
 
-// TypeAdapter specializations are the interfaces between naive (C++) types such
+// TypeAdapter specializations are the interfaces between native/C++ types such
 // as int32_t and JNI types such as jint. The template parameter T is the native
 // type, and each TypeAdapter specialization can have the following members:
 //
 //  * Call: JNIEnv member pointer for making a method call that returns T.
 //  * StaticCall: JNIEnv member pointer for making a static call that returns T.
 //  * Get: JNIEnv member pointer for getting a field of type T.
 //  * StaticGet: JNIEnv member pointer for getting a static field of type T.
 //  * Set: JNIEnv member pointer for setting a field of type T.
@@ -30,16 +29,18 @@ template<typename T> struct TypeAdapter;
 template<class Cls> struct TypeAdapter<LocalRef<Cls>> {
     typedef decltype(Ref<Cls>(nullptr).Get()) JNIType;
 
     static constexpr auto Call = &JNIEnv::CallObjectMethodA;
     static constexpr auto StaticCall = &JNIEnv::CallStaticObjectMethodA;
     static constexpr auto Get = &JNIEnv::GetObjectField;
     static constexpr auto StaticGet = &JNIEnv::GetStaticObjectField;
 
+    // Declare instance as jobject because JNI methods return
+    // jobject even if the return value is really jstring, etc.
     static LocalRef<Cls> ToNative(JNIEnv* env, jobject instance) {
         return LocalRef<Cls>::Adopt(env, instance);
     }
 
     static JNIType FromNative(JNIEnv*, LocalRef<Cls>&& instance) {
         return instance.Forget();
     }
 };
@@ -56,17 +57,17 @@ template<class Cls> constexpr jobject
 
 // TypeAdapter<Ref<Cls>> applies when jobject is a parameter value.
 template<class Cls> struct TypeAdapter<Ref<Cls>> {
     typedef decltype(Ref<Cls>(nullptr).Get()) JNIType;
 
     static constexpr auto Set = &JNIEnv::SetObjectField;
     static constexpr auto StaticSet = &JNIEnv::SetStaticObjectField;
 
-    static Ref<Cls> ToNative(JNIEnv* env, jobject instance) {
+    static Ref<Cls> ToNative(JNIEnv* env, JNIType instance) {
         return Ref<Cls>::From(instance);
     }
 
     static JNIType FromNative(JNIEnv*, const Ref<Cls>& instance) {
         return instance.Get();
     }
 };
 
@@ -76,16 +77,20 @@ template<class Cls> constexpr void
     (JNIEnv::*TypeAdapter<Ref<Cls>>::StaticSet)(jclass, jfieldID, jobject);
 
 
 // jstring has its own Param type.
 template<> struct TypeAdapter<Param<String>>
         : public TypeAdapter<String::Ref>
 {};
 
+template<class Cls> struct TypeAdapter<const Cls&>
+        : public TypeAdapter<Cls>
+{};
+
 
 #define DEFINE_PRIMITIVE_TYPE_ADAPTER(NativeType, JNIType, JNIName) \
     \
     template<> struct TypeAdapter<NativeType> { \
         typedef JNIType JNI##Type; \
     \
         static constexpr auto Call = &JNIEnv::Call ## JNIName ## MethodA; \
         static constexpr auto StaticCall = &JNIEnv::CallStatic ## JNIName ## MethodA; \
@@ -95,40 +100,30 @@ template<> struct TypeAdapter<Param<Stri
         static constexpr auto StaticSet = &JNIEnv::SetStatic ## JNIName ## Field; \
     \
         static JNIType FromNative(JNIEnv*, NativeType val) { \
             return static_cast<JNIType>(val); \
         } \
         static NativeType ToNative(JNIEnv*, JNIType val) { \
             return static_cast<NativeType>(val); \
         } \
-    }; \
-    \
-    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::Call) \
-            (jobject, jmethodID, jvalue*); \
-    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::StaticCall) \
-            (jclass, jmethodID, jvalue*); \
-    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::Get) \
-            (jobject, jfieldID); \
-    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::StaticGet) \
-            (jclass, jfieldID); \
-    constexpr void (JNIEnv::*TypeAdapter<NativeType>::Set) \
-            (jobject, jfieldID, JNIType); \
-    constexpr void (JNIEnv::*TypeAdapter<NativeType>::StaticSet) \
-            (jclass, jfieldID, JNIType)
+    }
 
 
 DEFINE_PRIMITIVE_TYPE_ADAPTER(bool,     jboolean, Boolean);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(int8_t,   jbyte,    Byte);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(char16_t, jchar,    Char);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(int16_t,  jshort,   Short);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(int32_t,  jint,     Int);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(int64_t,  jlong,    Long);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(float,    jfloat,   Float);
 DEFINE_PRIMITIVE_TYPE_ADAPTER(double,   jdouble,  Double);
 
 #undef DEFINE_PRIMITIVE_TYPE_ADAPTER
 
-} // namespace
+} // namespace detail
+
+using namespace detail;
+
 } // namespace jni
 } // namespace mozilla
 
 #endif // mozilla_jni_Types_h__
--- a/widget/android/jni/Utils.cpp
+++ b/widget/android/jni/Utils.cpp
@@ -1,18 +1,50 @@
 #include "Utils.h"
+#include "Types.h"
 
 #include "mozilla/Assertions.h"
 
 #include "AndroidBridge.h"
 #include "GeneratedJNIWrappers.h"
 
 namespace mozilla {
 namespace jni {
 
+namespace detail {
+
+#define DEFINE_PRIMITIVE_TYPE_ADAPTER(NativeType, JNIType, JNIName) \
+    \
+    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::Call) \
+            (jobject, jmethodID, jvalue*); \
+    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::StaticCall) \
+            (jclass, jmethodID, jvalue*); \
+    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::Get) \
+            (jobject, jfieldID); \
+    constexpr JNIType (JNIEnv::*TypeAdapter<NativeType>::StaticGet) \
+            (jclass, jfieldID); \
+    constexpr void (JNIEnv::*TypeAdapter<NativeType>::Set) \
+            (jobject, jfieldID, JNIType); \
+    constexpr void (JNIEnv::*TypeAdapter<NativeType>::StaticSet) \
+            (jclass, jfieldID, JNIType)
+
+DEFINE_PRIMITIVE_TYPE_ADAPTER(bool,     jboolean, Boolean);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(int8_t,   jbyte,    Byte);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(char16_t, jchar,    Char);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(int16_t,  jshort,   Short);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(int32_t,  jint,     Int);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(int64_t,  jlong,    Long);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(float,    jfloat,   Float);
+DEFINE_PRIMITIVE_TYPE_ADAPTER(double,   jdouble,  Double);
+
+#undef DEFINE_PRIMITIVE_TYPE_ADAPTER
+
+} // namespace detail
+
+
 bool ThrowException(JNIEnv *aEnv, const char *aClass,
                     const char *aMessage)
 {
     MOZ_ASSERT(aEnv, "Invalid thread JNI env");
 
     ClassObject::LocalRef cls =
             ClassObject::LocalRef::Adopt(aEnv->FindClass(aClass));
     MOZ_ASSERT(cls, "Cannot find exception class");