Fix a double release with Android bitmaps callbacks

The version of Texture::setBitmap() that tool an Android bitmap handled
callbacks differently and in certain case caused a "double release" of
the callback object. This didn't actually cause problems though.

We now use the same mechanism used elsewhere (i.e. JniCallback).

android/filament-android/src/main/cpp/Texture.cpp

@@ -31,6 +31,8 @@
 #include "common/CallbackUtils.h"
 #include "common/NioUtils.h"
 
+#include "private/backend/VirtualMachineEnv.h"
+
 using namespace filament;
 using namespace backend;
 
@@ -480,46 +482,37 @@ Java_com_google_android_filament_Texture_nGeneratePrefilterMipmap(JNIEnv *env, j
 #define BITMAP_CONFIG_RGBA_F16  4
 #define BITMAP_CONFIG_HARDWARE  5
 
-class AutoBitmap {
-public:
+class AutoBitmap : public JniCallback {
+private:
+
     AutoBitmap(JNIEnv* env, jobject bitmap) noexcept
-            : mEnv(env)
-            , mBitmap(env->NewGlobalRef(bitmap))
-    {
+            : JniCallback(),
+              mBitmap(env->NewGlobalRef(bitmap)) {
         if (mBitmap) {
-            AndroidBitmap_getInfo(mEnv, mBitmap, &mInfo);
-            AndroidBitmap_lockPixels(mEnv, mBitmap, &mData);
+            AndroidBitmap_getInfo(env, mBitmap, &mInfo);
+            AndroidBitmap_lockPixels(env, mBitmap, &mData);
         }
     }
 
     AutoBitmap(JNIEnv* env, jobject bitmap, jobject handler, jobject runnable) noexcept
-            : mEnv(env)
-            , mBitmap(env->NewGlobalRef(bitmap))
-            , mHandler(env->NewGlobalRef(handler))
-            , mCallback(env->NewGlobalRef(runnable))
-    {
-        acquireCallbackJni(env, mCallbackUtils);
+            : JniCallback(env, handler, runnable),
+              mBitmap(env->NewGlobalRef(bitmap)) {
         if (mBitmap) {
-            AndroidBitmap_getInfo(mEnv, mBitmap, &mInfo);
-            AndroidBitmap_lockPixels(mEnv, mBitmap, &mData);
+            AndroidBitmap_getInfo(env, mBitmap, &mInfo);
+            AndroidBitmap_lockPixels(env, mBitmap, &mData);
         }
     }
 
-    ~AutoBitmap() noexcept {
-        releaseCallbackJni(mEnv, mCallbackUtils, mHandler, mCallback);
+    void release(JNIEnv* env) {
         if (mBitmap) {
-            AndroidBitmap_unlockPixels(mEnv, mBitmap);
-            mEnv->DeleteGlobalRef(mBitmap);
+            AndroidBitmap_unlockPixels(env, mBitmap);
+            env->DeleteGlobalRef(mBitmap);
         }
     }
 
-    AutoBitmap(AutoBitmap &&rhs) noexcept {
-        mEnv = rhs.mEnv;
-        std::swap(mData, rhs.mData);
-        std::swap(mBitmap, rhs.mBitmap);
-        std::swap(mInfo, rhs.mInfo);
-    }
+    ~AutoBitmap() override = default;
 
+public:
     void* getData() const noexcept {
         return mData;
     }
@@ -546,28 +539,39 @@ public:
         }
     }
 
-    static void invoke(void* buffer, size_t n, void* user) {
-        AutoBitmap* data = reinterpret_cast<AutoBitmap*>(user);
-        delete data;
-    }
-
-    static AutoBitmap* make(Engine* engine, JNIEnv* env, jobject bitmap) {
-        return new AutoBitmap(env, bitmap);
-    }
-
-    static AutoBitmap* make(Engine* engine, JNIEnv* env, jobject bitmap,
-            jobject handler, jobject runnable) {
+    // create a AutoBitmap
+    static AutoBitmap* make(JNIEnv* env, jobject bitmap, jobject handler, jobject runnable) {
         return new AutoBitmap(env, bitmap, handler, runnable);
     }
 
+    // execute the callback on the java thread and destroy ourselves
+    static void invoke(void*, size_t, void* user) {
+        auto* autoBitmap = reinterpret_cast<AutoBitmap*>(user);
+        JNIEnv* env = filament::VirtualMachineEnv::get().getEnvironment();
+        releaseCallbackJni(env, autoBitmap->mCallbackUtils, autoBitmap->mHandler, autoBitmap->mCallback);
+        autoBitmap->release(env);
+        delete autoBitmap;
+    }
+
+    // create a AutoBitmap without a handler
+    static AutoBitmap* make(JNIEnv* env, jobject bitmap) {
+        return new AutoBitmap(env, bitmap);
+    }
+
+    // just destroy ourselves
+    static void invokeNoCallback(void*, size_t, void* user) {
+        auto* autoBitmap = reinterpret_cast<AutoBitmap*>(user);
+        JNIEnv* env = filament::VirtualMachineEnv::get().getEnvironment();
+        autoBitmap->release(env);
+        delete autoBitmap;
+    }
+
 private:
-    JNIEnv* mEnv;
     void* mData = nullptr;
     jobject mBitmap = nullptr;
     jobject mHandler = nullptr;
     jobject mCallback = nullptr;
     AndroidBitmapInfo mInfo{};
-    CallbackJni mCallbackUtils;
 };
 
 extern "C"
@@ -578,14 +582,14 @@ Java_com_google_android_filament_android_TextureHelper_nSetBitmap(JNIEnv* env, j
     Texture* texture = (Texture*) nativeTexture;
     Engine *engine = (Engine *) nativeEngine;
 
-    auto* autoBitmap = AutoBitmap::make(engine, env, bitmap);
+    auto* autoBitmap = AutoBitmap::make(env, bitmap);
 
     Texture::PixelBufferDescriptor desc(
             autoBitmap->getData(),
             autoBitmap->getSizeInBytes(),
             autoBitmap->getFormat(format),
             autoBitmap->getType(format),
-            &AutoBitmap::invoke, autoBitmap);
+            &AutoBitmap::invokeNoCallback, autoBitmap);
 
     texture->setImage(*engine, (size_t) level,
             (uint32_t) xoffset, (uint32_t) yoffset,
@@ -601,14 +605,14 @@ Java_com_google_android_filament_android_TextureHelper_nSetBitmapWithCallback(JN
     Texture* texture = (Texture*) nativeTexture;
     Engine *engine = (Engine *) nativeEngine;
 
-    auto* autoBitmap = AutoBitmap::make(engine, env, bitmap, handler, runnable);
+    auto* autoBitmap = AutoBitmap::make(env, bitmap, handler, runnable);
 
     Texture::PixelBufferDescriptor desc(
             autoBitmap->getData(),
             autoBitmap->getSizeInBytes(),
             autoBitmap->getFormat(format),
             autoBitmap->getType(format),
-            &AutoBitmap::invoke, autoBitmap);
+            autoBitmap->getHandler(), &AutoBitmap::invoke, autoBitmap);
 
     texture->setImage(*engine, (size_t) level,
             (uint32_t) xoffset, (uint32_t) yoffset,