Re-use the existing foreground notification for FileUploader progress notifications#6997
Conversation
f6bb199 to
7717318
Compare
| // remove stucked notification | ||
| mNotificationManager.cancel(R.string.uploader_upload_in_progress_ticker); | ||
| } | ||
|
|
There was a problem hiding this comment.
This does not seem to be necessary anymore as we now cancel the notification when the service is destroyed.
Was this maybe a relic from before using a foreground service or did you still see that the service was killed without calling onDestroy()?
There was a problem hiding this comment.
I think this is to prevent edge cases like
- upload ongoing, notification is shown
- app is killed
- notification is still shown
At least I remember something like that. Maybe it is already better handled by newer Android versions…
There was a problem hiding this comment.
Probably safer to leave it in then. I re-added it back.
| } | ||
|
|
||
| mNotificationManager.cancel(R.string.uploader_upload_in_progress_ticker); | ||
|
|
There was a problem hiding this comment.
First, I was replacing the progress notification with the general foreground service notification here (mNotification), but that doesn't seem to be necessary as we either cancel it when the service gets destroyed or replace it with a new notification when starting the next upload.
7717318 to
715b214
Compare
notifications Signed-off-by: Torsten Grote <t@grobox.de>
715b214 to
6dc30d8
Compare
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/15937.apk |
|
master-light-blue-Screenshot test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/15937-Screenshot-master-light-blue |
|
stable-light-blue-Screenshot test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/15937-Screenshot-stable-light-blue |
Codacy393Lint
SpotBugs (new)
SpotBugs (master)
|
…ication Re-use the existing foreground notification for FileUploader progress notifications Signed-off-by: thelittlefireman <thelittlefireman@users.noreply.github.com>
4 participants
Closes #4998