Post dummy notification then close player service on invalid intent

This should solve "Context.startForegroundService() did not then call Service.startForeground()" according to #12489 (comment)

Author: Stypox

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

@@ -40,6 +40,7 @@
 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,23 +157,22 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
             }
         }
 
-        if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
-                && (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
-             */
+        if (player == 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
+            // with `startForegroundService()` (for unknown reasons), we need to ensure that we post
+            // a (dummy) foreground notification, otherwise we'd incur in
+            // "Context.startForegroundService() did not then call Service.startForeground()". Then
+            // 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) {
-            player.handleIntent(intent);
-            player.UIs().get(MediaSessionPlayerUi.class)
-                    .ifPresent(ui -> ui.handleMediaButtonIntent(intent));
-        }
+        player.handleIntent(intent);
+        player.UIs().get(MediaSessionPlayerUi.class)
+                .ifPresent(ui -> ui.handleMediaButtonIntent(intent));
 
         return START_NOT_STICKY;
     }

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

@@ -5,7 +5,9 @@
 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;
@@ -23,6 +25,7 @@
 import org.schabi.newpipe.MainActivity;
 import org.schabi.newpipe.R;
 import org.schabi.newpipe.player.Player;
+import org.schabi.newpipe.player.PlayerService;
 import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
 import org.schabi.newpipe.util.NavigationHelper;
 
@@ -89,12 +92,9 @@ private synchronized NotificationCompat.Builder createNotification() {
             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
@@ -107,18 +107,9 @@ private synchronized NotificationCompat.Builder createNotification() {
                 .ifPresent(mediaStyle::setMediaSession);
 
         // setup notification builder
-        builder.setStyle(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))
+        final var builder = setupNotificationBuilder(player.getContext(), mediaStyle)
                 .setColorized(player.getPrefs().getBoolean(
-                        player.getContext().getString(R.string.notification_colorize_key), true))
-                .setDeleteIntent(PendingIntentCompat.getBroadcast(player.getContext(),
-                        NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
+                        player.getContext().getString(R.string.notification_colorize_key), true));
 
         // set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
         setLargeIcon(builder);
@@ -167,17 +158,17 @@ public boolean shouldUpdateBufferingSlot() {
                 && 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();
-
-        // 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);
+        startForeground(player.getService(), notificationBuilder.build());
     }
 
     public void cancelNotificationAndStopForeground() {
@@ -191,6 +182,34 @@ public void cancelNotificationAndStopForeground() {
     }
 
 
+    /////////////////////////////////////////////////////
+    // 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
     /////////////////////////////////////////////////////