Merge pull request #3371 from theuni/android-jholder-rewrite
authorCory Fields <foss@atlastechnologiesinc.com>
Fri, 11 Oct 2013 00:20:40 +0000 (17:20 -0700)
committerCory Fields <foss@atlastechnologiesinc.com>
Fri, 11 Oct 2013 00:20:40 +0000 (17:20 -0700)
Android jholder rewrite

xbmc/android/jni/Context.cpp
xbmc/android/jni/jutils/jutils-details.hpp
xbmc/android/jni/jutils/jutils.hpp

index defa2be..70e26e9 100644 (file)
@@ -63,7 +63,6 @@ CJNIContext::CJNIContext(const ANativeActivity *nativeActivity)
 CJNIContext::~CJNIContext()
 {
   m_appInstance = NULL;
-  m_context.release();
   xbmc_jni_on_unload();
 }
 
index 542a32c..9c3e833 100644 (file)
@@ -94,7 +94,7 @@ struct jcast_helper<std::string, jhstring>
 {
     static std::string cast(jhstring const &v)
     {
-        return jcast_helper<std::string, jstring>::cast(v);
+        return jcast_helper<std::string, jstring>::cast(v.get());
     }
 };
 
@@ -103,7 +103,7 @@ struct jcast_helper<std::vector<std::string>, jhobjectArray>
 {
     static std::vector<std::string> cast(jhobjectArray const &v)
     {
-        return jcast_helper<std::vector<std::string>, jobjectArray>::cast(v);
+        return jcast_helper<std::vector<std::string>, jobjectArray>::cast(v.get());
     }
 };
 
@@ -112,7 +112,7 @@ struct jcast_helper<std::vector<T>, jhobjectArray>
 {
     static std::vector<T> cast(jhobjectArray const &v)
     {
-        return jcast_helper<std::vector<T>, jobjectArray>::cast(v);
+        return jcast_helper<std::vector<T>, jobjectArray>::cast(v.get());
     }
 };
 
