From efaba04bd824ef2e98101411f82c14b1ffd42d1e Mon Sep 17 00:00:00 2001
From: Jamieson Pryor <jwhpryor@google.com>
Date: Fri, 13 Sep 2024 09:15:48 -0700
Subject: [PATCH] Add implementation/test for `GlobalString` destruction.

This was actually causing premature releases of GlobalString because it wasn't promoting its local representation.

Also, add tests for graceful destruction of GlobalObject which isn't returned.

PiperOrigin-RevId: 674322344
---
 implementation/global_string.h                       | 6 ++++++
 javatests/com/jnibind/test/GlobalObjectTest.java     | 9 +++++++--
 javatests/com/jnibind/test/StringTest.java           | 8 +++++++-
 javatests/com/jnibind/test/global_object_test_jni.cc | 6 ++++++
 javatests/com/jnibind/test/string_test_jni.cc        | 5 +++++
 5 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/implementation/global_string.h b/implementation/global_string.h
index 2a310f6b..eeae5c64 100644
--- a/implementation/global_string.h
+++ b/implementation/global_string.h
@@ -48,6 +48,12 @@ class GlobalString : public GlobalStringImpl {
   GlobalString(LocalString &&local_string)
       : Base(LifecycleT::Promote(local_string.Release())) {}
 
+  template <typename... Ts>
+  GlobalString(Ts &&...vals) : Base(std::forward<Ts &&>(vals)...) {
+    RefBase<jstring>::object_ref_ =
+        LifecycleT::Promote(RefBase<jstring>::object_ref_);
+  }
+
   // Returns a StringView which possibly performs an expensive pinning
   // operation.  String objects can be pinned multiple times.
   UtfStringView Pin() { return {RefBase<jstring>::object_ref_}; }
diff --git a/javatests/com/jnibind/test/GlobalObjectTest.java b/javatests/com/jnibind/test/GlobalObjectTest.java
index 6412a573..9660b0c3 100644
--- a/javatests/com/jnibind/test/GlobalObjectTest.java
+++ b/javatests/com/jnibind/test/GlobalObjectTest.java
@@ -49,6 +49,13 @@ public static void doShutDown() {
     jniTearDown();
   }
 
+  static native void jniObjectGracefullyDies();
+
+  @Test
+  public void objectGracefullyDies() {
+    jniObjectGracefullyDies();
+  }
+
   @Test
   public void createNewGlobalObjectUsingNoArgConstructor() {
     assertThat(jniCreateNewObject()).isNotEqualTo(null);
@@ -87,7 +94,6 @@ public void createFromCopyAssignmentIntoGlobal() {
   native ObjectTestHelper jniBuildNewObjectsFromExistingObjects(
       GlobalObjectTest testHelperObject, ObjectTestHelper objectToMutate);
 
-
   ObjectTestHelper methodTakesGlobalObjectReturnsNewObject(ObjectTestHelper object) {
     return new ObjectTestHelper(object.intVal1 + 5, object.intVal2 + 6, object.intVal3 + 7);
   }
@@ -136,5 +142,4 @@ public void materializeNewGlobalObjectSetIntVal159() {
     assertThat(object.intVal2).isEqualTo(11);
     assertThat(object.intVal3).isEqualTo(16);
   }
-  
 }
diff --git a/javatests/com/jnibind/test/StringTest.java b/javatests/com/jnibind/test/StringTest.java
index 98de5550..d3de51c9 100644
--- a/javatests/com/jnibind/test/StringTest.java
+++ b/javatests/com/jnibind/test/StringTest.java
@@ -66,7 +66,6 @@ public void multiFormPassTest() {
   native void jniVoidMethodTakesFiveStrings(
       StringTestHelper helper, String s1, String s2, String s3, String s4, String s5);
 
-
   // Calls each of the above native methods which will then call the similarly named method on the
   // StringTestHelper.  E.g. jniVoidMethodTakesString calls voidMethodTakesString.
   @Test
@@ -140,4 +139,11 @@ public void nullStringTests() {
     verify(rJniStringTestHelper).stringMethodTakesString(null);
   }
   */
+
+  native void jniReturnsAGlobalString();
+
+  @Test
+  public void globalReturnsCorrectlyOverJniBoundary() {
+    jniReturnsAGlobalString();
+  }
 }
diff --git a/javatests/com/jnibind/test/global_object_test_jni.cc b/javatests/com/jnibind/test/global_object_test_jni.cc
index 0ea0862c..ee27b44d 100644
--- a/javatests/com/jnibind/test/global_object_test_jni.cc
+++ b/javatests/com/jnibind/test/global_object_test_jni.cc
@@ -46,6 +46,12 @@ Java_com_jnibind_test_GlobalObjectTest_jniCreateNewObject(JNIEnv* env, jclass) {
   return jni::GlobalObject<kObjectTestHelperClass>{1, 2, 3}.Release();
 }
 
+JNIEXPORT void JNICALL
+Java_com_jnibind_test_GlobalObjectTest_jniObjectGracefullyDies(JNIEnv* env,
+                                                               jclass) {
+  jni::GlobalObject<kObjectTestHelperClass>{1, 2, 3};
+}
+
 JNIEXPORT jobject JNICALL
 Java_com_jnibind_test_GlobalObjectTest_jniCreateNewGlobalObjectSetIntVal123(
     JNIEnv* env, jclass) {
diff --git a/javatests/com/jnibind/test/string_test_jni.cc b/javatests/com/jnibind/test/string_test_jni.cc
index bba00225..d4aa07af 100644
--- a/javatests/com/jnibind/test/string_test_jni.cc
+++ b/javatests/com/jnibind/test/string_test_jni.cc
@@ -145,4 +145,9 @@ Java_com_jnibind_test_StringTest_jniStringMethodTakesFiveStrings(
       .Release();
 }
 
+JNIEXPORT void JNICALL
+Java_com_jnibind_test_StringTest_jniReturnsAGlobalString(JNIEnv* env, jclass) {
+  GlobalString g{"fake"};
+}
+
 }  // extern "C"