From eb71bf777a7386d30039915bd26ac63bf23990f7 Mon Sep 17 00:00:00 2001 From: Sharad Binjola Date: Mon, 6 Feb 2023 18:14:37 -0800 Subject: [PATCH] tv-casting-app: Handled uncaught exceptions and null pointer issues --- .../chip/casting/app/CertTestFragment.java | 55 +++++++++++++------ .../casting/app/MediaPlaybackFragment.java | 48 +++++++++------- .../jni/com/chip/casting/FailureCallback.java | 12 +++- .../chip/casting/MatterCallbackHandler.java | 12 +++- .../SubscriptionEstablishedCallback.java | 18 +++++- .../jni/com/chip/casting/SuccessCallback.java | 16 +++++- .../app/src/main/jni/cpp/ConversionUtils.cpp | 3 + .../jni/cpp/MatterCallbackHandler-JNI.cpp | 4 +- 8 files changed, 122 insertions(+), 46 deletions(-) diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CertTestFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CertTestFragment.java index 7540c3a3f8b6c1..16070eb1dfe59e 100644 --- a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CertTestFragment.java +++ b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CertTestFragment.java @@ -81,8 +81,12 @@ public void onClick(View v) { private void runCertTests(Activity activity) { CertTestMatterSuccessFailureCallback successFailureCallback = new CertTestMatterSuccessFailureCallback(activity); - CertTestMatterSuccessFailureCallbackInteger successFailureCallbackInteger = - new CertTestMatterSuccessFailureCallbackInteger(successFailureCallback); + CertTestMatterSuccessCallback successCallback = + new CertTestMatterSuccessCallback(successFailureCallback); + CertTestMatterFailureCallback failureCallback = + new CertTestMatterFailureCallback(successFailureCallback); + CertTestMatterSuccessCallbackInteger successCallbackInteger = + new CertTestMatterSuccessCallbackInteger(successFailureCallback); CertTestMatterCallbackHandler callback = new CertTestMatterCallbackHandler(successFailureCallback); @@ -269,7 +273,7 @@ private void runCertTests(Activity activity) { successFailureCallback, () -> { tvCastingApp.applicationBasic_readApplicationVersion( - kContentApp, successFailureCallback, successFailureCallback); + kContentApp, successCallback, failureCallback); }); runAndWait( @@ -277,7 +281,7 @@ private void runCertTests(Activity activity) { successFailureCallback, () -> { tvCastingApp.applicationBasic_readVendorName( - kContentApp, successFailureCallback, successFailureCallback); + kContentApp, successCallback, failureCallback); }); runAndWait( @@ -285,7 +289,7 @@ private void runCertTests(Activity activity) { successFailureCallback, () -> { tvCastingApp.applicationBasic_readApplicationName( - kContentApp, successFailureCallback, successFailureCallback); + kContentApp, successCallback, failureCallback); }); runAndWait( @@ -293,7 +297,7 @@ private void runCertTests(Activity activity) { successFailureCallback, () -> { tvCastingApp.applicationBasic_readVendorID( - kContentApp, successFailureCallbackInteger, successFailureCallbackInteger); + kContentApp, successCallbackInteger, failureCallback); }); runAndWait( @@ -301,7 +305,7 @@ private void runCertTests(Activity activity) { successFailureCallback, () -> { tvCastingApp.applicationBasic_readProductID( - kContentApp, successFailureCallbackInteger, successFailureCallbackInteger); + kContentApp, successCallbackInteger, failureCallback); }); runAndWait( @@ -320,7 +324,7 @@ public void handle(MediaPlaybackTypes.PlaybackStateEnum response) { activity, MatterError.NO_ERROR, "mediaPlayback_subscribeToCurrentState"); } }, - successFailureCallback, + failureCallback, 0, 20, new SubscriptionEstablishedCallback() { @@ -415,8 +419,7 @@ public void handle(MatterError error) { } } - class CertTestMatterSuccessFailureCallback extends FailureCallback - implements SuccessCallback { + class CertTestMatterSuccessFailureCallback { private Activity activity; private String testMethod; private CountDownLatch cdl; @@ -433,7 +436,6 @@ public void setCountDownLatch(CountDownLatch cdl) { this.cdl = cdl; } - @Override public void handle(MatterError error) { try { cdl.countDown(); @@ -446,7 +448,6 @@ public void handle(MatterError error) { } } - @Override public void handle(String response) { try { cdl.countDown(); @@ -460,18 +461,38 @@ public void handle(String response) { } } - class CertTestMatterSuccessFailureCallbackInteger extends FailureCallback - implements SuccessCallback { + class CertTestMatterSuccessCallback extends SuccessCallback { + private CertTestMatterSuccessFailureCallback delegate; + + CertTestMatterSuccessCallback(CertTestMatterSuccessFailureCallback delegate) { + this.delegate = delegate; + } + + @Override + public void handle(String response) { + delegate.handle(response); + } + } + class CertTestMatterFailureCallback extends FailureCallback { private CertTestMatterSuccessFailureCallback delegate; - CertTestMatterSuccessFailureCallbackInteger(CertTestMatterSuccessFailureCallback delegate) { + CertTestMatterFailureCallback(CertTestMatterSuccessFailureCallback delegate) { this.delegate = delegate; } @Override - public void handle(MatterError error) { - delegate.handle(error); + public void handle(MatterError err) { + delegate.handle(err); + } + } + + class CertTestMatterSuccessCallbackInteger extends SuccessCallback { + + private CertTestMatterSuccessFailureCallback delegate; + + CertTestMatterSuccessCallbackInteger(CertTestMatterSuccessFailureCallback delegate) { + this.delegate = delegate; } @Override diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MediaPlaybackFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MediaPlaybackFragment.java index 7e9070665b74b4..325b33e85b79f4 100644 --- a/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MediaPlaybackFragment.java +++ b/examples/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/MediaPlaybackFragment.java @@ -8,6 +8,7 @@ import android.widget.TextView; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentActivity; import com.chip.casting.ContentApp; import com.chip.casting.FailureCallback; import com.chip.casting.MatterError; @@ -63,13 +64,16 @@ public void onClick(View v) { TextView currentStateValue = getView().findViewById(R.id.currentStateValue); SuccessCallback successCallback = - playbackStateEnum -> { - Log.d( - TAG, - "handle() called on SuccessCallback with " - + playbackStateEnum); - getActivity() - .runOnUiThread( + new SuccessCallback() { + @Override + public void handle(MediaPlaybackTypes.PlaybackStateEnum playbackStateEnum) { + Log.d( + TAG, + "handle() called on SuccessCallback with " + + playbackStateEnum); + FragmentActivity fragmentActivity = getActivity(); + if (fragmentActivity != null) { + fragmentActivity.runOnUiThread( new Runnable() { @Override public void run() { @@ -78,6 +82,8 @@ public void run() { } } }); + } + } }; FailureCallback failureCallback = @@ -97,18 +103,22 @@ public void run() { }; SubscriptionEstablishedCallback subscriptionEstablishedCallback = - (SubscriptionEstablishedCallback) - () -> { - Log.d(TAG, "handle() called on SubscriptionEstablishedCallback"); - getActivity() - .runOnUiThread( - new Runnable() { - @Override - public void run() { - subscriptionStatus.setText("Subscription established!"); - } - }); - }; + new SubscriptionEstablishedCallback() { + @Override + public void handle() { + Log.d(TAG, "handle() called on SubscriptionEstablishedCallback"); + FragmentActivity fragmentActivity = getActivity(); + if (fragmentActivity != null) { + fragmentActivity.runOnUiThread( + new Runnable() { + @Override + public void run() { + subscriptionStatus.setText("Subscription established!"); + } + }); + } + } + }; boolean retVal = tvCastingApp.mediaPlayback_subscribeToCurrentState( diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/FailureCallback.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/FailureCallback.java index 1d6b4097e77cbc..6d6ddaa3357f6b 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/FailureCallback.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/FailureCallback.java @@ -17,10 +17,18 @@ */ package com.chip.casting; +import android.util.Log; + public abstract class FailureCallback { + private static final String TAG = FailureCallback.class.getSimpleName(); + public abstract void handle(MatterError err); - public final void handle(int errorCode, String errorMessage) { - handle(new MatterError(errorCode, errorMessage)); + private final void handleInternal(int errorCode, String errorMessage) { + try { + handle(new MatterError(errorCode, errorMessage)); + } catch (Exception e) { + Log.e(TAG, "FailureCallback::Caught an unhandled exception from the client: " + e); + } } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterCallbackHandler.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterCallbackHandler.java index a06f12248883db..0bf29b07ede3df 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterCallbackHandler.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterCallbackHandler.java @@ -17,10 +17,18 @@ */ package com.chip.casting; +import android.util.Log; + public abstract class MatterCallbackHandler { + private static final String TAG = MatterCallbackHandler.class.getSimpleName(); + public abstract void handle(MatterError err); - public final void handle(int errorCode, String errorMessage) { - handle(new MatterError(errorCode, errorMessage)); + private final void handleInternal(int errorCode, String errorMessage) { + try { + handle(new MatterError(errorCode, errorMessage)); + } catch (Exception e) { + Log.e(TAG, "MatterCallbackHandler::Caught an unhandled exception from the client: " + e); + } } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SubscriptionEstablishedCallback.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SubscriptionEstablishedCallback.java index e182c80e7ccdef..b821af9e7c3ebf 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SubscriptionEstablishedCallback.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SubscriptionEstablishedCallback.java @@ -17,6 +17,20 @@ */ package com.chip.casting; -public interface SubscriptionEstablishedCallback { - void handle(); +import android.util.Log; + +public abstract class SubscriptionEstablishedCallback { + private static final String TAG = SubscriptionEstablishedCallback.class.getSimpleName(); + + public abstract void handle(); + + private void handleInternal() { + try { + handle(); + } catch (Exception e) { + Log.e( + TAG, + "SubscriptionEstablishedCallback::Caught an unhandled exception from the client: " + e); + } + } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SuccessCallback.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SuccessCallback.java index 125ce4daabfc90..c20d640d49975b 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SuccessCallback.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SuccessCallback.java @@ -17,6 +17,18 @@ */ package com.chip.casting; -public interface SuccessCallback { - void handle(R response); +import android.util.Log; + +public abstract class SuccessCallback { + private static final String TAG = SuccessCallback.class.getSimpleName(); + + public abstract void handle(R response); + + public void handleInternal(R response) { + try { + handle(response); + } catch (Exception e) { + Log.e(TAG, "SuccessCallback::Caught an unhandled exception from the client: " + e); + } + } } diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp index cff8ddd8ba9677..3b5832af055963 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp +++ b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp @@ -61,6 +61,7 @@ CHIP_ERROR convertJAppParametersToCppAppParams(jobject appParameters, AppParams CHIP_ERROR convertJContentAppToTargetEndpointInfo(jobject contentApp, TargetEndpointInfo & outTargetEndpointInfo) { ChipLogProgress(AppServer, "convertJContentAppToTargetEndpointInfo called"); + VerifyOrReturnError(contentApp != nullptr, CHIP_ERROR_INVALID_ARGUMENT); JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread(); jclass jContentAppClass; @@ -124,6 +125,7 @@ CHIP_ERROR convertTargetEndpointInfoToJContentApp(TargetEndpointInfo * targetEnd CHIP_ERROR convertJVideoPlayerToTargetVideoPlayerInfo(jobject videoPlayer, TargetVideoPlayerInfo & outTargetVideoPlayerInfo) { ChipLogProgress(AppServer, "convertJVideoPlayerToTargetVideoPlayerInfo called"); + VerifyOrReturnError(videoPlayer != nullptr, CHIP_ERROR_INVALID_ARGUMENT); JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread(); jclass jVideoPlayerClass; @@ -241,6 +243,7 @@ CHIP_ERROR convertJDiscoveredNodeDataToCppDiscoveredNodeData(jobject jDiscovered chip::Dnssd::DiscoveredNodeData & outCppDiscoveredNodeData) { ChipLogProgress(AppServer, "convertJDiscoveredNodeDataToCppDiscoveredNodeData called"); + VerifyOrReturnError(jDiscoveredNodeData != nullptr, CHIP_ERROR_INVALID_ARGUMENT); JNIEnv * env = chip::JniReferences::GetInstance().GetEnvForCurrentThread(); diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp index eb19010ea33853..0c841c4f19039f 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp +++ b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp @@ -31,10 +31,10 @@ CHIP_ERROR CallbackBaseJNI::SetUp(JNIEnv * env, jobject inHandler) mClazz = env->GetObjectClass(mObject); VerifyOrExit(mClazz != nullptr, ChipLogError(AppServer, "Failed to get handler Java class")); - mMethod = env->GetMethodID(mClazz, "handle", mMethodSignature); + mMethod = env->GetMethodID(mClazz, "handleInternal", mMethodSignature); if (mMethod == nullptr) { - ChipLogError(AppServer, "Failed to access 'handle' method with signature %s", mMethodSignature); + ChipLogError(AppServer, "Failed to access 'handleInternal' method with signature %s", mMethodSignature); env->ExceptionClear(); }