still_image_camera: Fix incorrect JNI usage

As `jstring`s are also object references, if we would like to use them across native methods/threads, we would have to make a global reference.

We will need to delete this global reference explicitly. Since this string is shared across multiple Interfaces and also in Factory, I used shared_ptr to manage deletion. Added a fancy SharedGlobalRef to id_cache.h.

Also removed global reference creation for java/lang/String classes. Turns out that local references are guaranteed valid for the duration of the method, and I was just being too cautious.

src/android/app/src/main/jni/applets/mii_selector.cpp

@@ -34,8 +34,7 @@ void AndroidMiiSelector::Setup(const Frontend::MiiSelectorConfig& config) {
 
     // List mii names
     // The 'Standard Mii' is not included here as we need Java side to translate it
-    const jclass string_class =
-        reinterpret_cast<jclass>(env->NewGlobalRef(env->FindClass("java/lang/String")));
+    const jclass string_class = reinterpret_cast<jclass>(env->FindClass("java/lang/String"));
     const jobjectArray array =
         env->NewObjectArray(static_cast<jsize>(miis.size()), string_class, nullptr);
     for (std::size_t i = 0; i < miis.size(); ++i) {
@@ -45,7 +44,6 @@ void AndroidMiiSelector::Setup(const Frontend::MiiSelectorConfig& config) {
     env->SetObjectField(
         java_config,
         env->GetFieldID(s_mii_selector_config_class, "mii_names", "[Ljava/lang/String;"), array);
-    env->DeleteGlobalRef(string_class);
 
     // Invoke backend Execute method
     jobject data =

src/android/app/src/main/jni/applets/swkbd.cpp

@@ -41,8 +41,7 @@ static jobject ToJavaKeyboardConfig(const Frontend::KeyboardConfig& config) {
                         env->GetFieldID(s_keyboard_config_class, "hint_text", "Ljava/lang/String;"),
                         env->NewStringUTF(config.hint_text.c_str()));
     if (config.has_custom_button_text) {
-        const jclass string_class =
-            reinterpret_cast<jclass>(env->NewGlobalRef(env->FindClass("java/lang/String")));
+        const jclass string_class = reinterpret_cast<jclass>(env->FindClass("java/lang/String"));
         const jobjectArray array =
             env->NewObjectArray(static_cast<jsize>(config.button_text.size()), string_class,
                                 env->NewStringUTF(config.button_text[0].c_str()));
@@ -53,7 +52,6 @@ static jobject ToJavaKeyboardConfig(const Frontend::KeyboardConfig& config) {
         env->SetObjectField(
             object, env->GetFieldID(s_keyboard_config_class, "button_text", "[Ljava/lang/String;"),
             array);
-        env->DeleteGlobalRef(string_class);
     }
     return object;
 }

src/android/app/src/main/jni/camera/still_image_camera.cpp

@@ -7,7 +7,6 @@
 #include "common/logging/log.h"
 #include "core/frontend/camera/blank_camera.h"
 #include "jni/camera/still_image_camera.h"
-#include "jni/id_cache.h"
 
 static jclass s_still_image_camera_helper_class;
 static jmethodID s_open_file_picker;
@@ -29,7 +28,8 @@ void CleanupJNI(JNIEnv* env) {
     env->DeleteGlobalRef(s_still_image_camera_helper_class);
 }
 
-Interface::Interface(jstring path_, const Service::CAM::Flip& flip) : path(path_) {
+Interface::Interface(SharedGlobalRef<jstring> path_, const Service::CAM::Flip& flip)
+    : path(std::move(path_)) {
     mirror = base_mirror =
         flip == Service::CAM::Flip::Horizontal || flip == Service::CAM::Flip::Reverse;
     invert = base_invert =
@@ -37,14 +37,14 @@ Interface::Interface(jstring path_, const Service::CAM::Flip& flip) : path(path_
 }
 
 Interface::~Interface() {
-    Factory::last_path = nullptr;
+    Factory::last_path.reset();
 }
 
 void Interface::StartCapture() {
     JNIEnv* env = IDCache::GetEnvForThread();
     jobject bitmap =
-        env->CallStaticObjectMethod(s_still_image_camera_helper_class, s_load_image_from_file, path,
-                                    resolution.width, resolution.height);
+        env->CallStaticObjectMethod(s_still_image_camera_helper_class, s_load_image_from_file,
+                                    path.get(), resolution.width, resolution.height);
     if (bitmap == nullptr) {
         LOG_ERROR(Frontend, "Could not load image from file");
         opened = false;
@@ -118,7 +118,7 @@ bool Interface::IsPreviewAvailable() {
     return opened;
 }
 
-jstring Factory::last_path{};
+SharedGlobalRef<jstring> Factory::last_path{};
 
 std::unique_ptr<CameraInterface> Factory::Create(const std::string& config,
                                                  const Service::CAM::Flip& flip) {
@@ -134,8 +134,9 @@ std::unique_ptr<CameraInterface> Factory::Create(const std::string& config,
     if (path == nullptr) {
         return std::make_unique<Camera::BlankCamera>();
     } else {
-        last_path = path;
-        return std::make_unique<Interface>(path, flip);
+        auto shared_path = NewSharedGlobalRef(path);
+        last_path = shared_path;
+        return std::make_unique<Interface>(std::move(shared_path), flip);
     }
 }
 

src/android/app/src/main/jni/camera/still_image_camera.h

@@ -11,12 +11,13 @@
 #include "common/common_types.h"
 #include "core/frontend/camera/factory.h"
 #include "core/frontend/camera/interface.h"
+#include "jni/id_cache.h"
 
 namespace Camera::StillImage {
 
 class Interface final : public CameraInterface {
 public:
-    Interface(jstring path, const Service::CAM::Flip& flip);
+    Interface(SharedGlobalRef<jstring> path, const Service::CAM::Flip& flip);
     ~Interface();
     void StartCapture() override;
     void StopCapture() override{};
@@ -29,7 +30,7 @@ public:
     bool IsPreviewAvailable() override;
 
 private:
-    jstring path;
+    SharedGlobalRef<jstring> path;
     Service::CAM::Resolution resolution;
 
     // Flipping parameters. mirror = horizontal, invert = vertical.
@@ -50,7 +51,7 @@ public:
 
 private:
     /// Record the path chosen to avoid multiple prompt problem
-    static jstring last_path;
+    static SharedGlobalRef<jstring> last_path;
 
     friend class Interface;
 };

src/android/app/src/main/jni/id_cache.h

@@ -4,6 +4,8 @@
 
 #pragma once
 
+#include <memory>
+#include <type_traits>
 #include <jni.h>
 
 namespace IDCache {
@@ -19,3 +21,20 @@ jmethodID GetExitEmulationActivity();
 jmethodID GetRequestCameraPermission();
 
 } // namespace IDCache
+
+template <typename T = jobject>
+using SharedGlobalRef = std::shared_ptr<std::remove_pointer_t<T>>;
+
+struct SharedGlobalRefDeleter {
+    void operator()(jobject ptr) {
+        JNIEnv* env = IDCache::GetEnvForThread();
+        env->DeleteGlobalRef(ptr);
+    }
+};
+
+template <typename T = jobject>
+SharedGlobalRef<T> NewSharedGlobalRef(T object) {
+    JNIEnv* env = IDCache::GetEnvForThread();
+    auto* global_ref = reinterpret_cast<T>(env->NewGlobalRef(object));
+    return SharedGlobalRef<T>(global_ref, SharedGlobalRefDeleter());
+}