Bug 1187552 - Support direct ownership of C++ objects by Java objects; r=snorp
authorJim Chen <nchen@mozilla.com>
Tue, 04 Aug 2015 17:47:28 -0400
changeset 256193 20a9ea2255877e3697c4838eb372b3fe8bc8fc54
parent 256192 d2eaeb3217f43f87c4d5b10a85ef7fb2b4d04ac6
child 256194 6696b897b06e06a743ce6f78ce72dc74408c56f7
push id63252
push usernchen@mozilla.com
push dateTue, 04 Aug 2015 21:47:56 +0000
treeherdermozilla-inbound@5a5b8c293906 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1187552
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 1187552 - Support direct ownership of C++ objects by Java objects; r=snorp Add a direct ownership model where the Java object owns the corresponding C++ object directly, in addition to the WeakPtr model where the Java object owns a WeakPtr to the C++ object. The WeakPtr model is chosen when the implementing C++ class inherits from SupportsWeakPtr. Otherwise, the direct ownership model is chosen. Under the direct ownership model, a UniquePtr object must be used to attach the containing C++ object to a Java object, to ensure ownership is passed on to the Java object.
widget/android/jni/Natives.h
widget/android/jni/Utils.cpp
--- a/widget/android/jni/Natives.h
+++ b/widget/android/jni/Natives.h
@@ -1,69 +1,181 @@
 #ifndef mozilla_jni_Natives_h__
 #define mozilla_jni_Natives_h__
 
 #include <jni.h>
 
