From 3de7d878d7227b04391fd15894fb8e2243772e7f Mon Sep 17 00:00:00 2001 From: Philip Gregor Date: Fri, 26 Jan 2024 11:31:25 -0800 Subject: [PATCH] Addressed comments by yunhanw-google - 2nd review --- .../main/jni/cpp/core/CastingPlayer-JNI.cpp | 69 +++++++------------ src/lib/support/JniTypeWrappers.h | 12 ++++ 2 files changed, 36 insertions(+), 45 deletions(-) diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp index 59bcdf7c6727a6..2c0fe914232778 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp +++ b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/core/CastingPlayer-JNI.cpp @@ -71,73 +71,52 @@ JNI_METHOD(jobject, VerifyOrEstablishConnection) ConnectCallback callback = [completableFutureObjGlobalRef](CHIP_ERROR err, CastingPlayer * playerPtr) { ChipLogProgress(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback called"); + VerifyOrReturn(completableFutureObjGlobalRef != nullptr, + ChipLogError(AppServer, "ConnectCallback, completableFutureObjGlobalRef == nullptr")); + JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); - VerifyOrReturn(env != nullptr, - ChipLogError(AppServer, "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, env == nullptr")); + VerifyOrReturn(env != nullptr, ChipLogError(AppServer, "ConnectCallback, env == nullptr")); + // Ensures proper cleanup of local references to Java objects. JniLocalReferenceManager manager(env); - jclass completableFutureClass = env->FindClass("java/util/concurrent/CompletableFuture"); - VerifyOrReturn( - completableFutureClass != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, completableFutureClass == nullptr")); + // Ensures proper cleanup of global references to Java objects. + JniGlobalRefWrapper globalRefWrapper(completableFutureObjGlobalRef); - if (completableFutureObjGlobalRef == nullptr) - { - ChipLogError( - AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, completableFutureObjGlobalRef == nullptr"); - } + jclass completableFutureClass = env->FindClass("java/util/concurrent/CompletableFuture"); + VerifyOrReturn(completableFutureClass != nullptr, + ChipLogError(AppServer, "ConnectCallback, completableFutureClass == nullptr"); + env->DeleteGlobalRef(completableFutureObjGlobalRef);); if (err == CHIP_NO_ERROR) { - ChipLogProgress( - AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, Casting Player connection successful!"); + ChipLogProgress(AppServer, "ConnectCallback, Connected to Casting Player with device ID: %s", playerPtr->GetId()); jmethodID completeMethod = env->GetMethodID(completableFutureClass, "complete", "(Ljava/lang/Object;)Z"); - VerifyOrReturn( - completeMethod != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, completeMethod == nullptr")); + VerifyOrReturn(completeMethod != nullptr, ChipLogError(AppServer, "ConnectCallback, completeMethod == nullptr")); + chip::DeviceLayer::StackUnlock unlock; env->CallBooleanMethod(completableFutureObjGlobalRef, completeMethod, nullptr); } else { - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, connection error: %" CHIP_ERROR_FORMAT, - err.Format()); + ChipLogError(AppServer, "ConnectCallback, connection error: %" CHIP_ERROR_FORMAT, err.Format()); jmethodID completeExceptionallyMethod = env->GetMethodID(completableFutureClass, "completeExceptionally", "(Ljava/lang/Throwable;)Z"); - VerifyOrReturn( - completeExceptionallyMethod != nullptr, - ChipLogError( - AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, completeExceptionallyMethod == nullptr")); + VerifyOrReturn(completeExceptionallyMethod != nullptr, + ChipLogError(AppServer, "ConnectCallback, completeExceptionallyMethod == nullptr")); + // Create a Throwable object (e.g., RuntimeException) to pass to completeExceptionallyMethod jclass throwableClass = env->FindClass("java/lang/RuntimeException"); - VerifyOrReturn( - throwableClass != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, throwableClass == nullptr")); + VerifyOrReturn(throwableClass != nullptr, ChipLogError(AppServer, "ConnectCallback, throwableClass == nullptr")); jmethodID throwableConstructor = env->GetMethodID(throwableClass, "", "(Ljava/lang/String;)V"); - VerifyOrReturn( - throwableConstructor != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, throwableConstructor == nullptr")); + VerifyOrReturn(throwableConstructor != nullptr, + ChipLogError(AppServer, "ConnectCallback, throwableConstructor == nullptr")); jstring errorMessage = env->NewStringUTF(err.Format()); - VerifyOrReturn( - errorMessage != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, errorMessage == nullptr")); + VerifyOrReturn(errorMessage != nullptr, ChipLogError(AppServer, "ConnectCallback, errorMessage == nullptr")); jobject throwableObject = env->NewObject(throwableClass, throwableConstructor, errorMessage); - VerifyOrReturn( - throwableObject != nullptr, - ChipLogError(AppServer, - "CastingPlayer-JNI::VerifyOrEstablishConnection() ConnectCallback, throwableObject == nullptr")); + VerifyOrReturn(throwableObject != nullptr, ChipLogError(AppServer, "ConnectCallback, throwableObject == nullptr")); + chip::DeviceLayer::StackUnlock unlock; env->CallBooleanMethod(completableFutureObjGlobalRef, completeExceptionallyMethod, throwableObject); } - env->DeleteGlobalRef(completableFutureObjGlobalRef); }; if (desiredEndpointFilterJavaObject == nullptr) diff --git a/src/lib/support/JniTypeWrappers.h b/src/lib/support/JniTypeWrappers.h index 642e1b4358f79b..a6b018b665251a 100644 --- a/src/lib/support/JniTypeWrappers.h +++ b/src/lib/support/JniTypeWrappers.h @@ -180,6 +180,18 @@ class JniClass jclass mClassRef; }; +// Manages an pre-existing global reference to a jobject. +class JniGlobalRefWrapper +{ +public: + explicit JniGlobalRefWrapper(jobject mGlobalRef) : mGlobalRef(mGlobalRef) {} + ~JniGlobalRefWrapper() { chip::JniReferences::GetInstance().GetEnvForCurrentThread()->DeleteGlobalRef(mGlobalRef); } + jobject classRef() { return mGlobalRef; } + +private: + jobject mGlobalRef; +}; + class JniLocalReferenceManager { public: