Skip to content

Commit e2026dc

Browse files
StypoxStypox
authored
Merge pull request #12606 from Stypox/do-not-startService-randomly
2 parents 00f6203 + aa2b482 commit e2026dc

File tree

4 files changed

+62
-43
lines changed

4 files changed

+62
-43
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,10 +1416,8 @@ public void onReceive(final Context context, final Intent intent) {
14161416
bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED);
14171417
}
14181418
// Rebound to the service if it was closed via notification or mini player
1419-
if (!playerHolder.isBound()) {
1420-
playerHolder.startService(
1421-
false, VideoDetailFragment.this);
1422-
}
1419+
playerHolder.setListener(VideoDetailFragment.this);
1420+
playerHolder.tryBindIfNeeded(context);
14231421
break;
14241422
}
14251423
}

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer;
4141
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
4242
import org.schabi.newpipe.player.notification.NotificationPlayerUi;
43+
import org.schabi.newpipe.player.notification.NotificationUtil;
4344
import org.schabi.newpipe.util.ThemeHelper;
4445

4546
import java.lang.ref.WeakReference;
@@ -156,25 +157,24 @@ public int onStartCommand(final Intent intent, final int flags, final int startI
156157
}
157158
}
158159

159-
if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
160-
&& (player == null || player.getPlayQueue() == null)) {
161-
/*
162-
No need to process media button's actions if the player is not working, otherwise
163-
the player service would strangely start with nothing to play
164-
Stop the service in this case, which will be removed from the foreground and its
165-
notification cancelled in its destruction
166-
*/
160+
if (player == null) {
161+
// No need to process media button's actions or other system intents if the player is
162+
// not running. However, since the current intent might have been issued by the system
163+
// with `startForegroundService()` (for unknown reasons), we need to ensure that we post
164+
// a (dummy) foreground notification, otherwise we'd incur in
165+
// "Context.startForegroundService() did not then call Service.startForeground()". Then
166+
// we stop the service again.
167+
Log.d(TAG, "onStartCommand() got a useless intent, closing the service");
168+
NotificationUtil.startForegroundWithDummyNotification(this);
167169
destroyPlayerAndStopService();
168170
return START_NOT_STICKY;
169171
}
170172

171-
if (player != null) {
172-
final PlayerType oldPlayerType = player.getPlayerType();
173-
player.handleIntent(intent);
174-
player.handleIntentPost(oldPlayerType);
175-
player.UIs().get(MediaSessionPlayerUi.class)
176-
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
177-
}
173+
final PlayerType oldPlayerType = player.getPlayerType();
174+
player.handleIntent(intent);
175+
player.handleIntentPost(oldPlayerType);
176+
player.UIs().get(MediaSessionPlayerUi.class)
177+
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
178178

179179
return START_NOT_STICKY;
180180
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,11 @@ public void onServiceConnected(final ComponentName compName, final IBinder servi
192192
startPlayerListener();
193193
// ^ will call listener.onPlayerConnected() down the line if there is an active player
194194

195-
// notify the main activity that binding the service has completed, so that it can
196-
// open the bottom mini-player
197-
NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
195+
if (playerService != null && playerService.getPlayer() != null) {
196+
// notify the main activity that binding the service has completed and that there is
197+
// a player, so that it can open the bottom mini-player
198+
NavigationHelper.sendPlayerStartedEvent(localBinder.getService());
199+
}
198200
}
199201
}
200202

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

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_CLOSE;
66

77
import android.annotation.SuppressLint;
8+
import android.app.Notification;
89
import android.app.PendingIntent;
10+
import android.content.Context;
911
import android.content.Intent;
1012
import android.content.pm.ServiceInfo;
1113
import android.graphics.Bitmap;
@@ -24,6 +26,7 @@
2426
import org.schabi.newpipe.R;
2527
import org.schabi.newpipe.player.Player;
2628
import org.schabi.newpipe.player.PlayerIntentType;
29+
import org.schabi.newpipe.player.PlayerService;
2730
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
2831
import org.schabi.newpipe.util.NavigationHelper;
2932

