Skip to content

Commit

Permalink
tv-casting-app: Handle uncaught exceptions before JNI crashes app and…
Browse files Browse the repository at this point in the history 
… make iOS discoverCommissioners API callback based (#24896)

* iOS tv-casting-app: Making discovery API callback based

* tv-casting-app: Handled uncaught exceptions and null pointer issues

* Changing catch-all to catch Throwable instead of just Exception
  • Loading branch information
@sharadb-amazon @pull
sharadb-amazon authored and pull[bot] committed Dec 1, 2023
1 parent e1373fd commit 1760754
Show file tree
Hide file tree
Showing 16 changed files with 235 additions and 56 deletions.
55 changes: 38 additions & 17 deletions ...s/tv-casting-app/android/App/app/src/main/java/com/chip/casting/app/CertTestFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -269,39 +273,39 @@ private void runCertTests(Activity activity) {
successFailureCallback,
() -> {
tvCastingApp.applicationBasic_readApplicationVersion(
kContentApp, successFailureCallback, successFailureCallback);
kContentApp, successCallback, failureCallback);
});

runAndWait(
"applicationBasic_readVendorName",
successFailureCallback,
() -> {
tvCastingApp.applicationBasic_readVendorName(
kContentApp, successFailureCallback, successFailureCallback);
kContentApp, successCallback, failureCallback);
});

runAndWait(
"applicationBasic_readApplicationName",
successFailureCallback,
() -> {
tvCastingApp.applicationBasic_readApplicationName(
kContentApp, successFailureCallback, successFailureCallback);
kContentApp, successCallback, failureCallback);
});

runAndWait(
"applicationBasic_readVendorID",
successFailureCallback,
() -> {
tvCastingApp.applicationBasic_readVendorID(
kContentApp, successFailureCallbackInteger, successFailureCallbackInteger);
kContentApp, successCallbackInteger, failureCallback);
});

runAndWait(
"applicationBasic_readProductID",
successFailureCallback,
() -> {
tvCastingApp.applicationBasic_readProductID(
kContentApp, successFailureCallbackInteger, successFailureCallbackInteger);
kContentApp, successCallbackInteger, failureCallback);
});

runAndWait(
Expand All @@ -320,7 +324,7 @@ public void handle(MediaPlaybackTypes.PlaybackStateEnum response) {
activity, MatterError.NO_ERROR, "mediaPlayback_subscribeToCurrentState");
}
},
successFailureCallback,
failureCallback,
0,
20,
new SubscriptionEstablishedCallback() {
Expand Down Expand Up @@ -415,8 +419,7 @@ public void handle(MatterError error) {
}
}

