From 29ddf84cc4065c73b243eec041db829220d971de Mon Sep 17 00:00:00 2001
From: Fredy Wijaya <fredyw@google.com>
Date: Mon, 6 Nov 2023 17:54:40 +0000
Subject: [PATCH] mobile: Part 2: Update JNI usages with JniHelper (#30748)

This updates the use of `FindClass` with `JniHelper`.

Signed-off-by: Fredy Wijaya <fredyw@google.com>
---
 .../library/common/jni/android_jni_utility.cc |  8 +--
 mobile/library/common/jni/jni_interface.cc    | 23 +++----
 mobile/library/common/jni/jni_utility.cc      | 63 ++++++++-----------
 mobile/library/common/jni/jni_utility.h       |  2 +-
 4 files changed, 42 insertions(+), 54 deletions(-)

diff --git a/mobile/library/common/jni/android_jni_utility.cc b/mobile/library/common/jni/android_jni_utility.cc
index dfa085d632a0..e6e07f80e107 100644
--- a/mobile/library/common/jni/android_jni_utility.cc
+++ b/mobile/library/common/jni/android_jni_utility.cc
@@ -18,14 +18,14 @@ bool is_cleartext_permitted(absl::string_view hostname) {
 #if defined(__ANDROID_API__)
   envoy_data host = Envoy::Data::Utility::copyToBridgeData(hostname);
   Envoy::JNI::JniHelper jni_helper(Envoy::JNI::get_env());
-  jstring java_host = Envoy::JNI::native_data_to_string(jni_helper, host);
+  Envoy::JNI::LocalRefUniquePtr<jstring> java_host =
+      Envoy::JNI::native_data_to_string(jni_helper, host);
   jclass jcls_AndroidNetworkLibrary =
       Envoy::JNI::find_class("io.envoyproxy.envoymobile.utilities.AndroidNetworkLibrary");
   jmethodID jmid_isCleartextTrafficPermitted = jni_helper.getEnv()->GetStaticMethodID(
       jcls_AndroidNetworkLibrary, "isCleartextTrafficPermitted", "(Ljava/lang/String;)Z");
-  jboolean result = jni_helper.callStaticBooleanMethod(jcls_AndroidNetworkLibrary,
-                                                       jmid_isCleartextTrafficPermitted, java_host);
-  jni_helper.getEnv()->DeleteLocalRef(java_host);
+  jboolean result = jni_helper.callStaticBooleanMethod(
+      jcls_AndroidNetworkLibrary, jmid_isCleartextTrafficPermitted, java_host.get());
   release_envoy_data(host);
   return result == JNI_TRUE;
 #else
diff --git a/mobile/library/common/jni/jni_interface.cc b/mobile/library/common/jni/jni_interface.cc
index 45ecdca11794..c05aad17356f 100644
--- a/mobile/library/common/jni/jni_interface.cc
+++ b/mobile/library/common/jni/jni_interface.cc
@@ -59,16 +59,15 @@ static void jvm_on_log(envoy_data data, const void* context) {
   }
 
   Envoy::JNI::JniHelper jni_helper(Envoy::JNI::get_env());
-  jstring str = Envoy::JNI::native_data_to_string(jni_helper, data);
+  Envoy::JNI::LocalRefUniquePtr<jstring> str = Envoy::JNI::native_data_to_string(jni_helper, data);
 
   jobject j_context = static_cast<jobject>(const_cast<void*>(context));
   jclass jcls_JvmLoggerContext = jni_helper.getEnv()->GetObjectClass(j_context);
   jmethodID jmid_onLog =
       jni_helper.getMethodId(jcls_JvmLoggerContext, "log", "(Ljava/lang/String;)V");
-  jni_helper.callVoidMethod(j_context, jmid_onLog, str);
+  jni_helper.callVoidMethod(j_context, jmid_onLog, str.get());
 
   release_envoy_data(data);
-  jni_helper.getEnv()->DeleteLocalRef(str);
   jni_helper.getEnv()->DeleteLocalRef(jcls_JvmLoggerContext);
 }
 
@@ -197,10 +196,10 @@ Java_io_envoyproxy_envoymobile_engine_JniLibrary_dumpStats(JNIEnv* env,
   }
 
   Envoy::JNI::JniHelper jni_helper(env);
-  jstring str = Envoy::JNI::native_data_to_string(jni_helper, data);
+  Envoy::JNI::LocalRefUniquePtr<jstring> str = Envoy::JNI::native_data_to_string(jni_helper, data);
   release_envoy_data(data);
 
-  return str;
+  return str.release();
 }
 
 // JvmCallbackContext
