Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android docs: Clarify the usage of startForegroundService() #2479

Open
Levi-Lesches opened this issue Dec 2, 2024 · 9 comments · May be fixed by #2481
Open

Android docs: Clarify the usage of startForegroundService() #2479

Levi-Lesches opened this issue Dec 2, 2024 · 9 comments · May be fixed by #2481

Comments

@Levi-Lesches
Copy link
Contributor

Levi-Lesches commented Dec 2, 2024

The docs for startForegroundService() seems to be inaccurate in a few ways:

  1. The plugin does technically start a foreground service, but not in the normal way. A normal foreground service is associated with some work, like downloads, media playback, etc. In this plugin, the foreground service exits immediately after being created: its onStartCommand() returns after creating a notification, and its onBind() just returns null.

  2. The docs say to call the function multiple times to update the notification, but the standard way to update a foreground service notification is to call show() with the same ID (see Feature request: Adding a way to update sticky notification text without stopping and restarting that notification #2407)

  3. The docs claim to support any startType, but the plugin doesn't actually handle cases other than START_NOT_STICKY (see [flutter_local_notifications] Fix crashes related to Android foreground service start type #2475)

  4. The docs claim the notification will not be dismissible, but since API 33 (the plugin only supports 34+), foreground service notifications are dismissable by default.

  5. The docs for the function show a lot of Android-specific setup. They could instead link to the setup steps of the README (see also [flutter_local_notifications] Refactor the README into platform-specific guides #2477)

But most importantly, when combining points 1 and 3, I've come to realize that this function does not actually start a "real" foreground service. It shows the notification, yes, but the point of a foreground service is to promote some logic-heavy code to the foreground to prevent it from being background by the system. As far as I can tell, this plugin isn't doing that, and in my opinion, that would be out-of-scope and better left to a plugin like flutter_foreground_service or flutter_foreground_task.

It seems from this SO answer that even if a service does no work, the whole app is elevated to foreground status. This clarifies why the Java code isn't doing anything interesting, and why the Dart function doesn't take a callback to run, like the other plugins I linked do.

@EPNW-Eric, what are your thoughts? If you want to add these to your PR, that would be great, otherwise I can do them after yours is merged.

@Levi-Lesches
Copy link
Contributor Author

Levi-Lesches commented Dec 2, 2024

I'd suggest the following text. Note that I'm currently working on revamping the README to make the setup and nuances clearer in #2477. This also assumes that startType has been removed in #2475


Promotes this app to a foreground service with the given notification.

Calling this function will start an empty foreground service that lets Android know your app is doing important work and while it may be moved to the background, it should not be killed under normal circumstances. It is still possible for the service to be killed in some circumstances, such as if the service has been running for a while and the device is running low on memory. The service is not stopped automatically -- you must call [stopForegroundService] to stop it.

If you just want a non-dismissible notification, use [show] with [AndroidNotificationDetails.ongoing] set to true. Starting a foreground service requires extra setup. Refer to the Android Setup docs for details. A foreground service's ID must not be zero.

The provided notification will be displayed to indicate to the user that this service is running, and will be removed when [stopForegroundService] is called. To update the notification contents, call [show] with the same ID after this function returns.

foregroundServiceType is a set of foreground service types relevant to this service. This set may be null but must not be empty, and any service types within must also have been registered in the <service> tag of your app's AndroidManifest.xml file (refer to the Android Setup docs for more details).

@EPNW-Eric
Copy link

Seeing how deeply you are into the research of the topic I think it might be better (and more consistent) if you do a PR addressing it, so I closed mine. Hope that is Ok with you 😅?

@Levi-Lesches
Copy link
Contributor Author

Sounds good 👍

@Levi-Lesches
Copy link
Contributor Author

@EPNW-Eric See #2481. Hopefully we can get this in for the next breaking change

@leaf-node
Copy link

The plugin does technically start a foreground service, but not in the normal way. A normal foreground service is associated with some work, like downloads, media playback, etc. In this plugin, the foreground service exits immediately after being created: its onStartCommand() returns after creating a notification, and its onBind() just returns null.

I'm using this feature for a background task. Basically I spawn an isolate, and then run the .startForegroundService() method to keep that background service active. Are you saying that normally a background service would be a separate process that does more work than simply marking the whole app (so all of its isolates) as being in the foreground?

The docs claim to support any startType, but the plugin doesn't actually handle cases other than START_NOT_STICKY (see #2475)

I'm using startType: AndroidServiceStartType.startRedeliverIntent in my app, and that is working fine for me. I'm concerned that START_NOT_STICKY would make the foreground service non-durable to system resources temporarily tightening and expanding again. For my app, it's important that the foreground service resumes if it is temporarily stopped for any reason.

But most importantly, when combining points 1 and 3, I've come to realize that this function does not actually start a "real" foreground service. It shows the notification, yes, but the point of a foreground service is to promote some logic-heavy code to the foreground to prevent it from being background by the system. As far as I can tell, this plugin isn't doing that, and in my opinion, that would be out-of-scope and better left to a plugin like flutter_foreground_service or flutter_foreground_task.

Part of my concern with other frameworks is that they sometimes require passing a callback which gets started in another isolate, which can interfere with access to some state, or be redundant to creating one's own background isolate, which I need for auto starting after a phone reboot. In a particular module I tried, it was only possible to communicate with the generated isolate using JSON message passing, so I couldn't use enums or pass complex data types. So there can be limitations in how modules are implemented.

I'll take a look at the alternative options you suggested. I do think it would be nice to keep the feature here, especially if it is so simple, and removing it would cause breaking changes for developers who are using it.

@Levi-Lesches
Copy link
Contributor Author

(I'll respond to your comment in the PR here as well, since they both have the same answer)

The important bit about Flutter here is that Flutter doesn't make a native Dart app. It makes a native (insert-language-here) app that then invokes the Flutter engine. So your app's entrypoint is not void main() in lib/main.dart, but rather some function in some file in one of the platform-specific folders. In the case of Android, the main entrypoint is declared in your AndroidManfiest.xml, specifically, <activity android:name=".MainActivity". Now, back to your questions:

Are you saying that normally a background service would be a separate process that does more work than simply marking the whole app (so all of its isolates) as being in the foreground?

You're correct, the entire app is promoted to the foreground because services don't start a new process. But specifically, a foreground service is given a function, YourService.onStartCommand(intent) that tells it what to do when the service starts. That becomes the new entrypoint of the service.

This is my main concern: As we noted, you're not passing a function to the service at all, so onStartCommand() doesn't do anything except simply show the notification. No actual logic is associated with the service itself, it's just an empty service that is used to promote the whole app.

(From the PR)

If a value has to be hard-coded, it should be the START_REDELIVER_INTENT option so foreground services will continue to be reliable on Android.

That's where things change. As far as I can tell, this plugin is still relying on Service.onStartCommand(intent) to be the entrypoint when a service is re-instated. The regular main() function will not run with special notification that a service has been stopped, and since onStartCommand() itself is not associated with any logic, your logic will not be restarted either -- but the notification will still show. That can be problematic for developers and we don't currently offer any way of properly handling this. Not saying we can't, but we don't at the moment.


I still need to thoroughly test this and see if my claims are 100% correct, but this seems to be the conclusion of my research. The Android docs do note that we can check why the app was last killed, and maybe expose that in Dart, or maybe we can add support for a function to be passed and tie that directly to the service. By the way,

In a particular module I tried, it was only possible to communicate with the generated isolate using JSON message passing, so I couldn't use enums or pass complex data types. So there can be limitations in how modules are implemented.

You can send more complex data types, but there are restrictions. But from my understanding, the whole point is that the isolate would necessarily not be guaranteed access to the main thread, since it can be called straight from the native Java code.

Again, I would need to test more to validate all of this, especially seeing what can be done when a service is restarted and how to tie that to some actual Dart code. But if it's true that the notification is completely independent from logic, I do think it's appropriate to remove these options as they would fundamentally offer no value, only the illusion of it.

@leaf-node
Copy link

Hey, thanks for the reply. I was recently working on some code that coincidentally touches upon some of the topics you're asking about and sharing. Perhaps seeing it will offer some insights or a starting point if you're not already familiar with how the Java side of things interacts with FlutterEngine and DartExecutor. It was all very new to me, but take a look and let me know if any of it helps. : )

@leaf-node
Copy link

leaf-node commented Dec 4, 2024

Someone testing my app pointed out that despite swiping my app away in the task list, the foreground notification remained, but the background process seemed to stop delivering other notifications, which supports your theory. On the other hand, if I force stop my app, the foreground notification is removed, but that might be due to an extra cleanup process by Android.

Edit: Roughly 30 minutes after swiping the app away on the task list, the isolate / app is still running.

@Levi-Lesches
Copy link
Contributor Author

Thanks for taking the time to document everything fully, it's very interesting and I'll probably have to refer to it later.

After thinking over it for a while, I don't think this package should be handling real logic code in a foreground task. It's just out of scope, adds complexity, and only provides functionality for Android when iOS and possibly other platforms support it too. In my opinion, that's a worse developer experience compared to directing users to another package, like flutter_foreground_service or flutter_foreground_task that are better equipped to handle these use cases and can afford more maintenance on these features.

@MaikuB, @EPNW-Eric, do you agree? I think it's still okay to provide the foreground service for the sake of having one more type of notification. As of Android 13+ the notification can be easily dismissed, which does kind of defeat the purpose of this "special notification", but at least users can ask for special foreground privileges on Android, without the complexity of using isolates.

Anyway, I still need to work out a test to validate this 100%. A very simple test would be:

  • change the Java function to return START_REDELIVER_INTENT temporarily
  • Set the Dart code to write to a randomly named file on launch
  • Set the Java foreground service to write to a foreground-service.txt file in onStartCommand()
  • Start the app, start the foreground service, then kill it with adb, and let the foreground service restart
  • Check the contents of the directory. The Java file should be there and is mostly a sanity check, but
    • 1 file means the Flutter app did not restart (most likely)
    • 2 files means the Flutter app restarts without showing a UI (very unlikely, but interesting)

Again, my money is on case 1, in which case I will mark the PR as ready to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants