From 92a07a34456c5df560454a335f89ba677719c313 Mon Sep 17 00:00:00 2001 From: Stypox Date: Fri, 5 Sep 2025 22:47:45 +0200 Subject: [PATCH] Use tryBindIfNeeded(), send player started only if player!=null This commit fixes one way ghost notifications could be produced (although I don't know if there are other ways). This is the call chain that would lead to ghost notifications being created: 1. the system starts `PlayerService` to query information from it, without providing `SHOULD_START_FOREGROUND_EXTRA=true`, so NewPipe does not start the player nor show any notification, as expected 2. the `PlayerHolder::serviceConnection.onServiceConnected()` gets called by the system to inform `PlayerHolder` that the player started 3. `PlayerHolder` notifies `MainActivity` that the player has started (although in fact only the service has started), by sending a `ACTION_PLAYER_STARTED` broadcast 4. `MainActivity` receives the `ACTION_PLAYER_STARTED` broadcast and brings up the mini-player, but then also tries to make `PlayerHolder` bind to `PlayerService` just in case it was not bound yet, but does so using `PlayerHolder::startService()` instead of the more passive `PlayerHolder::tryBindIfNeeded()` 5. `PlayerHolder::startService()` sends an intent to the `PlayerService` again, this time with `startForegroundService` and with `SHOULD_START_FOREGROUND_EXTRA=true` 6. the `PlayerService` receives the intent and due to `SHOULD_START_FOREGROUND_EXTRA=true` decides to start up the player and show a dummy notification Steps 3 and 4 are wrong, and this commit fixes them: 3. `PlayerHolder` will now broadcast `ACTION_PLAYER_STARTED` when the service connects, only if the player is not-null 4. `PlayerHolder::tryBindIfNeeded()` is now used to passively try to bind, instead of `PlayerHolder::startService()` --- .../newpipe/fragments/detail/VideoDetailFragment.java | 6 ++---- .../org/schabi/newpipe/player/helper/PlayerHolder.java | 8 +++++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index c43007da4..7b8705565 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1416,10 +1416,8 @@ public final class VideoDetailFragment bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); } // Rebound to the service if it was closed via notification or mini player - if (!playerHolder.isBound()) { - playerHolder.startService( - false, VideoDetailFragment.this); - } + playerHolder.setListener(VideoDetailFragment.this); + playerHolder.tryBindIfNeeded(context); break; } } diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java index 97f2d6717..9edfc804a 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerHolder.java @@ -192,9 +192,11 @@ public final class PlayerHolder { startPlayerListener(); // ^ will call listener.onPlayerConnected() down the line if there is an active player - // notify the main activity that binding the service has completed, so that it can - // open the bottom mini-player - NavigationHelper.sendPlayerStartedEvent(localBinder.getService()); + if (playerService != null && playerService.getPlayer() != null) { + // notify the main activity that binding the service has completed and that there is + // a player, so that it can open the bottom mini-player + NavigationHelper.sendPlayerStartedEvent(localBinder.getService()); + } } }