Merge pull request #12606 from Stypox/do-not-startService-randomly

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java

@@ -1416,10 +1416,8 @@ public final class VideoDetailFragment
                             bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
                         }
                         // Rebound to the service if it was closed via notification or mini player
-                        if (!playerHolder.isBound()) {
+                        playerHolder.setListener(VideoDetailFragment.this);
-                            playerHolder.startService(
+                        playerHolder.tryBindIfNeeded(context);
-                                    false, VideoDetailFragment.this);
-                        }
                         break;
                 }
             }

app/src/main/java/org/schabi/newpipe/player/PlayerService.java

@@ -40,6 +40,7 @@ import org.schabi.newpipe.player.mediabrowser.MediaBrowserImpl;
 import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer;
 import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
 import org.schabi.newpipe.player.notification.NotificationPlayerUi;
+import org.schabi.newpipe.player.notification.NotificationUtil;
 import org.schabi.newpipe.util.ThemeHelper;
 
 import java.lang.ref.WeakReference;
@@ -156,25 +157,24 @@ public final class PlayerService extends MediaBrowserServiceCompat {
             }
         }
 
-        if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
+        if (player == null) {
-                && (player == null || player.getPlayQueue() == null)) {
+            // No need to process media button's actions or other system intents if the player is
-            /*
+            // not running. However, since the current intent might have been issued by the system
-            No need to process media button's actions if the player is not working, otherwise
+            // with `startForegroundService()` (for unknown reasons), we need to ensure that we post
-            the player service would strangely start with nothing to play
+            // a (dummy) foreground notification, otherwise we'd incur in
-            Stop the service in this case, which will be removed from the foreground and its
+            // "Context.startForegroundService() did not then call Service.startForeground()". Then
-            notification cancelled in its destruction
+            // we stop the service again.
-             */
+            Log.d(TAG, "onStartCommand() got a useless intent, closing the service");
+            NotificationUtil.startForegroundWithDummyNotification(this);
             destroyPlayerAndStopService();
             return START_NOT_STICKY;
         }
 
-        if (player != null) {
+        final PlayerType oldPlayerType = player.getPlayerType();
-            final PlayerType oldPlayerType = player.getPlayerType();
+        player.handleIntent(intent);
-            player.handleIntent(intent);
+        player.handleIntentPost(oldPlayerType);
-            player.handleIntentPost(oldPlayerType);
+        player.UIs().get(MediaSessionPlayerUi.class)
-            player.UIs().get(MediaSessionPlayerUi.class)
+                .ifPresent(ui -> ui.handleMediaButtonIntent(intent));
-                    .ifPresent(ui -> ui.handleMediaButtonIntent(intent));
-        }
 
         return START_NOT_STICKY;
     }

app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java

@@ -192,9 +192,11 @@ public final class PlayerHolder {
             startPlayerListener();
             // ^ will call listener.onPlayerConnected() down the line if there is an active player
 
-            // notify the main activity that binding the service has completed, so that it can
+            if (playerService != null && playerService.getPlayer() != null) {
-            // open the bottom mini-player
+                // notify the main activity that binding the service has completed and that there is
-            NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
+                // a player, so that it can open the bottom mini-player
+                NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
+            }
         }
     }
 

app/src/main/java/org/schabi/newpipe/player/notification/NotificationUtil.java

@@ -5,7 +5,9 @@ import static androidx.media.app.NotificationCompat.MediaStyle;
 import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_CLOSE;
 
 import android.annotation.SuppressLint;
+import android.app.Notification;
 import android.app.PendingIntent;
+import android.content.Context;
 import android.content.Intent;
 import android.content.pm.ServiceInfo;
 import android.graphics.Bitmap;
@@ -24,6 +26,7 @@ import org.schabi.newpipe.MainActivity;
 import org.schabi.newpipe.R;
 import org.schabi.newpipe.player.Player;
 import org.schabi.newpipe.player.PlayerIntentType;
+import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
 import org.schabi.newpipe.util.NavigationHelper;
 