@@ -130,7 +130,7 @@ std::vector<T> jcast_helper<std::vector<T>, jobjectArray>::cast(jobjectArray con
 
   for (int i = 0; i < size; i++)
   {
-    jhobject element = (jhobject)env->GetObjectArrayElement(v, i);
+    T element((jhobject)env->GetObjectArrayElement(v, i));
     vec.emplace_back(element);
   }
   return vec;
index 1b9e55c..059a2c0 100644 (file)
@@ -57,99 +57,208 @@ JNIEnv *xbmc_jnienv();
 
 namespace jni
 {
+
 template <typename T>
 class jholder
 {
-    typedef void (jholder::*safe_bool_type)();
-    void non_null_object() {}
+/* A native jni types wrapper class.
+
+  This templated class is used as a container for native jni types: jobject,
+  jclass, etc. It maintains scope and references so that parent objects don't
+  have to bother.
+
+  Here, a jobject will be used as an example.
+
+  Background: JNI uses reference-counted objects to facilitate
+  garbage-collection. A jobject is really just a pointer to some shared memory.
+  When a jobject is given to native code, it has a local
+  reference. When this local reference is removed, the JVM is free to
+  garbage-collect its contents. Local references are destroyed when the jobject
+  is destroyed (or loses scope) or when Java execution resumes, or they can be
+  destroyed manually. Objects which hold local references also cannot be shared
+  between threads. To get around these limitations, the local reference can be
+  upgraded to a global one.
+
+  This class handles this logic for the user. A jobject can be moved into a
+  jhobject via the jhobject constructor. After doing so, the original jobject's
+  state is undefined because the jhobject will unref it automatically.
+
+  Scope of copies is handled as well, so no extra care needs to be taken. When
+  a copy is made, the copy inherits the scope of the original object. The
+  class tries to operate in the safest possible way when assigning objects of
+  differing scope by choosing the widest possible scope.
+
+  Example:
+
+  jhobject somefunction(jobject androidObject)
+  {
+    jhobject foo(androidObject);
+    // store androidObject in foo. androidObject should not be used again.
+    // foo has a local ref.
+
+    jhobject bar(foo);
+    // copy foo to bar. foo and bar are both local. They can both be used
+    // on this thread until returning to java. It would not be safe to return
+    // them because the caller cannot be trusted to obey the local-ref rules.
+    //
+    // Note: This copy makes no practical sense, it's only used for
+    // demonstrating scope copies. This is effectively making a copy of a
+    // pointer.
+
+    bar.setGlobal();
+    // Now foo has a local ref and bar has a global ref.
+
+    foo = bar;
+    // foo's local ref is destroyed. it's value is set to bar's and the widest
+    // possible scope is chosen. foo and bar now both have global refs.
+
+    return foo;
+  } // bar's global is unref'd and it is destroyed. global foo is returned.
+
+*/
+
+typedef void (jholder::*safe_bool_type)();
+void non_null_object() {}
 
 public:
-    explicit jholder(T obj = 0)
-        :object(obj)
-        ,global(false)
-    {
-    }
 
-    jholder(jholder const &c)
-        :object(c.object ? (T)xbmc_jnienv()->NewLocalRef(c.object) : 0)
-        ,global(false)
-    {
-    }
+/*! \brief constructor for jholder().
+   Object is null, type is invalid
+*/
+  jholder()
+  :m_refType(JNIInvalidRefType)
+  ,m_object(0)
+  {
+  }
 
+/*! \brief jholder copy constructor.
+   Object is copied, scope matches the original
+*/
+  jholder(jholder<T> const &c)
+  :m_refType(JNIInvalidRefType)
+  ,m_object(c.get())
+  {
+    setscope(c.m_refType);
+  }
 
-    template <typename U>
-    explicit jholder(jholder<U> const &c)
-        :object(c.object ? (T)xbmc_jnienv()->NewLocalRef(c.object) : 0)
-        ,global(false)
-    {
-    }
+/*! \brief jholder assignment operator
+   Object takes on the widest scope of the two, local at a minimum
+*/
+  jholder &operator=(jholder const &c)
+  {
+    jobjectRefType newtype = JNILocalRefType;
+    if(c.m_refType == JNIGlobalRefType || m_refType == JNIGlobalRefType)
+      newtype = JNIGlobalRefType;
 
-    ~jholder()
-    {
-      reset();
-    }
+    reset(c.get());
+    setscope(newtype);
+    return *this;
+  }
 
-    jholder &operator=(jholder const &c)
-    {
-        jholder tmp(c);
-        swap(tmp);
-        return *this;
-    }
+/*! \brief jholder constructor from a native jni object
+   Incoming objects already hold a local ref.
+*/
+  explicit jholder(const T& obj)
+  :m_refType(JNILocalRefType)
+  ,m_object(obj)
+  {
+    setscope(JNILocalRefType);
+  }
 
-    void reset(T obj = 0)
-    {
-        if (object)
-        {
-          if(global)
-            xbmc_jnienv()->DeleteGlobalRef(object);
-          else
-            xbmc_jnienv()->DeleteLocalRef(object);
-        }
-        object = obj;
-        global = false;
-    }
+/*! \brief jholder dtor.
+   Held objects are deref'd and destroyed.
+*/
+  ~jholder()
+  {
+    reset();
+  }
 
-    const T& get() const {return object;}
+/*! \brief cast operator for native types
+   Grr, no explicit operator casts without c++11.
+   This enables automatic down-casting to native types.
+*/
+  operator const T&() const
+  {
+    return get();
+  }
 
-    T release()
-    {
-        T ret = object;
-        object = 0;
-        global = false;
-        return ret;
-    }
+/*! \brief get native type
+   Same as the above, mainly for internal usage for readability and explicitness
+*/
+  const T& get() const
+  {
+    return m_object;
+  }
 
-    void setGlobal()
-    {
-      if(object)
-      {
-        T globalRef = (T)xbmc_jnienv()->NewGlobalRef(object);
-        reset(globalRef);
-        global = true;
-      }
-    }
+/*! \brief set an object to global scope. Has no effect if it's already global
+*/
+  void setGlobal()
+  {
+    setscope(JNIGlobalRefType);
+  }
 
-    bool operator!() const {return !object;}
-    operator safe_bool_type () const
-    {
-        return object ? &jholder::non_null_object : 0;
-    }
+/*! \brief not operator.
+     queries whether the jholder contains a valid object
+*/
+  bool operator!() const {return !m_object;}
+  operator safe_bool_type () const
+  {
+    return m_object ? &jholder::non_null_object : 0;
+  }
 
-    operator const T&() const
+/*! \brief Change the internal object
+  Unref the original object. The new ref type is invalid.
+
+  This should almost never be used outside of this class. Use it only to
+  maintain static objects that should never be unref'd, such as
+  nativeActivity->clazz.
+  Repeat: Usage of reset() is almost definitely wrong!
+*/
+  void reset(const T &obj = 0)
+  {
+    if(m_object)
     {
-      return object;
+      if(m_refType == JNIGlobalRefType)
+        xbmc_jnienv()->DeleteGlobalRef(m_object);
+      else if (m_refType == JNILocalRefType)
+        xbmc_jnienv()->DeleteLocalRef(m_object);
     }
+    m_refType = JNIInvalidRefType;
+    m_object = obj;
+  }
 
 private:
-    void swap(jholder &c)
+
+/*! \brief Set an object to local/global scope
+    New refs will be created as needed.
+    If the scope is being set to invalid, its ref will be destroyed as needed.
+*/
+  void setscope(const jobjectRefType type)
+  {
+    // Don't attempt anything on a bad object. Update its status to invalid.
+    if (!m_object)
     {
-        T tmp = object;
-        object = c.object;
-        c.object = tmp;
+      m_refType = JNIInvalidRefType;
+      return;
     }
 
-    T object;
-    bool global;
+    // Don't bother if the scope isn't actually changing
+    if (m_refType == type)
+      return;
+
+    if(type == JNIGlobalRefType)
+      reset((T)xbmc_jnienv()->NewGlobalRef(m_object));
+
+    else if (type == JNILocalRefType)
+      reset((T)xbmc_jnienv()->NewLocalRef(m_object));
+
+    else if (type == JNIInvalidRefType)
+      reset();
+    m_refType = type;
+  }
+
+  jobjectRefType m_refType;
+  T m_object;
 };
 
 typedef jholder<jclass> jhclass;