From 84d50da009b5f82bdb6df54addc13b72c6e69f5b Mon Sep 17 00:00:00 2001 From: AudricV <74829229+AudricV@users.noreply.github.com> Date: Sun, 17 Sep 2023 20:49:13 +0200 Subject: [PATCH] Restore player service start handling before player UI separation This behavior was present before 0.24.0 and the player UI separation and avoided crashes for which their exception contained "Context.startForegroundService() did not then call Service.startForeground()". Some player nullability checks have been also added, and the player service is now stopped when it has been started from a media button and there is nothing to play. --- .../schabi/newpipe/player/PlayerService.java | 47 +++++++++++++++---- .../notification/NotificationPlayerUi.java | 14 ++---- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java index ad6c9405d..e7abf4320 100644 --- a/app/src/main/java/org/schabi/newpipe/player/PlayerService.java +++ b/app/src/main/java/org/schabi/newpipe/player/PlayerService.java @@ -29,6 +29,7 @@ import android.os.IBinder; import android.util.Log; import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi; +import org.schabi.newpipe.player.notification.NotificationPlayerUi; import org.schabi.newpipe.util.ThemeHelper; import java.lang.ref.WeakReference; @@ -59,6 +60,14 @@ public final class PlayerService extends Service { ThemeHelper.setTheme(this); player = new Player(this); + /* + Create the player notification and start immediately the service in foreground, + otherwise if nothing is played or initializing the player and its components (especially + loading stream metadata) takes a lot of time, the app would crash on Android 8+ as the + service would never be put in the foreground while we said to the system we would do so + */ + player.UIs().get(NotificationPlayerUi.class) + .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); } @Override @@ -68,16 +77,38 @@ public final class PlayerService extends Service { + "], flags = [" + flags + "], startId = [" + startId + "]"); } + /* + Be sure that the player notification is set and the service is started in foreground, + otherwise, the app may crash on Android 8+ as the service would never be put in the + foreground while we said to the system we would do so + The service is always requested to be started in foreground, so always creating a + notification if there is no one already and starting the service in foreground should + not create any issues + If the service is already started in foreground, requesting it to be started shouldn't + do anything + */ + if (player != null) { + player.UIs().get(NotificationPlayerUi.class) + .ifPresent(NotificationPlayerUi::createNotificationAndStartForeground); + } + if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction()) - && player.getPlayQueue() == null) { - // No need to process media button's actions if the player is not working, otherwise the - // player service would strangely start with nothing to play + && (player == null || player.getPlayQueue() == null)) { + /* + No need to process media button's actions if the player is not working, otherwise + the player service would strangely start with nothing to play + Stop the service in this case, which will be removed from the foreground and its + notification cancelled in its destruction + */ + stopSelf(); return START_NOT_STICKY; } - player.handleIntent(intent); - player.UIs().get(MediaSessionPlayerUi.class) - .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); + if (player != null) { + player.handleIntent(intent); + player.UIs().get(MediaSessionPlayerUi.class) + .ifPresent(ui -> ui.handleMediaButtonIntent(intent)); + } return START_NOT_STICKY; } @@ -87,7 +118,7 @@ public final class PlayerService extends Service { Log.d(TAG, "stopForImmediateReusing() called"); } - if (!player.exoPlayerIsNull()) { + if (player != null && !player.exoPlayerIsNull()) { // Releases wifi & cpu, disables keepScreenOn, etc. // We can't just pause the player here because it will make transition // from one stream to a new stream not smooth @@ -98,7 +129,7 @@ public final class PlayerService extends Service { @Override public void onTaskRemoved(final Intent rootIntent) { super.onTaskRemoved(rootIntent); - if (!player.videoPlayerSelected()) { + if (player != null && !player.videoPlayerSelected()) { return; } onDestroy(); diff --git a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java index 4b1fc417f..75b27545c 100644 --- a/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/notification/NotificationPlayerUi.java @@ -17,7 +17,6 @@ import org.schabi.newpipe.player.helper.PlayerHelper; import org.schabi.newpipe.player.ui.PlayerUi; public final class NotificationPlayerUi extends PlayerUi { - private boolean foregroundNotificationAlreadyCreated = false; private final NotificationUtil notificationUtil; public NotificationPlayerUi(@NonNull final Player player) { @@ -25,15 +24,6 @@ public final class NotificationPlayerUi extends PlayerUi { notificationUtil = new NotificationUtil(player); } - @Override - public void initPlayer() { - super.initPlayer(); - if (!foregroundNotificationAlreadyCreated) { - notificationUtil.createNotificationAndStartForeground(); - foregroundNotificationAlreadyCreated = true; - } - } - @Override public void destroy() { super.destroy(); @@ -122,4 +112,8 @@ public final class NotificationPlayerUi extends PlayerUi { super.onPlayQueueEdited(); notificationUtil.createNotificationIfNeededAndUpdate(false); } + + public void createNotificationAndStartForeground() { + notificationUtil.createNotificationAndStartForeground(); + } }