1
0
Fork 0
mirror of https://github.com/TeamNewPipe/NewPipe.git synced 2025-10-02 17:29:31 +02:00

Compare commits

...

5 commits

Author SHA1 Message Date
Stypox
e2026dc378
Merge pull request #12606 from Stypox/do-not-startService-randomly 2025-09-30 17:47:08 +02:00
Stypox
00f6203904
Merge pull request #12605 from TeamNewPipe/open-in-browser 2025-09-30 17:45:29 +02:00
Stypox
aa2b4821e2
Post dummy notification then close player service on invalid intent
This should solve "Context.startForegroundService() did not then call Service.startForeground()" according to https://github.com/TeamNewPipe/NewPipe/issues/12489#issuecomment-3290318112
2025-09-17 11:50:46 +02:00
Stypox
92a07a3445
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()`
2025-09-17 11:49:16 +02:00
Fynn Godau
83a0abddcc Fix and simplify openUrlInBrowser
The code was not previously working in case no default browser is set[1]
AND NewPipe is set as default handler for the link in question.

We improve it by telling the system to choose the target app as if the
URI was `http://`, which works even if the user has not set a default
browser.

[1]: also the case if platform refuses to tell an app what the user's
default browser is, which I observed on CalyxOS.
2025-09-05 17:49:58 +02:00
5 changed files with 92 additions and 78 deletions

View file

@ -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;
}
}

View file

@ -40,6 +40,7 @@ import org.schabi.newpipe.player.mediabrowser.MediaBrowserImpl;
import org.schabi.newpipe.player.mediabrowser.MediaBrowserPlaybackPreparer;
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.player.notification.NotificationPlayerUi;
import org.schabi.newpipe.player.notification.NotificationUtil;
import org.schabi.newpipe.util.ThemeHelper;
import java.lang.ref.WeakReference;
@ -156,25 +157,24 @@ public final class PlayerService extends MediaBrowserServiceCompat {
}
}
if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction())
&& (player == null || player.getPlayQueue() == null)) {
/*
No need to process media button's actions if the player is not working, otherwise
the player service would strangely start with nothing to play
Stop the service in this case, which will be removed from the foreground and its
notification cancelled in its destruction
*/
if (player == null) {
// No need to process media button's actions or other system intents if the player is
// not running. However, since the current intent might have been issued by the system
// with `startForegroundService()` (for unknown reasons), we need to ensure that we post
// a (dummy) foreground notification, otherwise we'd incur in
// "Context.startForegroundService() did not then call Service.startForeground()". Then
// we stop the service again.
Log.d(TAG, "onStartCommand() got a useless intent, closing the service");
NotificationUtil.startForegroundWithDummyNotification(this);
destroyPlayerAndStopService();
return START_NOT_STICKY;
}
if (player != null) {
final PlayerType oldPlayerType = player.getPlayerType();
player.handleIntent(intent);
player.handleIntentPost(oldPlayerType);
player.UIs().get(MediaSessionPlayerUi.class)
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
}
final PlayerType oldPlayerType = player.getPlayerType();
player.handleIntent(intent);
player.handleIntentPost(oldPlayerType);
player.UIs().get(MediaSessionPlayerUi.class)
.ifPresent(ui -> ui.handleMediaButtonIntent(intent));
return START_NOT_STICKY;
}

View file

@ -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());
}
}
}

View file