class CertTestMatterSuccessFailureCallback extends FailureCallback
implements SuccessCallback<String> {
class CertTestMatterSuccessFailureCallback {
private Activity activity;
private String testMethod;
private CountDownLatch cdl;
Expand All @@ -433,7 +436,6 @@ public void setCountDownLatch(CountDownLatch cdl) {
this.cdl = cdl;
}

@Override
public void handle(MatterError error) {
try {
cdl.countDown();
Expand All @@ -446,7 +448,6 @@ public void handle(MatterError error) {
}
}

@Override
public void handle(String response) {
try {
cdl.countDown();
Expand All @@ -460,18 +461,38 @@ public void handle(String response) {
}
}

class CertTestMatterSuccessFailureCallbackInteger extends FailureCallback
implements SuccessCallback<Integer> {
class CertTestMatterSuccessCallback extends SuccessCallback<String> {
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<Integer> {

private CertTestMatterSuccessFailureCallback delegate;

CertTestMatterSuccessCallbackInteger(CertTestMatterSuccessFailureCallback delegate) {
this.delegate = delegate;
}

@Override
Expand Down
48 changes: 29 additions & 19 deletions ...casting-app/android/App/app/src/main/java/com/chip/casting/app/MediaPlaybackFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -63,13 +64,16 @@ public void onClick(View v) {
TextView currentStateValue = getView().findViewById(R.id.currentStateValue);

SuccessCallback<MediaPlaybackTypes.PlaybackStateEnum> successCallback =
playbackStateEnum -> {
Log.d(
TAG,
"handle() called on SuccessCallback<MediaPlaybackResponseTypes.PlaybackStateEnum> with "
+ playbackStateEnum);
getActivity()
.runOnUiThread(
new SuccessCallback<MediaPlaybackTypes.PlaybackStateEnum>() {
@Override
public void handle(MediaPlaybackTypes.PlaybackStateEnum playbackStateEnum) {
Log.d(
TAG,
"handle() called on SuccessCallback<MediaPlaybackResponseTypes.PlaybackStateEnum> with "
+ playbackStateEnum);
FragmentActivity fragmentActivity = getActivity();
if (fragmentActivity != null) {
fragmentActivity.runOnUiThread(
new Runnable() {
@Override
public void run() {
Expand All @@ -78,6 +82,8 @@ public void run() {
}
}
});
}
}
};

FailureCallback failureCallback =
Expand All @@ -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(
Expand Down
12 changes: 10 additions & 2 deletions examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/FailureCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (Throwable t) {
Log.e(TAG, "FailureCallback::Caught an unhandled Throwable from the client: " + t);
}
}
}
12 changes: 10 additions & 2 deletions ...s/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/MatterCallbackHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (Throwable t) {
Log.e(TAG, "MatterCallbackHandler::Caught an unhandled Throwable from the client: " + t);
}
}
}
18 changes: 16 additions & 2 deletions ...ng-app/android/App/app/src/main/jni/com/chip/casting/SubscriptionEstablishedCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (Throwable t) {
Log.e(
TAG,
"SubscriptionEstablishedCallback::Caught an unhandled Throwable from the client: " + t);
}
}
}
16 changes: 14 additions & 2 deletions examples/tv-casting-app/android/App/app/src/main/jni/com/chip/casting/SuccessCallback.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@
*/
package com.chip.casting;

public interface SuccessCallback<R> {
void handle(R response);
import android.util.Log;

public abstract class SuccessCallback<R> {
private static final String TAG = SuccessCallback.class.getSimpleName();

public abstract void handle(R response);

public void handleInternal(R response) {
try {
handle(response);
} catch (Throwable t) {
Log.e(TAG, "SuccessCallback::Caught an unhandled Throwable from the client: " + t);
}
}
}
3 changes: 3 additions & 0 deletions examples/tv-casting-app/android/App/app/src/main/jni/cpp/ConversionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions examples/tv-casting-app/android/App/app/src/main/jni/cpp/MatterCallbackHandler-JNI.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
4 changes: 4 additions & 0 deletions ...-casting-app/darwin/MatterTvCastingBridge/MatterTvCastingBridge.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
3CCB8742286A593700771BAD /* ConversionUtils.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 3CCB873C286A593700771BAD /* ConversionUtils.hpp */; };
3CCB8743286A593700771BAD /* CastingServerBridge.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3CCB873D286A593700771BAD /* CastingServerBridge.mm */; };
3CCB8744286A593700771BAD /* ConversionUtils.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3CCB873E286A593700771BAD /* ConversionUtils.mm */; };
3CD6D01A298CDA2100D7569A /* CommissionerDiscoveryDelegateImpl.h in Headers */ = {isa = PBXBuildFile; fileRef = 3CD6D019298CDA2100D7569A /* CommissionerDiscoveryDelegateImpl.h */; };
3CE868F42946D76200FCB92B /* CommissionableDataProviderImpl.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3CE868F32946D76200FCB92B /* CommissionableDataProviderImpl.mm */; };
3CF8532728E37F1000F07B9F /* MatterError.mm in Sources */ = {isa = PBXBuildFile; fileRef = 3CF8532628E37F1000F07B9F /* MatterError.mm */; };
/* End PBXBuildFile section */
Expand Down Expand Up @@ -60,6 +61,7 @@
3CCB873C286A593700771BAD /* ConversionUtils.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = ConversionUtils.hpp; sourceTree = "<group>"; };
3CCB873D286A593700771BAD /* CastingServerBridge.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CastingServerBridge.mm; sourceTree = "<group>"; };
3CCB873E286A593700771BAD /* ConversionUtils.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ConversionUtils.mm; sourceTree = "<group>"; };
3CD6D019298CDA2100D7569A /* CommissionerDiscoveryDelegateImpl.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CommissionerDiscoveryDelegateImpl.h; sourceTree = "<group>"; };
3CE868F32946D76200FCB92B /* CommissionableDataProviderImpl.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CommissionableDataProviderImpl.mm; sourceTree = "<group>"; };
3CF8532528E37ED800F07B9F /* MatterError.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MatterError.h; sourceTree = "<group>"; };
3CF8532628E37F1000F07B9F /* MatterError.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = MatterError.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -128,6 +130,7 @@
3C26AC8F2927008900BA6881 /* DeviceAttestationCredentialsProviderImpl.mm */,
3C0D9CDF2920A30C00D3332B /* CommissionableDataProviderImpl.hpp */,
3CE868F32946D76200FCB92B /* CommissionableDataProviderImpl.mm */,
3CD6D019298CDA2100D7569A /* CommissionerDiscoveryDelegateImpl.h */,
);
path = MatterTvCastingBridge;
sourceTree = "<group>";
Expand All @@ -139,6 +142,7 @@
isa = PBXHeadersBuildPhase;
buildActionMask = 2147483647;
files = (
3CD6D01A298CDA2100D7569A /* CommissionerDiscoveryDelegateImpl.h in Headers */,
3C26AC8C2926FE0C00BA6881 /* DeviceAttestationCredentialsProviderImpl.hpp in Headers */,
3CCB8740286A593700771BAD /* CastingServerBridge.h in Headers */,
3CCB8742286A593700771BAD /* ConversionUtils.hpp in Headers */,
Expand Down
3 changes: 2 additions & 1 deletion ...s/tv-casting-app/darwin/MatterTvCastingBridge/MatterTvCastingBridge/CastingServerBridge.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@
@param discoveryRequestSentHandler Handler to call after the Commissioner discovery request has been sent
*/
- (void)discoverCommissioners:(dispatch_queue_t _Nonnull)clientQueue
discoveryRequestSentHandler:(nullable void (^)(bool))discoveryRequestSentHandler;
discoveryRequestSentHandler:(nullable void (^)(bool))discoveryRequestSentHandler
discoveredCommissionerHandler:(nullable void (^)(DiscoveredNodeData * _Nonnull))discoveredCommissionerHandler;

/*!
@brief Retrieve a discovered commissioner TV
Expand Down
Loading

0 comments on commit 1760754

Please sign in to comment.