From b5c63db0e9b0d7c7b5c983340cb9cb4b4b8ee003 Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Fri, 23 Aug 2019 18:54:05 +0200 Subject: Fixed 'Service started without startForeground' When we called stopForeground(), the service went to background state. If we then somehow receive a second intent to onStartCommand (without onCreate), we assume that the service already is in foreground, what is wrong. This commit moves the service to foreground in onStartCommand if it is no longer in foreground. --- .../core/service/playback/PlaybackService.java | 77 +++++++++++----------- .../playback/PlaybackServiceStateManager.java | 43 ++++++++++++ .../core/util/playback/PlaybackController.java | 2 +- 3 files changed, 81 insertions(+), 41 deletions(-) create mode 100644 core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceStateManager.java (limited to 'core/src/main/java/de') diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java index ab1edc8e9..1eb0628d1 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackService.java @@ -195,10 +195,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { * Is true if service is running. */ public static boolean isRunning = false; - /** - * Is true if service has received a valid start command. - */ - public static boolean started = false; /** * Is true if the service was running, but paused due to headphone disconnect */ @@ -214,6 +210,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { private PlaybackServiceMediaPlayer mediaPlayer; private PlaybackServiceTaskManager taskManager; private PlaybackServiceFlavorHelper flavorHelper; + private PlaybackServiceStateManager stateManager; /** * Used for Lollipop notifications, Android Wear, and Android Auto. @@ -268,8 +265,9 @@ public class PlaybackService extends MediaBrowserServiceCompat { Log.d(TAG, "Service created."); isRunning = true; + stateManager = new PlaybackServiceStateManager(this); PlaybackServiceNotificationBuilder notificationBuilder = new PlaybackServiceNotificationBuilder(this); - startForeground(NOTIFICATION_ID, notificationBuilder.build()); + stateManager.startForeground(NOTIFICATION_ID, notificationBuilder.build()); registerReceiver(autoStateUpdated, new IntentFilter("com.google.android.gms.car.media.STATUS")); registerReceiver(headsetDisconnected, new IntentFilter(Intent.ACTION_HEADSET_PLUG)); @@ -328,9 +326,8 @@ public class PlaybackService extends MediaBrowserServiceCompat { public void onDestroy() { super.onDestroy(); Log.d(TAG, "Service is about to be destroyed"); - stopForeground(true); + stateManager.stopForeground(true); isRunning = false; - started = false; currentMediaType = MediaType.UNKNOWN; PreferenceManager.getDefaultSharedPreferences(this) @@ -351,11 +348,6 @@ public class PlaybackService extends MediaBrowserServiceCompat { mediaPlayer.shutdown(); taskManager.shutdown(); } - - private void stopService() { - stopForeground(true); - stopSelf(); - } @Override public BrowserRoot onGetRoot(@NonNull String clientPackageName, int clientUid, Bundle rootHints) { @@ -449,28 +441,34 @@ public class PlaybackService extends MediaBrowserServiceCompat { super.onStartCommand(intent, flags, startId); Log.d(TAG, "OnStartCommand called"); + + if (!stateManager.isInForeground()) { + PlaybackServiceNotificationBuilder notificationBuilder = new PlaybackServiceNotificationBuilder(this); + startForeground(NOTIFICATION_ID, notificationBuilder.build()); + } + final int keycode = intent.getIntExtra(MediaButtonReceiver.EXTRA_KEYCODE, -1); final boolean castDisconnect = intent.getBooleanExtra(EXTRA_CAST_DISCONNECT, false); Playable playable = intent.getParcelableExtra(EXTRA_PLAYABLE); if (keycode == -1 && playable == null && !castDisconnect) { Log.e(TAG, "PlaybackService was started with no arguments"); - stopService(); + stateManager.stopService(); return Service.START_NOT_STICKY; } if ((flags & Service.START_FLAG_REDELIVERY) != 0) { Log.d(TAG, "onStartCommand is a redelivered intent, calling stopForeground now."); - stopForeground(true); + stateManager.stopForeground(true); } else { if (keycode != -1) { Log.d(TAG, "Received media button event"); boolean handled = handleKeycode(keycode, true); if (!handled) { - stopService(); + stateManager.stopService(); return Service.START_NOT_STICKY; } } else if (!flavorHelper.castDisconnect(castDisconnect) && playable != null) { - started = true; + stateManager.validStartCommandWasReceived(); boolean stream = intent.getBooleanExtra(EXTRA_SHOULD_STREAM, true); boolean allowStreamThisTime = intent.getBooleanExtra(EXTRA_ALLOW_STREAM_THIS_TIME, false); boolean startWhenPrepared = intent.getBooleanExtra(EXTRA_START_WHEN_PREPARED, false); @@ -484,7 +482,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { if (stream && !NetworkUtils.isStreamingAllowed() && !allowStreamThisTime) { displayStreamingNotAllowedNotification(intent); writePlaybackPreferencesNoMediaPlaying(); - stopService(); + stateManager.stopService(); return Service.START_NOT_STICKY; } mediaPlayer.playMediaObject(playable, stream, startWhenPrepared, prepareImmediately); @@ -590,10 +588,9 @@ public class PlaybackService extends MediaBrowserServiceCompat { case KeyEvent.KEYCODE_MEDIA_STOP: if (status == PlayerStatus.PLAYING) { mediaPlayer.pause(true, true); - started = false; } - stopForeground(true); // gets rid of persistent notification + stateManager.stopForeground(true); // gets rid of persistent notification return true; default: Log.d(TAG, "Unhandled key code: " + keycode); @@ -609,10 +606,10 @@ public class PlaybackService extends MediaBrowserServiceCompat { Playable playable = Playable.PlayableUtils.createInstanceFromPreferences(getApplicationContext()); if (playable != null) { mediaPlayer.playMediaObject(playable, false, true, true); - started = true; + stateManager.validStartCommandWasReceived(); PlaybackService.this.updateMediaSessionMetadata(playable); } else { - stopService(); + stateManager.stopService(); } } @@ -629,7 +626,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { mediaPlayer.pause(true, false); mediaPlayer.resetVideoSurface(); setupNotification(getPlayable()); - stopForeground(!UserPreferences.isPersistNotify()); + stateManager.stopForeground(!UserPreferences.isPersistNotify()); } private final PlaybackServiceTaskManager.PSTMCallback taskManagerCallback = new PlaybackServiceTaskManager.PSTMCallback() { @@ -699,7 +696,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { setupNotification(newInfo); } else if (!UserPreferences.isPersistNotify() && !isCasting) { // remove notification on pause - stopForeground(true); + stateManager.stopForeground(true); } writePlayerStatusPlaybackPreferences(); break; @@ -712,7 +709,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { case PLAYING: writePlayerStatusPlaybackPreferences(); setupNotification(newInfo); - started = true; + stateManager.validStartCommandWasReceived(); // set sleep timer if auto-enabled if (newInfo.oldPlayerStatus != null && newInfo.oldPlayerStatus != PlayerStatus.SEEKING && SleepTimerPreferences.autoEnable() && !sleepTimerActive()) { @@ -723,7 +720,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { case ERROR: writePlaybackPreferencesNoMediaPlaying(); - stopService(); + stateManager.stopService(); break; } @@ -736,7 +733,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { @Override public void shouldStop() { - stopService(); + stateManager.stopService(); } @Override @@ -785,7 +782,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { } sendNotificationBroadcast(NOTIFICATION_TYPE_ERROR, what); writePlaybackPreferencesNoMediaPlaying(); - stopService(); + stateManager.stopService(); return true; } @@ -870,7 +867,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { .shouldStream(true) .getIntent()); writePlaybackPreferencesNoMediaPlaying(); - stopService(); + stateManager.stopService(); return null; } return nextItem.getMedia(); @@ -886,7 +883,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { taskManager.cancelPositionSaver(); writePlaybackPreferencesNoMediaPlaying(); if (!isCasting) { - stopForeground(true); + stateManager.stopForeground(true); } } if (mediaType == null) { @@ -1218,7 +1215,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { builder.putString(MediaMetadataCompat.METADATA_KEY_DISPLAY_ICON_URI, imageLocation); } } - if (!Thread.currentThread().isInterrupted() && started) { + if (!Thread.currentThread().isInterrupted() && stateManager.hasReceivedValidStartCommand()) { mediaSession.setSessionActivity(PendingIntent.getActivity(this, 0, PlaybackService.getPlayerActivityIntent(this), PendingIntent.FLAG_UPDATE_CURRENT)); @@ -1254,8 +1251,8 @@ public class PlaybackService extends MediaBrowserServiceCompat { } if (playable == null) { Log.d(TAG, "setupNotification: playable is null" + Log.getStackTraceString(new Exception())); - if (!started) { - stopService(); + if (!stateManager.hasReceivedValidStartCommand()) { + stateManager.stopService(); } return; } @@ -1266,8 +1263,8 @@ public class PlaybackService extends MediaBrowserServiceCompat { if (mediaPlayer == null) { Log.d(TAG, "notificationSetupTask: mediaPlayer is null"); - if (!started) { - stopService(); + if (!stateManager.hasReceivedValidStartCommand()) { + stateManager.stopService(); } return; } @@ -1279,20 +1276,20 @@ public class PlaybackService extends MediaBrowserServiceCompat { if (!notificationBuilder.isIconCached(playable)) { // To make sure that the notification is shown instantly notificationBuilder.loadDefaultIcon(); - startForeground(NOTIFICATION_ID, notificationBuilder.build()); + stateManager.startForeground(NOTIFICATION_ID, notificationBuilder.build()); } notificationBuilder.loadIcon(playable); - if (!Thread.currentThread().isInterrupted() && started) { + if (!Thread.currentThread().isInterrupted() && stateManager.hasReceivedValidStartCommand()) { Notification notification = notificationBuilder.build(); if (playerStatus == PlayerStatus.PLAYING || playerStatus == PlayerStatus.PREPARING || playerStatus == PlayerStatus.SEEKING || isCasting) { - startForeground(NOTIFICATION_ID, notification); + stateManager.startForeground(NOTIFICATION_ID, notification); } else { - stopForeground(false); + stateManager.stopForeground(false); NotificationManager mNotificationManager = (NotificationManager) getSystemService(NOTIFICATION_SERVICE); mNotificationManager.notify(NOTIFICATION_ID, notification); } @@ -1481,7 +1478,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { @Override public void onReceive(Context context, Intent intent) { if (TextUtils.equals(intent.getAction(), ACTION_SHUTDOWN_PLAYBACK_SERVICE)) { - stopService(); + stateManager.stopService(); } } @@ -1832,7 +1829,7 @@ public class PlaybackService extends MediaBrowserServiceCompat { android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { PlaybackService.this.setupNotification(info); } else if (!UserPreferences.isPersistNotify()) { - PlaybackService.this.stopForeground(true); + stateManager.stopForeground(true); } } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceStateManager.java b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceStateManager.java new file mode 100644 index 000000000..b8541748e --- /dev/null +++ b/core/src/main/java/de/danoeh/antennapod/core/service/playback/PlaybackServiceStateManager.java @@ -0,0 +1,43 @@ +package de.danoeh.antennapod.core.service.playback; + +import android.app.Notification; + +class PlaybackServiceStateManager { + private final PlaybackService playbackService; + + private volatile boolean isInForeground = false; + private volatile boolean hasReceivedValidStartCommand = false; + + PlaybackServiceStateManager(PlaybackService playbackService) { + this.playbackService = playbackService; + } + + void startForeground(int notificationId, Notification notification) { + playbackService.startForeground(notificationId, notification); + isInForeground = true; + } + + void stopService() { + stopForeground(true); + isInForeground = false; + playbackService.stopSelf(); + } + + void stopForeground(boolean removeNotification) { + playbackService.stopForeground(removeNotification); + isInForeground = false; + hasReceivedValidStartCommand = false; + } + + boolean isInForeground() { + return isInForeground; + } + + boolean hasReceivedValidStartCommand() { + return hasReceivedValidStartCommand; + } + + void validStartCommandWasReceived() { + this.hasReceivedValidStartCommand = true; + } +} diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java index 6191d4491..5b2d7b13e 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java +++ b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackController.java @@ -210,7 +210,7 @@ public abstract class PlaybackController { .observeOn(AndroidSchedulers.mainThread()) .subscribe(optionalIntent -> { boolean bound = false; - if (!PlaybackService.started) { + if (!PlaybackService.isRunning) { if (optionalIntent.isPresent()) { Log.d(TAG, "Calling start service"); ContextCompat.startForegroundService(activity, optionalIntent.get()); -- cgit v1.2.3