@ -5,7 +5,9 @@ import static androidx.media.app.NotificationCompat.MediaStyle;
import static org.schabi.newpipe.player.notification.NotificationConstants.ACTION_CLOSE;
import android.annotation.SuppressLint;
import android.app.Notification;
import android.app.PendingIntent;
import android.content.Context;
import android.content.Intent;
import android.content.pm.ServiceInfo;
import android.graphics.Bitmap;
@ -24,6 +26,7 @@ import org.schabi.newpipe.MainActivity;
import org.schabi.newpipe.R;
import org.schabi.newpipe.player.Player;
import org.schabi.newpipe.player.PlayerIntentType;
import org.schabi.newpipe.player.PlayerService;
import org.schabi.newpipe.player.mediasession.MediaSessionPlayerUi;
import org.schabi.newpipe.util.NavigationHelper;
@ -90,12 +93,9 @@ public final class NotificationUtil {
Log.d(TAG, "createNotification()");
}
notificationManager = NotificationManagerCompat.from(player.getContext());
final NotificationCompat.Builder builder =
new NotificationCompat.Builder(player.getContext(),
player.getContext().getString(R.string.notification_channel_id));
final MediaStyle mediaStyle = new MediaStyle();
// setup media style (compact notification slots and media session)
final MediaStyle mediaStyle = new MediaStyle();
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
// notification actions are ignored on Android 13+, and are replaced by code in
// MediaSessionPlayerUi
@ -108,18 +108,9 @@ public final class NotificationUtil {
.ifPresent(mediaStyle::setMediaSession);
// setup notification builder
builder.setStyle(mediaStyle)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
.setShowWhen(false)
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
.setColor(ContextCompat.getColor(player.getContext(),
R.color.dark_background_color))
final var builder = setupNotificationBuilder(player.getContext(), mediaStyle)
.setColorized(player.getPrefs().getBoolean(
player.getContext().getString(R.string.notification_colorize_key), true))
.setDeleteIntent(PendingIntentCompat.getBroadcast(player.getContext(),
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
player.getContext().getString(R.string.notification_colorize_key), true));
// set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
setLargeIcon(builder);
@ -168,17 +159,17 @@ public final class NotificationUtil {
&& notificationBuilder.mActions.get(2).actionIntent != null);
}
public static void startForegroundWithDummyNotification(final PlayerService service) {
final var builder = setupNotificationBuilder(service, new MediaStyle());
startForeground(service, builder.build());
}
public void createNotificationAndStartForeground() {
if (notificationBuilder == null) {
notificationBuilder = createNotification();
}
updateNotification();
// ServiceInfo constants are not used below Android Q, so 0 is set here
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
ServiceCompat.startForeground(player.getService(), NOTIFICATION_ID,
notificationBuilder.build(), serviceType);
startForeground(player.getService(), notificationBuilder.build());
}
public void cancelNotificationAndStopForeground() {
@ -192,6 +183,34 @@ public final class NotificationUtil {
}
/////////////////////////////////////////////////////
// STATIC FUNCTIONS IN COMMON BETWEEN DUMMY AND REAL NOTIFICATION
/////////////////////////////////////////////////////
private static NotificationCompat.Builder setupNotificationBuilder(final Context context,
final MediaStyle style) {
return new NotificationCompat.Builder(context,
context.getString(R.string.notification_channel_id))
.setStyle(style)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
.setCategory(NotificationCompat.CATEGORY_TRANSPORT)
.setShowWhen(false)
.setSmallIcon(R.drawable.ic_newpipe_triangle_white)
.setColor(ContextCompat.getColor(context, R.color.dark_background_color))
.setDeleteIntent(PendingIntentCompat.getBroadcast(context,
NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT, false));
}
private static void startForeground(final PlayerService service,
final Notification notification) {
// ServiceInfo constants are not used below Android Q, so 0 is set here
final int serviceType = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q
? ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK : 0;
ServiceCompat.startForeground(service, NOTIFICATION_ID, notification, serviceType);
}
/////////////////////////////////////////////////////
// ACTIONS
/////////////////////////////////////////////////////

View file

@ -5,10 +5,9 @@ import static org.schabi.newpipe.MainActivity.DEBUG;
import android.content.ActivityNotFoundException;
import android.content.ClipData;
import android.content.ClipboardManager;
import android.content.ComponentName;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.content.pm.ResolveInfo;
import android.graphics.Bitmap;
import android.net.Uri;
import android.os.Build;
@ -23,6 +22,7 @@ import androidx.core.content.FileProvider;
import org.schabi.newpipe.BuildConfig;
import org.schabi.newpipe.R;
import org.schabi.newpipe.RouterActivity;
import org.schabi.newpipe.extractor.Image;
import org.schabi.newpipe.util.image.ImageStrategy;
import org.schabi.newpipe.util.image.PicassoHelper;
@ -62,8 +62,9 @@ public final class ShareUtils {
}
/**
* Open the url with the system default browser. If no browser is set as default, falls back to
* {@link #openAppChooser(Context, Intent, boolean)}.
* Open the url with the system default browser. If no browser is installed, falls back to
* {@link #openAppChooser(Context, Intent, boolean)} (for displaying that no apps are available
* to handle the action, or possible OEM-related edge cases).
* <p>
* This function selects the package to open based on which apps respond to the {@code http://}
* schema alone, which should exclude special non-browser apps that are can handle the url (e.g.
@ -77,44 +78,26 @@ public final class ShareUtils {
* @param url the url to browse
**/
public static void openUrlInBrowser(@NonNull final Context context, final String url) {
// Resolve using a generic http://, so we are sure to get a browser and not e.g. the yt app.
// Target a generic http://, so we are sure to get a browser and not e.g. the yt app.
// Note that this requires the `http` schema to be added to `<queries>` in the manifest.
final ResolveInfo defaultBrowserInfo;
final Intent browserIntent = new Intent(Intent.ACTION_VIEW, Uri.parse("http://"));
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
defaultBrowserInfo = context.getPackageManager().resolveActivity(browserIntent,
PackageManager.ResolveInfoFlags.of(PackageManager.MATCH_DEFAULT_ONLY));
} else {
defaultBrowserInfo = context.getPackageManager().resolveActivity(browserIntent,
PackageManager.MATCH_DEFAULT_ONLY);
}
final Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url))
.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
if (defaultBrowserInfo == null) {
// No app installed to open a web URL, but it may be handled by other apps so try
// opening a system chooser for the link in this case (it could be bypassed by the
// system if there is only one app which can open the link or a default app associated
// with the link domain on Android 12 and higher)
// See https://stackoverflow.com/a/58801285 and `setSelector` documentation
intent.setSelector(browserIntent);
try {
context.startActivity(intent);
} catch (final ActivityNotFoundException e) {
// No browser is available. This should, in the end, yield a nice AOSP error message
// indicating that no app is available to handle this action.
//
// Note: there are some situations where modified OEM ROMs have apps that appear
// to be browsers but are actually app choosers. If starting the Activity fails
// related to this, opening the system app chooser is still the correct behavior.
intent.setSelector(null);
openAppChooser(context, intent, true);
return;
}
final String defaultBrowserPackage = defaultBrowserInfo.activityInfo.packageName;
if (defaultBrowserPackage.equals("android")) {
// No browser set as default (doesn't work on some devices)
openAppChooser(context, intent, true);
} else {
try {
intent.setPackage(defaultBrowserPackage);
context.startActivity(intent);
} catch (final ActivityNotFoundException e) {
// Not a browser but an app chooser because of OEMs changes
intent.setPackage(null);
openAppChooser(context, intent, true);
}
}
}
@ -190,6 +173,18 @@ public final class ShareUtils {
chooserIntent.putExtra(Intent.EXTRA_TITLE, context.getString(R.string.open_with));
}
// Avoid opening in NewPipe
// (Implementation note: if the URL is one for which NewPipe itself
// is set as handler on Android >= 12, we actually remove the only eligible app
// for this link, and browsers will not be offered to the user. For that, use
// `openUrlInBrowser`.)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
chooserIntent.putExtra(
Intent.EXTRA_EXCLUDE_COMPONENTS,
new ComponentName[]{new ComponentName(context, RouterActivity.class)}
);
}
// Migrate any clip data and flags from the original intent.
final int permFlags = intent.getFlags() & (Intent.FLAG_GRANT_READ_URI_PERMISSION
| Intent.FLAG_GRANT_WRITE_URI_PERMISSION