@@ -90,12 +93,9 @@ public final class NotificationUtil {
             Log.d(TAG, "createNotification()");
         }
         notificationManager = NotificationManagerCompat.from(player.getContext());
-        final NotificationCompat.Builder builder =
-                new NotificationCompat.Builder(player.getContext(),
-                player.getContext().getString(R.string.notification_channel_id));
-        final MediaStyle mediaStyle = new MediaStyle();
 
         // setup media style (compact notification slots and media session)
+        final MediaStyle mediaStyle = new MediaStyle();
         if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
             // notification actions are ignored on Android 13+, and are replaced by code in
             // MediaSessionPlayerUi
@@ -108,18 +108,9 @@ public final class NotificationUtil {
                 .ifPresent(mediaStyle::setMediaSession);
 
         // setup notification builder
-        builder.setStyle(mediaStyle)
+        final var builder = setupNotificationBuilder(player.getContext(), mediaStyle)
-                .setPriority(NotificationCompat.PRIORITY_HIGH)
-                .setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
-                .setCategory(NotificationCompat.CATEGORY_TRANSPORT)
-                .setShowWhen(false)
-                .setSmallIcon(R.drawable.ic_newpipe_triangle_white)
-                .setColor(ContextCompat.getColor(player.getContext(),
-                        R.color.dark_background_color))
                 .setColorized(player.getPrefs().getBoolean(
-                        player.getContext().getString(R.string.notification_colorize_key), true))
+                        player.getContext().getString(R.string.notification_colorize_key), true));
-                .setDeleteIntent(PendingIntentCompat.getBroadcast(player.getContext(),
-                        NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
 
         // set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
         setLargeIcon(builder);
@@ -168,17 +159,17 @@ public final class NotificationUtil {
                 && notificationBuilder.mActions.get(2).actionIntent != null);
     }
 
+    public static void startForegroundWithDummyNotification(final PlayerService service) {
+        final var builder = setupNotificationBuilder(service, new MediaStyle());
+        startForeground(service, builder.build());
+    }
+
     public void createNotificationAndStartForeground() {
         if (notificationBuilder == null) {
             notificationBuilder = createNotification();
         }
         updateNotification();
-
+        startForeground(player.getService(), notificationBuilder.build());
-        // ServiceInfo constants are not used below Android Q, so 0 is set here
-        final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
-                ? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
-        ServiceCompat.startForeground(player.getService(), NOTIFICATION_ID,
-                notificationBuilder.build(), serviceType);
     }
 
     public void cancelNotificationAndStopForeground() {
@@ -192,6 +183,34 @@ public final class NotificationUtil {
     }
 
 
+    /////////////////////////////////////////////////////
+    // STATIC FUNCTIONS IN COMMON BETWEEN DUMMY AND REAL NOTIFICATION
+    /////////////////////////////////////////////////////
+
+    private static NotificationCompat.Builder setupNotificationBuilder(final Context context,
+                                                                       final MediaStyle style) {
+        return new NotificationCompat.Builder(context,
+                context.getString(R.string.notification_channel_id))
+                .setStyle(style)
+                .setPriority(NotificationCompat.PRIORITY_HIGH)
+                .setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
+                .setCategory(NotificationCompat.CATEGORY_TRANSPORT)
+                .setShowWhen(false)
+                .setSmallIcon(R.drawable.ic_newpipe_triangle_white)
+                .setColor(ContextCompat.getColor(context, R.color.dark_background_color))
+                .setDeleteIntent(PendingIntentCompat.getBroadcast(context,
+                        NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
+    }
+
+    private static void startForeground(final PlayerService service,
+                                        final Notification notification) {
+        // ServiceInfo constants are not used below Android Q, so 0 is set here
+        final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
+                ? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
+        ServiceCompat.startForeground(service, NOTIFICATION_ID, notification, serviceType);
+    }
+
+
     /////////////////////////////////////////////////////
     // ACTIONS
     /////////////////////////////////////////////////////