From 9cf0bc6c82751b2c5c672fc143efde101ee013e9 Mon Sep 17 00:00:00 2001 From: Stypox Date: Tue, 8 Sep 2020 21:19:43 +0200 Subject: [PATCH] Make notification creation and cancelling more consistent --- .../org/schabi/newpipe/player/MainPlayer.java | 12 ++---- .../newpipe/player/NotificationUtil.java | 41 ++++++++++--------- .../newpipe/player/VideoPlayerImpl.java | 41 +++++++------------ 3 files changed, 39 insertions(+), 55 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java b/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java index a16fc3cbf..b885d105b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java +++ b/app/src/main/java/org/schabi/newpipe/player/MainPlayer.java @@ -121,7 +121,7 @@ public final class MainPlayer extends Service { if (Intent.ACTION_MEDIA_BUTTON.equals(intent.getAction()) || intent.getStringExtra(VideoPlayer.PLAY_QUEUE_KEY) != null) { - showNotificationAndStartForeground(); + NotificationUtil.getInstance().createNotificationAndStartForeground(playerImpl, this); } playerImpl.handleIntent(intent); @@ -154,7 +154,7 @@ public final class MainPlayer extends Service { // So we should hide the notification at all. // When autoplay enabled such notification flashing is annoying so skip this case if (!autoplayEnabled) { - stopForeground(true); + NotificationUtil.getInstance().cancelNotificationAndStopForeground(this); } } } @@ -202,9 +202,8 @@ public final class MainPlayer extends Service { playerImpl.removePopupFromView(); playerImpl.destroy(); } - NotificationUtil.getInstance().cancelNotification(); - stopForeground(true); + NotificationUtil.getInstance().cancelNotificationAndStopForeground(this); stopSelf(); } @@ -243,11 +242,6 @@ public final class MainPlayer extends Service { } } - private void showNotificationAndStartForeground() { - NotificationUtil.getInstance().createNotificationIfNeeded(playerImpl, true); - NotificationUtil.getInstance().startForegroundServiceWithNotification(this); - } - public class LocalBinder extends Binder { diff --git a/app/src/main/java/org/schabi/newpipe/player/NotificationUtil.java b/app/src/main/java/org/schabi/newpipe/player/NotificationUtil.java index 2c5e882bb..e05d7d19d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/NotificationUtil.java +++ b/app/src/main/java/org/schabi/newpipe/player/NotificationUtil.java @@ -42,7 +42,7 @@ import static org.schabi.newpipe.player.MainPlayer.ACTION_SHUFFLE; * @author cool-student */ public final class NotificationUtil { - private static final String TAG = "NotificationUtil"; + private static final String TAG = NotificationUtil.class.getSimpleName(); private static final boolean DEBUG = BasePlayer.DEBUG; private static final int NOTIFICATION_ID = 123789; @@ -70,21 +70,25 @@ public final class NotificationUtil { ///////////////////////////////////////////////////// /** - * Creates the notification if it does not exist already or unless forceRecreate is true. + * Creates the notification if it does not exist already and recreates it if forceRecreate is + * true. Updates the notification with the data in the player. * @param player the player currently open, to take data from * @param forceRecreate whether to force the recreation of the notification even if it already * exists */ - void createNotificationIfNeeded(final VideoPlayerImpl player, final boolean forceRecreate) { + synchronized void createNotificationIfNeededAndUpdate(final VideoPlayerImpl player, + final boolean forceRecreate) { if (notificationBuilder == null || forceRecreate) { if (DEBUG) { - Log.d(TAG, "N_ createNotificationIfNeeded(true)"); + Log.d(TAG, "N_ createNotificationIfNeededAndUpdate(true)"); } notificationBuilder = createNotification(player); } + updateNotification(player); } - private NotificationCompat.Builder createNotification(final VideoPlayerImpl player) { + private synchronized NotificationCompat.Builder createNotification( + final VideoPlayerImpl player) { notificationManager = (NotificationManager) player.context.getSystemService(NOTIFICATION_SERVICE); final NotificationCompat.Builder builder = new NotificationCompat.Builder(player.context, @@ -115,17 +119,12 @@ public final class NotificationUtil { .setPriority(NotificationCompat.PRIORITY_HIGH) .setSmallIcon(R.drawable.ic_newpipe_triangle_white) .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) - .setContentTitle(player.getVideoTitle()) - .setContentText(player.getUploaderName()) .setColor(ContextCompat.getColor(player.context, R.color.gray)) .setContentIntent(PendingIntent.getActivity(player.context, NOTIFICATION_ID, getIntentForNotification(player), FLAG_UPDATE_CURRENT)) .setDeleteIntent(PendingIntent.getBroadcast(player.context, NOTIFICATION_ID, new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT)); - updateActions(builder, player); - setLargeIcon(builder, player); - return builder; } @@ -133,7 +132,7 @@ public final class NotificationUtil { * Updates the notification and the button icons depending on the playback state. * @param player the player currently open, to take data from */ - synchronized void updateNotification(final VideoPlayerImpl player) { + private synchronized void updateNotification(final VideoPlayerImpl player) { if (DEBUG) { Log.d(TAG, "N_ updateNotification()"); } @@ -151,7 +150,15 @@ public final class NotificationUtil { } - void startForegroundServiceWithNotification(final Service service) { + boolean hasSlotWithBuffering() { + return notificationSlots[1] == NotificationConstants.PLAY_PAUSE_BUFFERING + || notificationSlots[2] == NotificationConstants.PLAY_PAUSE_BUFFERING; + } + + + void createNotificationAndStartForeground(final VideoPlayerImpl player, final Service service) { + createNotificationIfNeededAndUpdate(player, true); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { service.startForeground(NOTIFICATION_ID, notificationBuilder.build(), ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK); @@ -160,13 +167,7 @@ public final class NotificationUtil { } } - - boolean hasSlotWithBuffering() { - return notificationSlots[1] == NotificationConstants.PLAY_PAUSE_BUFFERING - || notificationSlots[2] == NotificationConstants.PLAY_PAUSE_BUFFERING; - } - - void cancelNotification() { + void cancelNotificationAndStopForeground(final Service service) { try { if (notificationManager != null) { notificationManager.cancel(NOTIFICATION_ID); @@ -176,6 +177,8 @@ public final class NotificationUtil { } notificationManager = null; notificationBuilder = null; + + service.stopForeground(true); } diff --git a/app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java b/app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java index c1171efb7..e2e382bb2 100644 --- a/app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java +++ b/app/src/main/java/org/schabi/newpipe/player/VideoPlayerImpl.java @@ -575,7 +575,7 @@ public class VideoPlayerImpl extends VideoPlayer void onShuffleOrRepeatModeChanged() { updatePlaybackButtons(); updatePlayback(); - resetNotification(false); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @Override @@ -612,7 +612,7 @@ public class VideoPlayerImpl extends VideoPlayer titleTextView.setText(tag.getMetadata().getName()); channelTextView.setText(tag.getMetadata().getUploaderName()); - resetNotification(false); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); updateMetadata(); } @@ -1059,9 +1059,7 @@ public class VideoPlayerImpl extends VideoPlayer animatePlayButtons(false, 100); getRootView().setKeepScreenOn(false); - NotificationUtil.getInstance().createNotificationIfNeeded(this, false); - NotificationUtil.getInstance().updateNotification( - this); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @Override @@ -1077,7 +1075,7 @@ public class VideoPlayerImpl extends VideoPlayer isForwardPressed = false; isRewindPressed = false; } else { - NotificationUtil.getInstance().updateNotification(this); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } } } @@ -1097,7 +1095,7 @@ public class VideoPlayerImpl extends VideoPlayer checkLandscape(); getRootView().setKeepScreenOn(true); - resetNotification(false); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @Override @@ -1113,12 +1111,12 @@ public class VideoPlayerImpl extends VideoPlayer updateWindowFlags(IDLE_WINDOW_FLAGS); - resetNotification(false); - // Remove running notification when user don't want music (or video in popup) // to be played in background if (!minimizeOnPopupEnabled() && !backgroundPlaybackEnabled() && videoPlayerSelected()) { - service.stopForeground(true); + NotificationUtil.getInstance().cancelNotificationAndStopForeground(service); + } else { + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } getRootView().setKeepScreenOn(false); @@ -1130,9 +1128,7 @@ public class VideoPlayerImpl extends VideoPlayer animatePlayButtons(false, 100); getRootView().setKeepScreenOn(true); - NotificationUtil.getInstance().createNotificationIfNeeded(this, false); - NotificationUtil.getInstance().updateNotification( - this); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @@ -1146,9 +1142,7 @@ public class VideoPlayerImpl extends VideoPlayer getRootView().setKeepScreenOn(false); updateWindowFlags(IDLE_WINDOW_FLAGS); - NotificationUtil.getInstance().createNotificationIfNeeded(this, false); - NotificationUtil.getInstance().updateNotification(this); - + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); super.onCompleted(); } @@ -1239,7 +1233,7 @@ public class VideoPlayerImpl extends VideoPlayer onShuffleClicked(); break; case ACTION_RECREATE_NOTIFICATION: - resetNotification(true); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, true); break; case Intent.ACTION_HEADSET_PLUG: //FIXME /*notificationManager.cancel(NOTIFICATION_ID); @@ -1309,19 +1303,12 @@ public class VideoPlayerImpl extends VideoPlayer // Thumbnail Loading //////////////////////////////////////////////////////////////////////////*/ - void resetNotification(final boolean recreate) { - NotificationUtil.getInstance().createNotificationIfNeeded(this, recreate); - NotificationUtil.getInstance().updateNotification(this); - } - @Override public void onLoadingComplete(final String imageUri, final View view, final Bitmap loadedImage) { - // rebuild OLD notification here since remote view does not release bitmaps, - // causing memory leaks super.onLoadingComplete(imageUri, view, loadedImage); - resetNotification(true); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @Override @@ -1329,13 +1316,13 @@ public class VideoPlayerImpl extends VideoPlayer final View view, final FailReason failReason) { super.onLoadingFailed(imageUri, view, failReason); - resetNotification(true); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } @Override public void onLoadingCancelled(final String imageUri, final View view) { super.onLoadingCancelled(imageUri, view); - resetNotification(true); + NotificationUtil.getInstance().createNotificationIfNeededAndUpdate(this, false); } /*//////////////////////////////////////////////////////////////////////////