Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix potential crash when user reopens the app quickly #911

Merged
merged 2 commits into from
Dec 4, 2022
Merged

Fix potential crash when user reopens the app quickly #911

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca4ee1e
fixed potential crash when user reopens the app quickly
susonthapa Nov 27, 2022
d377753
Fix further crashes and rewrite parts of the callback logic
Maxr1998 Dec 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 63 additions & 37 deletions app/src/proprietary/java/org/jellyfin/mobile/player/cast/ChromecastConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import android.content.SharedPreferences;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.arch.core.util.Function;
import androidx.mediarouter.app.MediaRouteChooserDialog;
import androidx.mediarouter.media.MediaRouteSelector;
Expand All @@ -24,7 +26,6 @@

import org.jellyfin.mobile.R;
import org.jellyfin.mobile.bridge.JavascriptCallback;
import org.jellyfin.mobile.player.cast.CastOptionsProvider;
import org.json.JSONObject;

import java.util.ArrayList;
Expand All @@ -33,9 +34,15 @@

public class ChromecastConnection {

/**
* A shared handler for this connection instance.
*/
private final Handler handler;

/**
* Lifetime variable.
*/
@Nullable
private Activity activity;
/**
* settings object.
Expand Down Expand Up @@ -69,11 +76,12 @@ public class ChromecastConnection {
* @param connectionListener client callbacks for specific events
*/
ChromecastConnection(Activity act, Listener connectionListener) {
this.activity = act;
this.settings = activity.getSharedPreferences("CORDOVA-PLUGIN-CHROMECAST_ChromecastConnection", 0);
this.appId = settings.getString("appId", CastMediaControlIntent.DEFAULT_MEDIA_RECEIVER_APPLICATION_ID);
this.listener = connectionListener;
this.media = new ChromecastSession(activity, listener);
handler = new Handler(Looper.getMainLooper());
activity = act;
settings = activity.getSharedPreferences("CORDOVA-PLUGIN-CHROMECAST_ChromecastConnection", 0);
appId = settings.getString("appId", CastMediaControlIntent.DEFAULT_MEDIA_RECEIVER_APPLICATION_ID);
listener = connectionListener;
media = new ChromecastSession(activity, listener);

// Set the initial appId
CastOptionsProvider.setAppId(appId);
Expand All @@ -90,7 +98,7 @@ public class ChromecastConnection {
* @return the ChromecastSession object
*/
ChromecastSession getChromecastSession() {
return this.media;
return media;
}

/**
Expand Down Expand Up @@ -144,8 +152,9 @@ void onRouteUpdate(List<RouteInfo> routes) {
});
}

@Nullable
private MediaRouter getMediaRouter() {
return MediaRouter.getInstance(activity);
return activity != null ? MediaRouter.getInstance(activity) : null;
}

private CastContext getContext() {
Expand Down Expand Up @@ -174,19 +183,23 @@ private void setAppId(String applicationId) {
*/
private boolean isValidAppId(String applicationId) {
try {
MediaRouter mediaRouter = getMediaRouter();
if (mediaRouter == null) return false;
ScanCallback cb = new ScanCallback() {
@Override
void onRouteUpdate(List<RouteInfo> routes) {
}
};
// This will throw if the applicationId is invalid
getMediaRouter().addCallback(new MediaRouteSelector.Builder()
.addControlCategory(CastMediaControlIntent.categoryForCast(applicationId))
.build(),
cb,
MediaRouter.CALLBACK_FLAG_PERFORM_ACTIVE_SCAN);
mediaRouter.addCallback(
new MediaRouteSelector.Builder()
.addControlCategory(CastMediaControlIntent.categoryForCast(applicationId))
.build(),
cb,
MediaRouter.CALLBACK_FLAG_PERFORM_ACTIVE_SCAN
);
// If no exception we passed, so remove the callback
getMediaRouter().removeCallback(cb);
mediaRouter.removeCallback(cb);
return true;
} catch (IllegalArgumentException e) {
// Don't set the appId if it is not a valid receiverApplicationID
Expand All @@ -205,7 +218,7 @@ public void selectRoute(final String routeId, SelectRouteCallback callback) {
activity.runOnUiThread(() -> {
if (getSession() != null && getSession().isConnected()) {
callback.onError(ChromecastUtilities.createError("session_error",
"Leave or stop current session before attempting to join new session."));
"Leave or stop current session before attempting to join new session."));
return;
}

Expand Down Expand Up @@ -237,7 +250,7 @@ void onRouteUpdate(List<RouteInfo> routes) {
// https://github.com/jellyfin/cordova-plugin-chromecast/issues/48
try {
// Try selecting the route!
getMediaRouter().selectRoute(route);
Objects.requireNonNull(getMediaRouter()).selectRoute(route);
} catch (NullPointerException e) {
// Let it try to find the route again
foundRoute[0] = false;
Expand All @@ -253,7 +266,7 @@ void onRouteUpdate(List<RouteInfo> routes) {
// Feed current routes into scan so that it can retry.
// If route is there, it will try to join,
// if not, it should wait for the scan to find the route
scan.onRouteUpdate(getMediaRouter().getRoutes());
scan.onRouteUpdate(Objects.requireNonNull(getMediaRouter()).getRoutes());
};

Function<JSONObject, Void> sendErrorResult = message -> {
Expand Down Expand Up @@ -281,7 +294,7 @@ public boolean onSessionStartFailed(int errorCode) {
return false;
} else {
sendErrorResult.apply(ChromecastUtilities.createError("session_error",
"Failed to start session with error code: " + errorCode));
"Failed to start session with error code: " + errorCode));
return true;
}
}
Expand All @@ -294,14 +307,14 @@ public boolean onSessionEndedBeforeStart(int errorCode) {
return false;
} else {
sendErrorResult.apply(ChromecastUtilities.createError("session_error",
"Failed to to join existing route (" + routeId + ") " + retries[0] + 1 + " times before giving up."));
"Failed to to join existing route (" + routeId + ") " + retries[0] + 1 + " times before giving up."));
return true;
}
}
});

startRouteScan(15000L, scan, () ->
sendErrorResult.apply(ChromecastUtilities.createError("timeout", "Failed to join route (" + routeId + ") after 15s and " + (retries[0] + 1) + " tries."))
sendErrorResult.apply(ChromecastUtilities.createError("timeout", "Failed to join route (" + routeId + ") after 15s and " + (retries[0] + 1) + " tries."))
);
});
}
Expand Down Expand Up @@ -338,8 +351,8 @@ public void requestSession(RequestSessionCallback callback) {
// TODO accept theme as a config.xml option
MediaRouteChooserDialog builder = new MediaRouteChooserDialog(activity, R.style.AppTheme);
builder.setRouteSelector(new MediaRouteSelector.Builder()
.addControlCategory(CastMediaControlIntent.categoryForCast(appId))
.build());
.addControlCategory(CastMediaControlIntent.categoryForCast(appId))
.build());
builder.setCanceledOnTouchOutside(true);
builder.setOnCancelListener(dialog -> {
getSessionManager().removeSessionManagerListener(newConnectionListener, CastSession.class);
Expand All @@ -354,7 +367,7 @@ public void requestSession(RequestSessionCallback callback) {
}
builder.setOnDismissListener(dialog -> callback.onCancel());
builder.setPositiveButton("Stop Casting", (dialog, which) ->
endSession(true, null)
endSession(true, null)
);
builder.show();
}
Expand Down Expand Up @@ -405,9 +418,14 @@ public void onSessionEnded(@NonNull CastSession castSession, int errCode) {
* @param onTimeout called when the timeout hits
*/
public void startRouteScan(Long timeout, ScanCallback callback, Runnable onTimeout) {
if (activity == null) return;

// Add the callback in active scan mode
activity.runOnUiThread(() -> {
callback.setMediaRouter(getMediaRouter());
MediaRouter mediaRouter = getMediaRouter();
if (mediaRouter == null) return;

callback.setMediaRouter(mediaRouter);

if (timeout != null && timeout == 0) {
// Send out the one time routes
Expand All @@ -416,11 +434,12 @@ public void startRouteScan(Long timeout, ScanCallback callback, Runnable onTimeo
}

// Add the callback in active scan mode
getMediaRouter().addCallback(new MediaRouteSelector.Builder()
.addControlCategory(CastMediaControlIntent.categoryForCast(appId))
.build(),
callback,
MediaRouter.CALLBACK_FLAG_PERFORM_ACTIVE_SCAN);
mediaRouter.addCallback(new MediaRouteSelector.Builder()
.addControlCategory(CastMediaControlIntent.categoryForCast(appId))
.build(),
callback,
MediaRouter.CALLBACK_FLAG_PERFORM_ACTIVE_SCAN
);

// Send out the initial routes after the callback has been added.
// This is important because if the callback calls stopRouteScan only once, and it
Expand All @@ -430,9 +449,10 @@ public void startRouteScan(Long timeout, ScanCallback callback, Runnable onTimeo

if (timeout != null) {
// remove the callback after timeout ms, and notify caller
new Handler().postDelayed(() -> {
handler.postDelayed(() -> {
// And stop the scan for routes
getMediaRouter().removeCallback(callback);
// MediaRouter should never be null, since all callbacks are be removed in destroy()
Objects.requireNonNull(getMediaRouter()).removeCallback(callback);
// Notify
if (onTimeout != null) {
onTimeout.run();
Expand All @@ -449,13 +469,18 @@ public void startRouteScan(Long timeout, ScanCallback callback, Runnable onTimeo
* @param completionCallback called on completion
*/
public void stopRouteScan(ScanCallback callback, Runnable completionCallback) {
if (callback == null) {
completionCallback.run();
if (callback == null || activity == null) {
if (completionCallback != null) {
completionCallback.run();
}
return;
}
activity.runOnUiThread(() -> {
callback.stop();
getMediaRouter().removeCallback(callback);
MediaRouter mediaRouter = getMediaRouter();
if (mediaRouter != null) {
mediaRouter.removeCallback(callback);
}
if (completionCallback != null) {
completionCallback.run();
}
Expand Down Expand Up @@ -490,6 +515,7 @@ public void onSessionEnded(@NonNull CastSession castSession, int error) {

public void destroy() {
getSessionManager().removeSessionManagerListener(newConnectionListener, CastSession.class);
handler.removeCallbacksAndMessages(null);
activity = null;
}

Expand Down Expand Up @@ -647,17 +673,17 @@ private void onFilteredRouteUpdate() {
}

@Override
public final void onRouteAdded(MediaRouter router, RouteInfo route) {
public final void onRouteAdded(@NonNull MediaRouter router, @NonNull RouteInfo route) {
onFilteredRouteUpdate();
}

@Override
public final void onRouteChanged(MediaRouter router, RouteInfo route) {
public final void onRouteChanged(@NonNull MediaRouter router, @NonNull RouteInfo route) {
onFilteredRouteUpdate();
}

@Override
public final void onRouteRemoved(MediaRouter router, RouteInfo route) {
public final void onRouteRemoved(@NonNull MediaRouter router, @NonNull RouteInfo route) {
onFilteredRouteUpdate();
}
}
Expand Down