@@ -272,12 +271,13 @@ static void* jvm_on_headers(const char* method, const Envoy::Types::ManagedEnvoy
   // Create a "no operation" result:
   //  1. Tell the filter chain to continue the iteration.
   //  2. Return headers received on as method's input as part of the method's output.
-  jclass jcls_object_array = jni_helper.getEnv()->FindClass("java/lang/Object");
-  jobjectArray noopResult = jni_helper.getEnv()->NewObjectArray(2, jcls_object_array, NULL);
+  Envoy::JNI::LocalRefUniquePtr<jclass> jcls_object_array =
+      jni_helper.findClass("java/lang/Object");
+  jobjectArray noopResult = jni_helper.getEnv()->NewObjectArray(2, jcls_object_array.get(), NULL);
 
-  jclass jcls_int = jni_helper.getEnv()->FindClass("java/lang/Integer");
-  jmethodID jmid_intInit = jni_helper.getMethodId(jcls_int, "<init>", "(I)V");
-  jobject j_status = jni_helper.getEnv()->NewObject(jcls_int, jmid_intInit, 0);
+  Envoy::JNI::LocalRefUniquePtr<jclass> jcls_int = jni_helper.findClass("java/lang/Integer");
+  jmethodID jmid_intInit = jni_helper.getMethodId(jcls_int.get(), "<init>", "(I)V");
+  jobject j_status = jni_helper.getEnv()->NewObject(jcls_int.get(), jmid_intInit, 0);
   // Set status to "0" (FilterHeadersStatus::Continue). Signal that the intent
   // is to continue the iteration of the filter chain.
   jni_helper.getEnv()->SetObjectArrayElement(noopResult, 0, j_status);
@@ -286,9 +286,6 @@ static void* jvm_on_headers(const char* method, const Envoy::Types::ManagedEnvoy
   jni_helper.getEnv()->SetObjectArrayElement(
       noopResult, 1, Envoy::JNI::ToJavaArrayOfObjectArray(jni_helper, headers));
 
-  jni_helper.getEnv()->DeleteLocalRef(jcls_object_array);
-  jni_helper.getEnv()->DeleteLocalRef(jcls_int);
-
   return noopResult;
 }
 
diff --git a/mobile/library/common/jni/jni_utility.cc b/mobile/library/common/jni/jni_utility.cc
index d455aa2449cc..f371d993e22b 100644
--- a/mobile/library/common/jni/jni_utility.cc
+++ b/mobile/library/common/jni/jni_utility.cc
@@ -23,18 +23,15 @@ jobject get_class_loader() {
 }
 
 jclass find_class(const char* class_name) {
-  JNIEnv* env = get_env();
-  jclass class_loader = env->FindClass("java/lang/ClassLoader");
-  Envoy::JNI::Exception::checkAndClear("find_class:FindClass");
-  jmethodID find_class_method =
-      env->GetMethodID(class_loader, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;");
+  JniHelper jni_helper(get_env());
+  LocalRefUniquePtr<jclass> class_loader = jni_helper.findClass("java/lang/ClassLoader");
+  jmethodID find_class_method = jni_helper.getMethodId(class_loader.get(), "loadClass",
+                                                       "(Ljava/lang/String;)Ljava/lang/Class;");
   Envoy::JNI::Exception::checkAndClear("find_class:GetMethodID");
-  jstring str_class_name = env->NewStringUTF(class_name);
-  Envoy::JNI::Exception::checkAndClear("find_class:NewStringUTF");
-  jclass clazz =
-      (jclass)(env->CallObjectMethod(get_class_loader(), find_class_method, str_class_name));
+  LocalRefUniquePtr<jstring> str_class_name = jni_helper.newStringUtf(class_name);
+  jclass clazz = (jclass)(jni_helper.getEnv()->CallObjectMethod(
+      get_class_loader(), find_class_method, str_class_name.get()));
   Envoy::JNI::Exception::checkAndClear("find_class:CallObjectMethod");
-  env->DeleteLocalRef(str_class_name);
   return clazz;
 }
 
@@ -51,9 +48,8 @@ void jni_delete_const_global_ref(const void* context) {
 }
 
 int unbox_integer(JniHelper& jni_helper, jobject boxedInteger) {
-  jclass jcls_Integer = jni_helper.getEnv()->FindClass("java/lang/Integer");
-  jmethodID jmid_intValue = jni_helper.getMethodId(jcls_Integer, "intValue", "()I");
-  jni_helper.getEnv()->DeleteLocalRef(jcls_Integer);
+  LocalRefUniquePtr<jclass> jcls_Integer = jni_helper.findClass("java/lang/Integer");
+  jmethodID jmid_intValue = jni_helper.getMethodId(jcls_Integer.get(), "intValue", "()I");
   return jni_helper.callIntMethod(boxedInteger, jmid_intValue);
 }
 
@@ -70,11 +66,10 @@ envoy_data array_to_native_data(JniHelper& jni_helper, jbyteArray j_data, size_t
   return {data_length, native_bytes, free, native_bytes};
 }
 
-jstring native_data_to_string(JniHelper& jni_helper, envoy_data data) {
+LocalRefUniquePtr<jstring> native_data_to_string(JniHelper& jni_helper, envoy_data data) {
   // Ensure we get a null-terminated string, the data coming in via envoy_data might not be.
   std::string str(reinterpret_cast<const char*>(data.bytes), data.length);
-  jstring jstrBuf = jni_helper.getEnv()->NewStringUTF(str.c_str());
-  return jstrBuf;
+  return jni_helper.newStringUtf(str.c_str());
 }
 
 jbyteArray native_data_to_array(JniHelper& jni_helper, envoy_data data) {
@@ -135,19 +130,17 @@ jlongArray native_final_stream_intel_to_array(JniHelper& jni_helper,
 }
 
 jobject native_map_to_map(JniHelper& jni_helper, envoy_map map) {
-  jclass jcls_hashMap = jni_helper.getEnv()->FindClass("java/util/HashMap");
-  jmethodID jmid_hashMapInit = jni_helper.getMethodId(jcls_hashMap, "<init>", "(I)V");
-  jobject j_hashMap = jni_helper.getEnv()->NewObject(jcls_hashMap, jmid_hashMapInit, map.length);
+  LocalRefUniquePtr<jclass> jcls_hashMap = jni_helper.findClass("java/util/HashMap");
+  jmethodID jmid_hashMapInit = jni_helper.getMethodId(jcls_hashMap.get(), "<init>", "(I)V");
+  jobject j_hashMap =
+      jni_helper.getEnv()->NewObject(jcls_hashMap.get(), jmid_hashMapInit, map.length);
   jmethodID jmid_hashMapPut = jni_helper.getMethodId(
-      jcls_hashMap, "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
+      jcls_hashMap.get(), "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
   for (envoy_map_size_t i = 0; i < map.length; i++) {
-    auto key = native_data_to_string(jni_helper, map.entries[i].key);
-    auto value = native_data_to_string(jni_helper, map.entries[i].value);
-    callObjectMethod(jni_helper, j_hashMap, jmid_hashMapPut, key, value);
-    jni_helper.getEnv()->DeleteLocalRef(key);
-    jni_helper.getEnv()->DeleteLocalRef(value);
+    LocalRefUniquePtr<jstring> key = native_data_to_string(jni_helper, map.entries[i].key);
+    LocalRefUniquePtr<jstring> value = native_data_to_string(jni_helper, map.entries[i].value);
+    callObjectMethod(jni_helper, j_hashMap, jmid_hashMapPut, key.get(), value.get());
   }
-  jni_helper.getEnv()->DeleteLocalRef(jcls_hashMap);
   return j_hashMap;
 }
 
@@ -156,13 +149,12 @@ envoy_data buffer_to_native_data(JniHelper& jni_helper, jobject j_data) {
   jlong data_length = jni_helper.getEnv()->GetDirectBufferCapacity(j_data);
 
   if (data_length < 0) {
-    jclass jcls_ByteBuffer = jni_helper.getEnv()->FindClass("java/nio/ByteBuffer");
+    LocalRefUniquePtr<jclass> jcls_ByteBuffer = jni_helper.findClass("java/nio/ByteBuffer");
     // We skip checking hasArray() because only direct ByteBuffers or array-backed ByteBuffers
     // are supported. We will crash here if this is an invalid buffer, but guards may be
     // implemented in the JVM layer.
-    jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer, "array", "()[B");
+    jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer.get(), "array", "()[B");
     jbyteArray array = static_cast<jbyteArray>(callObjectMethod(jni_helper, j_data, jmid_array));
-    jni_helper.getEnv()->DeleteLocalRef(jcls_ByteBuffer);
 
     envoy_data native_data = array_to_native_data(jni_helper, array);
     jni_helper.getEnv()->DeleteLocalRef(array);
@@ -178,13 +170,12 @@ envoy_data buffer_to_native_data(JniHelper& jni_helper, jobject j_data, size_t d
       static_cast<uint8_t*>(jni_helper.getEnv()->GetDirectBufferAddress(j_data));
 
   if (direct_address == nullptr) {
-    jclass jcls_ByteBuffer = jni_helper.getEnv()->FindClass("java/nio/ByteBuffer");
+    LocalRefUniquePtr<jclass> jcls_ByteBuffer = jni_helper.findClass("java/nio/ByteBuffer");
     // We skip checking hasArray() because only direct ByteBuffers or array-backed ByteBuffers
     // are supported. We will crash here if this is an invalid buffer, but guards may be
     // implemented in the JVM layer.
-    jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer, "array", "()[B");
+    jmethodID jmid_array = jni_helper.getMethodId(jcls_ByteBuffer.get(), "array", "()[B");
     jbyteArray array = static_cast<jbyteArray>(callObjectMethod(jni_helper, j_data, jmid_array));
-    jni_helper.getEnv()->DeleteLocalRef(jcls_ByteBuffer);
 
     envoy_data native_data = array_to_native_data(jni_helper, array, data_length);
     jni_helper.getEnv()->DeleteLocalRef(array);
@@ -271,9 +262,9 @@ envoy_map to_native_map(JniHelper& jni_helper, jobjectArray entries) {
 
 jobjectArray ToJavaArrayOfObjectArray(JniHelper& jni_helper,
                                       const Envoy::Types::ManagedEnvoyHeaders& map) {
-  jclass jcls_byte_array = jni_helper.getEnv()->FindClass("java/lang/Object");
+  LocalRefUniquePtr<jclass> jcls_byte_array = jni_helper.findClass("java/lang/Object");
   jobjectArray javaArray =
-      jni_helper.getEnv()->NewObjectArray(2 * map.get().length, jcls_byte_array, nullptr);
+      jni_helper.getEnv()->NewObjectArray(2 * map.get().length, jcls_byte_array.get(), nullptr);
 
   for (envoy_map_size_t i = 0; i < map.get().length; i++) {
     jbyteArray key = native_data_to_array(jni_helper, map.get().entries[i].key);
@@ -287,8 +278,8 @@ jobjectArray ToJavaArrayOfObjectArray(JniHelper& jni_helper,
 }
 
 jobjectArray ToJavaArrayOfByteArray(JniHelper& jni_helper, const std::vector<std::string>& v) {
-  jclass jcls_byte_array = jni_helper.getEnv()->FindClass("[B");
-  jobjectArray joa = jni_helper.getEnv()->NewObjectArray(v.size(), jcls_byte_array, nullptr);
+  LocalRefUniquePtr<jclass> jcls_byte_array = jni_helper.findClass("[B");
+  jobjectArray joa = jni_helper.getEnv()->NewObjectArray(v.size(), jcls_byte_array.get(), nullptr);
 
   for (size_t i = 0; i < v.size(); ++i) {
     jbyteArray byte_array =
diff --git a/mobile/library/common/jni/jni_utility.h b/mobile/library/common/jni/jni_utility.h
index 60dc74b244da..b60f455e32f8 100644
--- a/mobile/library/common/jni/jni_utility.h
+++ b/mobile/library/common/jni/jni_utility.h
@@ -76,7 +76,7 @@ jlongArray native_final_stream_intel_to_array(JniHelper& jni_helper,
  */
 jobject native_map_to_map(JniHelper& jni_helper, envoy_map map);
 
-jstring native_data_to_string(JniHelper& jni_helper, envoy_data data);
+LocalRefUniquePtr<jstring> native_data_to_string(JniHelper& jni_helper, envoy_data data);
 
 envoy_data buffer_to_native_data(JniHelper& jni_helper, jobject j_data);