From e26a54bdbccac6eb9d9060b725192c575b3913bb Mon Sep 17 00:00:00 2001
From: orionlee <orionlee@yahoo.com>
Date: Sat, 5 Jan 2019 14:35:53 -0800
Subject: start playbackService code paths reviewed (context.startService() and
 ContextCompat.startForegroundService())

---
 .../core/service/playback/PlaybackService.java          | 17 +++++++++++------
 .../core/util/playback/PlaybackController.java          |  4 ++--
 .../core/util/playback/PlaybackServiceStarter.java      |  7 +++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

(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 b0b3aee1c..f34866c25 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
@@ -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;
         }
 
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 3f538da84..565ef8a96 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
@@ -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) {
diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java
index c42b6dd49..a6cf500c1 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/playback/PlaybackServiceStarter.java
@@ -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;
     }
 
-- 
cgit v1.2.3