Merge pull request #10578 from AudricV/try-fix-player-service-foreground-start

Restore player service start handling before player UI separation and fix some issues in this service

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) {
+                && (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
+            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;
         }
 
+        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();

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();
+    }
 }