Separate notification displaying from the service lifecycle

[nt4f04uNd]

Is your feature request related to a problem? Please describe.
Currently notification can be created if playing == true and processingState == idle.
But also currently the service will be stopped if playing == true and processingState == idle, removing the notification as well.

This makes it inconsistent and therefore hard to understand from the point of the user of this API, especially, because it's not documented.

Describe the solution you'd like
What we actually would like is to let processingState to manage the lifecycle of the service and playing and other parameters to update the look of the notification, because notification can exist without the service running, then also update the documentation accrodingly (#655)

Describe alternatives you've considered
No

Additional context
Extracted from discussion in #834

[nt4f04uNd]

Let's break down what use cases might exist, that might help to come up with what and how we should change

1. when create notification
   a. I want to create a notification without making the service foreground
   b. I want to start the playback and then create a notification for it (currently supported)
2. when stop service
   a. I want to stop the service, but keep the notification around and be able to update it. If service is destroyed (when there's no activity), pressing notification controls should start it again
   b. I want to stop the service and remove the notification (currently supported)
3. when service is not running (cases 1. a., 2. a.), I should be able to remove the notification

Now let's take a look of what set of properties in PlaybackState could support this, which seems to sufficiently cover all of them

1. a.

notificationCreated=null|false -> true, processingState=idle

1. b.

notificationCreated=any value (no-op), processingState=idle -> not idle

2. a.

notificationCreated=true, processingState=not idle -> idle

2. b.

notificationCreated=null|false, processingState=not idle -> idle

notificationCreated=true -> false, processingState=idle

What the new parameter should do

The notification is by default bound to processingState, i.e. it's created when processingState=not idle

• notificationCreated can be used to create and update fake notification (fake=when idle)
• notificationCreated=false is only for fake notification and should not do anything when processingState=not idle

Fixing edge cases and separating responsibilities of playing and processingState

The confusing part is that the service "starting" and stopping is spread along these two parameters. In quotes, because from the API user perspective it might look like service starts when notification is created, however in fact it's created as soon as any MediaBrowserCompat.ConnectionCallback connects. Here and further I mean by that the creation of the notification.

Let's separate responsibilieties of them and make processingState be a starter and stopper of the service, and all other parameters no-op (unless notificationCreated is true). That means playing doesn't do anything unless the processingState state is updated to not idle

• When it's not idle - playing still manages whether the service is foreground or not
• The service is stopped by idle

Other than a more clean API, that will as well ensure the user of it will pay close attention to both parameters, because now they have to run and stop the service with the processingState. Thus, it will exlude invalid states when processingState is always idle, but there's a notification that can't be removed when you stop, because it only happens when processingState changes from not idle to idle.

[ryanheise]

What do you mean by "fake" notification?

Just copying my comment from #849 :

I originally wanted to get rid of notificationCreated completely so that it would be possible to display the notification even before entering the foreground state. Maybe with !idle being used to determine whether updateNotification should show the notification rather than notificationCreated.

[nt4f04uNd]

What do you mean by "fake" notification?

By that I mean the notification created/updated when state is idle. That's the same notification, naming it like this was misleading, sorry.

Just copying my comment from #849 :

I think you didn't understand. This new notificationCreated parameter in PlaybackState has nothing to do with the variable we currently have on the native side. Please make sure to re-read what I propose it to do.

[ryanheise]

I didn't mean to imply that it was the same thing, in fact mine is quite a different approach but it seemed you wanted the discussion of my approach over here as well so that we can consider the different approaches together.

[nt4f04uNd]

Ok I see. It seems to me that with your approach 2. a. and 3 could not be accomplished, could they?

[ryanheise]

With 2a, I think I could support that via androidResumeOnClick = true. I think 3 should also be supported by setting the state to idle.

[nt4f04uNd]

I think 3 should also be supported by setting the state to idle.

How do you distinguish between 1a and 3? In both cases you have idle

With 2a, I think I could support that via androidResumeOnClick = true.

I mean, you could, but wouldn't notificationCreated make all of this more clear? It would have an imperative control over notification rather a vague collection of heuristic states and config parameters, that's hard to document and understand as a user.

[ryanheise]

I think 3 should also be supported by setting the state to idle.

How do you distinguish between 1a and 3? In both cases you have idle

Well I don't want to just map states to states. I would rather map state transitions. So inside the Android setState code it would always update the notification unless the state is idle. The moment the app decides it wants to show a notification, it should move out of the idle state. That won't in itself enter the foreground state, that will come later once the isActuallyPlaying condition becomes true.

With 2a, I think I could support that via androidResumeOnClick = true.

I mean, you could, but wouldn't notificationCreated make all of this more clear? It would have an imperative control over notification rather a vague collection of heuristic states and config parameters, that's hard to document and understand as a user.

I'm not necessarily against making androidResumeOnClick one of those ones we turn into a dynamic config option. At the moment there is too much static config in the config object, but I don't necessarily want to go too far in the other direction either and have too much dynamic config in the state object. So I'd first want to understand the use cases for it. In any case, the feature we're talking about is what the androidResumeOnClick option is for.

[nt4f04uNd]

I see it now. I don't like the fact that there's a moment where notification is shown but cannot be updated, i.e. when androidResumeOnClick=true, and we just became idle. Ideally, if we have a notification we should always be able to update it with mediaItem, for example.

Also, I don't like the name of androidResumeOnClick for what you want it to do, it looks like it was initially thought in mind to accomplish this https://developer.android.com/guide/topics/media-apps/mediabuttons#restarting-inactive-mediasessions, not keeping the notification around.

[ryanheise]

I see it now. I don't like the fact that there's a moment where notification is shown but cannot be updated, i.e. when androidResumeOnClick=true, and we just became idle. Ideally, if we have a notification we should always be able to update it with mediaItem, for example.

That's a fair point. Inside the Java code, the condition could be something like !idle || notificationCreated.

I'm also in favour of renaming androidResumeOnClick.