+#include "mozilla/Move.h"
+#include "mozilla/TypeTraits.h"
+#include "mozilla/UniquePtr.h"
 #include "mozilla/WeakPtr.h"
 #include "mozilla/jni/Accessors.h"
 #include "mozilla/jni/Refs.h"
 #include "mozilla/jni/Types.h"
 #include "mozilla/jni/Utils.h"
 
 namespace mozilla {
 namespace jni {
 
-// Get the native pointer stored in a Java instance.
-template<class Impl>
-Impl* GetNativePtr(JNIEnv* env, jobject instance)
-{
-    const auto ptr = reinterpret_cast<const WeakPtr<Impl>*>(
-            GetNativeHandle(env, instance));
-    if (!ptr) {
-        return nullptr;
-    }
+/**
+ * C++ classes implementing instance (non-static) native methods can choose
+ * from one of two ownership models, when associating a C++ object with a Java
+ * instance.
+ *
+ * * If the C++ class inherits from mozilla::SupportsWeakPtr, weak pointers
+ *   will be used. The Java instance will store and own the pointer to a
+ *   WeakPtr object. The C++ class itself is otherwise not owned or directly
+ *   referenced. To attach a Java instance to a C++ instance, pass in a pointer
+ *   to the C++ class (i.e. MyClass*).
+ *
+ *   class MyClass : public SupportsWeakPtr<MyClass>
+ *                 , public MyJavaClass::Natives<MyClass>
+ *   {
+ *       // ...
+ *
+ *   public:
+ *       MOZ_DECLARE_WEAKREFERENCE_TYPENAME(MyClass)
+ *       using MyJavaClass::Natives<MyClass>::Dispose;
+ *
+ *       void AttachTo(const MyJavaClass::LocalRef& instance)
+ *       {
+ *           MyJavaClass::Natives<MyClass>::AttachInstance(instance, this);
+ *
+ *           // "instance" does NOT own "this", so the C++ object
+ *           // lifetime is separate from the Java object lifetime.
+ *       }
+ *   };
+ *
+ * * If the C++ class doesn't inherit from mozilla::SupportsWeakPtr, the Java
+ *   instance will store and own a pointer to the C++ object itself. This
+ *   pointer must not be stored or deleted elsewhere. To attach a Java instance
+ *   to a C++ instance, pass in a reference to a UniquePtr of the C++ class
+ *   (i.e. UniquePtr<MyClass>).
+ *
+ *   class MyClass : public MyJavaClass::Natives<MyClass>
+ *   {
+ *       // ...
+ *
+ *   public:
+ *       using MyJavaClass::Natives<MyClass>::Dispose;
+ *
+ *       static void AttachTo(const MyJavaClass::LocalRef& instance)
+ *       {
+ *           MyJavaClass::Natives<MyClass>::AttachInstance(
+ *                   instance, mozilla::MakeUnique<MyClass>());
+ *
+ *           // "instance" owns the newly created C++ object, so the C++
+ *           // object is destroyed as soon as instance.dispose() is called.
+ *       }
+ *   };
+ */
 
-    Impl* const impl = *ptr;
-    if (!impl) {
-        ThrowException(env, "java/lang/NullPointerException",
-                       "Native object already released");
+namespace {
+
+uintptr_t CheckNativeHandle(JNIEnv* env, uintptr_t handle)
+{
+    if (!handle) {
+        if (!env->ExceptionCheck()) {
+            ThrowException(env, "java/lang/NullPointerException",
+                           "Null native pointer");
+        }
+        return 0;
     }
-    return impl;
-}
-
-template<class Impl, class LocalRef>
-Impl* GetNativePtr(const LocalRef& instance)
-{
-    return GetNativePtr<Impl>(instance.Env(), instance.Get());
+    return handle;
 }
 
-template<class Impl, class LocalRef>
-void ClearNativePtr(const LocalRef& instance)
+template<class Impl, bool UseWeakPtr = mozilla::IsBaseOf<
+                         SupportsWeakPtr<Impl>, Impl>::value /* = false */>
+struct NativePtr
+{
+    static Impl* Get(JNIEnv* env, jobject instance)
+    {
+        return reinterpret_cast<Impl*>(CheckNativeHandle(
+                env, GetNativeHandle(env, instance)));
+    }
+
+    template<class LocalRef>
+    static Impl* Get(const LocalRef& instance)
+    {
+        return Get(instance.Env(), instance.Get());
+    }
+
+    template<class LocalRef>
+    static void Set(const LocalRef& instance, UniquePtr<Impl>&& ptr)
+    {
+        Clear(instance);
+        SetNativeHandle(instance.Env(), instance.Get(),
+                          reinterpret_cast<uintptr_t>(ptr.release()));
+        HandleUncaughtException(instance.Env());
+    }
+
+    template<class LocalRef>
+    static void Clear(const LocalRef& instance)
+    {
+        UniquePtr<Impl> ptr(reinterpret_cast<Impl*>(
+                GetNativeHandle(instance.Env(), instance.Get())));
+        HandleUncaughtException(instance.Env());
+
+        if (ptr) {
+            SetNativeHandle(instance.Env(), instance.Get(), 0);
+            HandleUncaughtException(instance.Env());
+        }
+    }
+};
+
+template<class Impl>
+struct NativePtr<Impl, /* UseWeakPtr = */ true>
 {
-    JNIEnv* const env = instance.Env();
-    const auto ptr = reinterpret_cast<WeakPtr<Impl>*>(
-            GetNativeHandle(env, instance.Get()));
-    if (ptr) {
-        SetNativeHandle(env, instance.Get(), 0);
-        delete ptr;
-    } else {
-        // GetNativeHandle throws an exception when returning null.
-        MOZ_ASSERT(env->ExceptionCheck());
-        env->ExceptionClear();
+    static Impl* Get(JNIEnv* env, jobject instance)
+    {
+        const auto ptr = reinterpret_cast<WeakPtr<Impl>*>(
+                CheckNativeHandle(env, GetNativeHandle(env, instance)));
+        if (!ptr) {
+            return nullptr;
+        }
+
+        Impl* const impl = *ptr;
+        if (!impl) {
+            ThrowException(env, "java/lang/NullPointerException",
+                           "Native object already released");
+        }
+        return impl;
+    }
+
+    template<class LocalRef>
+    static Impl* Get(const LocalRef& instance)
+    {
+        return Get(instance.Env(), instance.Get());
     }
-}
+
+    template<class LocalRef>
+    static void Set(const LocalRef& instance, SupportsWeakPtr<Impl>* ptr)
+    {
+        Clear(instance);
+        SetNativeHandle(instance.Env(), instance.Get(),
+                          reinterpret_cast<uintptr_t>(new WeakPtr<Impl>(ptr)));
+        HandleUncaughtException(instance.Env());
+    }
 
-template<class Impl, class LocalRef>
-void SetNativePtr(const LocalRef& instance, Impl* ptr)
-{
-    ClearNativePtr<Impl>(instance);
-    SetNativeHandle(instance.Env(), instance.Get(),
-                      reinterpret_cast<uintptr_t>(new WeakPtr<Impl>(ptr)));
-}
+    template<class LocalRef>
+    static void Clear(const LocalRef& instance)
+    {
+        const auto ptr = reinterpret_cast<WeakPtr<Impl>*>(
+                GetNativeHandle(instance.Env(), instance.Get()));
+        HandleUncaughtException(instance.Env());
+
+        if (ptr) {
+            SetNativeHandle(instance.Env(), instance.Get(), 0);
+            HandleUncaughtException(instance.Env());
+            delete ptr;
+        }
+    }
+};
+
+} // namespace
 
 namespace detail {
 
 // Wrapper methods that convert arguments from the JNI types to the native
 // types, e.g. from jobject to jni::Object::Ref. For instance methods, the
 // wrapper methods also convert calls to calls on objects.
 //
 // We need specialization for static/non-static because the two have different
@@ -83,30 +195,30 @@ class NativeStubImpl<false, ReturnType, 
     typedef typename TypeAdapter<ReturnType>::JNIType ReturnJNIType;
 
 public:
     // Instance method
     template<ReturnType (Impl::*Method) (Args...)>
     static ReturnJNIType Wrap(JNIEnv* env, jobject instance,
                               typename TypeAdapter<Args>::JNIType... args)
     {
-        Impl* const impl = GetNativePtr<Impl>(env, instance);
+        Impl* const impl = NativePtr<Impl>::Get(env, instance);
         if (!impl) {
             return ReturnJNIType();
         }
         return TypeAdapter<ReturnType>::FromNative(env,
                 (impl->*Method)(TypeAdapter<Args>::ToNative(env, args)...));
     }
 
     // Instance method with instance reference
     template<ReturnType (Impl::*Method) (const typename Owner::LocalRef&, Args...)>
     static ReturnJNIType Wrap(JNIEnv* env, jobject instance,
                               typename TypeAdapter<Args>::JNIType... args)
     {
-        Impl* const impl = GetNativePtr<Impl>(env, instance);
+        Impl* const impl = NativePtr<Impl>::Get(env, instance);
         if (!impl) {
             return ReturnJNIType();
         }
         auto self = Owner::LocalRef::Adopt(env, instance);
         const auto res = TypeAdapter<ReturnType>::FromNative(env,
                 (impl->*Method)(self, TypeAdapter<Args>::ToNative(env, args)...));
         self.Forget();
         return res;
@@ -120,29 +232,29 @@ class NativeStubImpl<false, void, Traits
     typedef typename Traits::Owner Owner;
 
 public:
     // Instance method
     template<void (Impl::*Method) (Args...)>
     static void Wrap(JNIEnv* env, jobject instance,
                      typename TypeAdapter<Args>::JNIType... args)
     {
-        Impl* const impl = GetNativePtr<Impl>(env, instance);
+        Impl* const impl = NativePtr<Impl>::Get(env, instance);
         if (!impl) {
             return;
         }
         (impl->*Method)(TypeAdapter<Args>::ToNative(env, args)...);
     }
 
     // Instance method with instance reference
     template<void (Impl::*Method) (const typename Owner::LocalRef&, Args...)>
     static void Wrap(JNIEnv* env, jobject instance,
                      typename TypeAdapter<Args>::JNIType... args)
     {
-        Impl* const impl = GetNativePtr<Impl>(env, instance);
+        Impl* const impl = NativePtr<Impl>::Get(env, instance);
         if (!impl) {
             return;
         }
         auto self = Owner::LocalRef::Adopt(env, instance);
         (impl->*Method)(self, TypeAdapter<Args>::ToNative(env, args)...);
         self.Forget();
     }
 };
@@ -240,31 +352,47 @@ public:
         MOZ_ALWAYS_TRUE(!env->RegisterNatives(
                 Accessor::EnsureClassRef<Cls>(env),
                  Natives::methods,
                  sizeof(Natives::methods) / sizeof(Natives::methods[0])));
         sInited = true;
     }
 
 protected:
+
+    // Associate a C++ instance with a Java instance.
+    static void AttachNative(const typename Cls::LocalRef& instance,
+                             SupportsWeakPtr<Impl>* ptr)
+    {
+        static_assert(mozilla::IsBaseOf<SupportsWeakPtr<Impl>, Impl>::value,
+                      "Attach with UniquePtr&& when not using WeakPtr");
+        return NativePtr<Impl>::Set(instance, ptr);
+    }
+
+    static void AttachNative(const typename Cls::LocalRef& instance,
+                             UniquePtr<Impl>&& ptr)
+    {
+        static_assert(!mozilla::IsBaseOf<SupportsWeakPtr<Impl>, Impl>::value,
+                      "Attach with SupportsWeakPtr* when using WeakPtr");
+        return NativePtr<Impl>::Set(instance, mozilla::Move(ptr));
+    }
+
+    // Get the C++ instance associated with a Java instance.
+    // There is always a pending exception if the return value is nullptr.
     static Impl* GetNative(const typename Cls::LocalRef& instance) {
-        return GetNativePtr<Impl>(instance);
+        return NativePtr<Impl>::Get(instance);
     }
 
     NativeImpl() {
         // Initialize on creation if not already initialized.
         Init();
     }
 
-    void AttachNative(const typename Cls::LocalRef& instance) {
-        SetNativePtr<>(instance, static_cast<Impl*>(this));
-    }
-
     void DisposeNative(const typename Cls::LocalRef& instance) {
-        ClearNativePtr<Impl>(instance);
+        NativePtr<Impl>::Clear(instance);
     }
 };
 
 // Define static member.
 template<class C, class I>
 bool NativeImpl<C, I>::sInited;
 
 } // namespace jni
--- a/widget/android/jni/Utils.cpp
+++ b/widget/android/jni/Utils.cpp
@@ -106,24 +106,18 @@ bool EnsureJNIObject(JNIEnv* env, jobjec
 } // namespace
 
 uintptr_t GetNativeHandle(JNIEnv* env, jobject instance)
 {
     if (!EnsureJNIObject(env, instance)) {
         return 0;
     }
 
-    auto handle = static_cast<uintptr_t>(
+    return static_cast<uintptr_t>(
             env->GetLongField(instance, sJNIObjectHandleField));
-
-    if (!handle && !env->ExceptionCheck()) {
-        ThrowException(env, "java/lang/NullPointerException",
-                       "Null native pointer");
-    }
-    return handle;
 }
 
 void SetNativeHandle(JNIEnv* env, jobject instance, uintptr_t handle)
 {
     if (!EnsureJNIObject(env, instance)) {
         return;
     }