start playbackService code paths reviewed (context.startService() and

ContextCompat.startForegroundService())
This commit is contained in:
orionlee 2019-01-05 14:35:53 -08:00
parent 76fbab8e82
commit e26a54bdbc
4 changed files with 26 additions and 17 deletions

View File

@ -21,13 +21,14 @@ import de.danoeh.antennapod.core.storage.DownloadRequester;
import de.danoeh.antennapod.core.util.IntentUtils;
import de.danoeh.antennapod.core.util.LongList;
import de.danoeh.antennapod.core.util.NetworkUtils;
import de.danoeh.antennapod.core.util.playback.PlaybackServiceStarter;
/**
* Default implementation of an ActionButtonCallback
*/
public class DefaultActionButtonCallback implements ActionButtonCallback {
private static final String TAG = "DefaultActionButtonCallback";
private static final String TAG = "DefaultActionBtnCb";
private final Context context;
@ -81,16 +82,12 @@ public class DefaultActionButtonCallback implements ActionButtonCallback {
}
} else { // media is downloaded
if (media.isCurrentlyPlaying()) {
// new PlaybackServiceStarter(context, media) // TODO: [2716] probably not needed but not 100% sure
// .startWhenPrepared(true)
// .shouldStream(false)
// .start();
IntentUtils.sendLocalBroadcast(context, PlaybackService.ACTION_PAUSE_PLAY_CURRENT_EPISODE);
} else if (media.isCurrentlyPaused()) {
// new PlaybackServiceStarter(context, media) // TODO: [2716] probably not needed but not 100% sure
// .startWhenPrepared(true)
// .shouldStream(false)
// .start();
new PlaybackServiceStarter(context, media) // need to start the service in case it's been stopped by system.
.startWhenPrepared(true)
.shouldStream(false)
.start();
IntentUtils.sendLocalBroadcast(context, PlaybackService.ACTION_RESUME_PLAY_CURRENT_EPISODE);
} else {
DBTasks.playMedia(context, media, false, true, false);

View File

@ -76,7 +76,13 @@ import de.greenrobot.event.EventBus;
/**
* Controls the MediaPlayer that plays a FeedMedia-file
* TODO [2716] add some guidelines on how to / not to start / stop the service from callers.
*
* Callers should connect to the service with either:
* - .bindService()
* - .startService(), optionally with arguments such as media to be played.
*
* Caller should not call startForegroundService(). The PlaybackService will make itself foreground
* when appropriate.
*/
public class PlaybackService extends MediaBrowserServiceCompat {
/**
@ -463,11 +469,10 @@ public class PlaybackService extends MediaBrowserServiceCompat {
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");
// some code call startService with no arg, causing playback to stop prematurely in typical case
// temporarily comment it out for now.
// TODO: Next step - find the source of the no-arg service start
// stopService(); // TODO: find the source of starting service with no argument that causes me to comment out this line temporarily
// Typical cases when the service was started with no argument
// - when it is first bound, and then moved to startedState, as in <code>serviceManager.moveServiceToStartedState()</code>
// - callers (e.g., Controller) explicitly
Log.d(TAG, "PlaybackService was started with no arguments.");
return Service.START_NOT_STICKY;
}

View File

@ -192,7 +192,6 @@ public abstract class PlaybackController {
if (!PlaybackService.started) {
if (intent != null) {
Log.d(TAG, "Calling start service");
// ContextCompat.startForegroundService(activity, intent); // TODO: [2716] likely not needed but is not 100% sure
bound = activity.bindService(intent, mConnection, 0);
} else {
status = PlayerStatus.STOPPED;
@ -587,7 +586,8 @@ public abstract class PlaybackController {
.startWhenPrepared(true)
.streamIfLastWasStream()
.start();
Log.w(TAG, "Play/Pause button was pressed, but playbackservice was null!");
Log.d(TAG, "Play/Pause button was pressed, but playbackservice was null - " +
"it is likely to have been released by Android system. Restarting it.");
return;
}
switch (status) {

View File

@ -2,11 +2,14 @@ package de.danoeh.antennapod.core.util.playback;
import android.content.Context;
import android.content.Intent;
import android.util.Log;
import de.danoeh.antennapod.core.preferences.PlaybackPreferences;
import de.danoeh.antennapod.core.service.playback.PlaybackService;
public class PlaybackServiceStarter {
private static final String TAG = "PlaybackServiceStarter";
private final Context context;
private final Playable media;
private boolean startWhenPrepared = false;
@ -63,6 +66,10 @@ public class PlaybackServiceStarter {
launchIntent.putExtra(PlaybackService.EXTRA_SHOULD_STREAM, shouldStream);
launchIntent.putExtra(PlaybackService.EXTRA_PREPARE_IMMEDIATELY, prepareImmediately);
if (media == null) {
Log.e(TAG, "getIntent() - media is unexpectedly null. intent:" + launchIntent);
}
return launchIntent;
}