PlayerHolder/PlayerService: inline & remove duplicate player passing

The player in playerHolder is exactly the player inside the
`PlayerService`, which in turn is exactly passed through the IBinder
interface. Thus we don’t have to pass both.

Instead add `PlayerService.getPlayer()`.

Also inline a few methods of `PlayerHolder` and simplify.

Author: Profpatsch

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

@@ -234,10 +234,9 @@ public final class VideoDetailFragment
     // Service management
     //////////////////////////////////////////////////////////////////////////*/
     @Override
-    public void onServiceConnected(final Player connectedPlayer,
-                                   final PlayerService connectedPlayerService,
+    public void onServiceConnected(final PlayerService connectedPlayerService,
                                    final boolean playAfterConnect) {
-        player = connectedPlayer;
+        player = connectedPlayerService.getPlayer();
         playerService = connectedPlayerService;
 
         // It will do nothing if the player is not in fullscreen mode

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

@@ -217,11 +217,16 @@ public void onServiceDisconnected(final ComponentName name) {
             }
 
             @Override
-            public void onServiceConnected(final ComponentName name, final IBinder service) {
+            public void onServiceConnected(final ComponentName name, final IBinder binder) {
                 Log.d(TAG, "Player service is connected");
 
-                if (service instanceof PlayerService.LocalBinder) {
-                    player = ((PlayerService.LocalBinder) service).getPlayer();
+                if (binder instanceof PlayerService.LocalBinder localBinder) {
+                    final @Nullable PlayerService s = localBinder.getService();
+                    if (s == null) {
+                        player = null;
+                    } else {
+                        player = s.getPlayer();
+                    }
                 }
 
                 if (player == null || player.getPlayQueue() == null || player.exoPlayerIsNull()) {

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

@@ -28,6 +28,8 @@
 import android.os.IBinder;
 import android.util.Log;
 
+import androidx.annotation.Nullable;
+
 import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
 import org.schabi.newpipe.player.notification.NotificationPlayerUi;
 import org.schabi.newpipe.util.ThemeHelper;
@@ -36,7 +38,9 @@
 
 
 /**
- * One service for all players.
+ * One background service for our player. Even though the player has multiple UIs
+ * (e.g. the audio-only UI, the main UI, the pulldown-menu UI),
+ * this allows us to keep playing even when switching between the different UIs.
  */
 public final class PlayerService extends Service {
     private static final String TAG = PlayerService.class.getSimpleName();
@@ -46,6 +50,9 @@ public final class PlayerService extends Service {
 
     private final IBinder mBinder = new PlayerService.LocalBinder(this);
 
+    public Player getPlayer() {
+        return player;
+    }
 
     /*//////////////////////////////////////////////////////////////////////////
     // Service's LifeCycle
@@ -167,19 +174,21 @@ public IBinder onBind(final Intent intent) {
         return mBinder;
     }
 
+    /** Allows us this {@link org.schabi.newpipe.player.PlayerService} over the Service boundary
+     * back to our {@link org.schabi.newpipe.player.helper.PlayerHolder}.
+      */
     public static class LocalBinder extends Binder {
         private final WeakReference<PlayerService> playerService;
 
         LocalBinder(final PlayerService playerService) {
             this.playerService = new WeakReference<>(playerService);
         }
 
-        public PlayerService getService() {
+        /** Get the PlayerService object itself.
+         * @return this
+         * */
+        public @Nullable PlayerService getService() {
             return playerService.get();
         }
-
-        public Player getPlayer() {
-            return playerService.get().player;
-        }
     }
 }

app/src/main/java/org/schabi/newpipe/player/event/PlayerServiceExtendedEventListener.java

@@ -1,11 +1,9 @@
 package org.schabi.newpipe.player.event;
 
 import org.schabi.newpipe.player.PlayerService;
-import org.schabi.newpipe.player.Player;
 
 public interface PlayerServiceExtendedEventListener extends PlayerServiceEventListener {
-    void onServiceConnected(Player player,
-                            PlayerService playerService,
+    void onServiceConnected(PlayerService playerService,
                             boolean playAfterConnect);
     void onServiceDisconnected();
 }

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

@@ -43,6 +43,7 @@ public static synchronized PlayerHolder getInstance() {
 
     private final PlayerServiceConnection serviceConnection = new PlayerServiceConnection();
     private boolean bound;
+
     @Nullable private PlayerService playerService;
     @Nullable private Player player;
 
@@ -108,13 +109,16 @@ public void setListener(@Nullable final PlayerServiceExtendedEventListener newLi
 
         // Force reload data from service
         if (player != null) {
-            listener.onServiceConnected(player, playerService, false);
-            startPlayerListener();
+            listener.onServiceConnected(playerService, false);
+            player.setFragmentListener(internalListener);
         }
     }
 
-    // helper to handle context in common place as using the same
-    // context to bind/unbind a service is crucial
+    /** Helper to handle context in common place as using the same
+     * context to bind/unbind a service is crucial.
+     *
+     * @return the common context
+     * */
     private Context getCommonContext() {
         return App.getInstance();
     }
@@ -131,7 +135,7 @@ public void startService(final boolean playAfterConnect,
         // bound twice. Prevent it with unbinding first
         unbind(context);
         ContextCompat.startForegroundService(context, new Intent(context, PlayerService.class));
-        serviceConnection.doPlayAfterConnect(playAfterConnect);
+        serviceConnection.playAfterConnect = playAfterConnect;
         bind(context);
     }
 
@@ -145,10 +149,6 @@ class PlayerServiceConnection implements ServiceConnection {
 
         private boolean playAfterConnect = false;
 
-        public void doPlayAfterConnect(final boolean playAfterConnection) {
-            this.playAfterConnect = playAfterConnection;
-        }
-
         @Override
         public void onServiceDisconnected(final ComponentName compName) {
             if (DEBUG) {
@@ -167,14 +167,21 @@ public void onServiceConnected(final ComponentName compName, final IBinder servi
             final PlayerService.LocalBinder localBinder = (PlayerService.LocalBinder) service;
 
             playerService = localBinder.getService();
-            player = localBinder.getPlayer();
+            player = playerService != null ? playerService.getPlayer() : null;
+
             if (listener != null) {
-                listener.onServiceConnected(player, playerService, playAfterConnect);
+                listener.onServiceConnected(playerService, playAfterConnect);
+            }
+            if (player != null) {
+                player.setFragmentListener(internalListener);
             }
-            startPlayerListener();
         }
     }
 
+    /** Connect to (and if needed start) the {@link PlayerService}
+     * and bind {@link PlayerServiceConnection} to it.
+     * @param context common holder context
+     * */
     private void bind(final Context context) {
         if (DEBUG) {
             Log.d(TAG, "bind() called");
@@ -196,7 +203,9 @@ private void unbind(final Context context) {
         if (bound) {
             context.unbindService(serviceConnection);
             bound = false;
-            stopPlayerListener();
+            if (player != null) {
+                player.removeFragmentListener(internalListener);
+            }
             playerService = null;
             player = null;
             if (listener != null) {
@@ -205,18 +214,6 @@ private void unbind(final Context context) {
         }
     }
 
-    private void startPlayerListener() {
-        if (player != null) {
-            player.setFragmentListener(internalListener);
-        }
-    }
-
-    private void stopPlayerListener() {
-        if (player != null) {
-            player.removeFragmentListener(internalListener);
-        }
-    }
-
     private final PlayerServiceEventListener internalListener =
             new PlayerServiceEventListener() {
                 @Override