@@ -90,12 +93,9 @@ private synchronized NotificationCompat.Builder createNotification() {
9093
Log.d(TAG, "createNotification()");
9194
}
9295
notificationManager = NotificationManagerCompat.from(player.getContext());
93-
final NotificationCompat.Builder builder =
94-
new NotificationCompat.Builder(player.getContext(),
95-
player.getContext().getString(R.string.notification_channel_id));
96-
final MediaStyle mediaStyle = new MediaStyle();
9796

9897
// setup media style (compact notification slots and media session)
98+
final MediaStyle mediaStyle = new MediaStyle();
9999
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
100100
// notification actions are ignored on Android 13+, and are replaced by code in
101101
// MediaSessionPlayerUi
@@ -108,18 +108,9 @@ private synchronized NotificationCompat.Builder createNotification() {
108108
.ifPresent(mediaStyle::setMediaSession);
109109

110110
// setup notification builder
111-
builder.setStyle(mediaStyle)
112-
.setPriority(NotificationCompat.PRIORITY_HIGH)
113-
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
114-
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
115-
.setShowWhen(false)
116-
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
117-
.setColor(ContextCompat.getColor(player.getContext(),
118-
R.color.dark_background_color))
111+
final var builder = setupNotificationBuilder(player.getContext(), mediaStyle)
119112
.setColorized(player.getPrefs().getBoolean(
120-
player.getContext().getString(R.string.notification_colorize_key), true))
121-
.setDeleteIntent(PendingIntentCompat.getBroadcast(player.getContext(),
122-
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
113+
player.getContext().getString(R.string.notification_colorize_key), true));
123114

124115
// set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
125116
setLargeIcon(builder);
@@ -168,17 +159,17 @@ public boolean shouldUpdateBufferingSlot() {
168159
&& notificationBuilder.mActions.get(2).actionIntent != null);
169160
}
170161

162+
public static void startForegroundWithDummyNotification(final PlayerService service) {
163+
final var builder = setupNotificationBuilder(service, new MediaStyle());
164+
startForeground(service, builder.build());
165+
}
166+
171167
public void createNotificationAndStartForeground() {
172168
if (notificationBuilder == null) {
173169
notificationBuilder = createNotification();
174170
}
175171
updateNotification();
176-
177-
// ServiceInfo constants are not used below Android Q, so 0 is set here
178-
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
179-
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
180-
ServiceCompat.startForeground(player.getService(), NOTIFICATION_ID,
181-
notificationBuilder.build(), serviceType);
172+
startForeground(player.getService(), notificationBuilder.build());
182173
}
183174

184175
public void cancelNotificationAndStopForeground() {
@@ -192,6 +183,34 @@ public void cancelNotificationAndStopForeground() {
192183
}
193184

194185

186+
/////////////////////////////////////////////////////
187+
// STATIC FUNCTIONS IN COMMON BETWEEN DUMMY AND REAL NOTIFICATION
188+
/////////////////////////////////////////////////////
189+
190+
private static NotificationCompat.Builder setupNotificationBuilder(final Context context,
191+
final MediaStyle style) {
192+
return new NotificationCompat.Builder(context,
193+
context.getString(R.string.notification_channel_id))
194+
.setStyle(style)
195+
.setPriority(NotificationCompat.PRIORITY_HIGH)
196+
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
197+
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
198+
.setShowWhen(false)
199+
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
200+
.setColor(ContextCompat.getColor(context, R.color.dark_background_color))
201+
.setDeleteIntent(PendingIntentCompat.getBroadcast(context,
202+
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
203+
}
204+
205+
private static void startForeground(final PlayerService service,
206+
final Notification notification) {
207+
// ServiceInfo constants are not used below Android Q, so 0 is set here
208+
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
209+
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
210+
ServiceCompat.startForeground(service, NOTIFICATION_ID, notification, serviceType);
211+
}
212+
213+
195214
/////////////////////////////////////////////////////
196215
// ACTIONS
197216
/////////////////////////////////////////////////////

0 commit comments

Comments
 (0)