Enforce using foreground service during playback#5563
Conversation
sztomek
added
[Type] Enhancement
|
Claude finished @sztomek's task in 4m 25s —— View job PR Review: Enforce using foreground service during playbackSolid, well-scoped change — the suspend gate + shared 🔴 Correctness
🟡 Worth verifying / polish
🟢 Minor notes
✅ Looks good
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Android 17: reach the foreground service before requesting audio focus, otherwise focus is denied. | ||
| if (FeatureFlag.isEnabled(Feature.FOREGROUND_BEFORE_PLAYBACK) && !Util.isAutomotive(application) && player?.isRemote != true) { | ||
| val foreground = mediaSessionManager.ensureForegroundServiceStarted(application) | ||
| if (!foreground) { | ||
| LogBuffer.e(LogBuffer.TAG_PLAYBACK, "Foreground service not ready before playback, proceeding with fallback") | ||
| eventHorizon.track(PlaybackForegroundServiceErrorEvent) | ||
| } | ||
| } |
There was a problem hiding this comment.
Foreground promotion (and isSwitchingPlayer) isn't rolled back when play() returns early right after this.
By the time we get here resetPlayer() has already set isSwitchingPlayer = true, and ensureForegroundServiceStarted() has promoted the service to the foreground with the "Loading" notification. But immediately below, if tryToGetAudioFocus() returns false, play() returns and nothing undoes either side effect:
- the service stays foreground showing a stale "Loading" notification with no audio, and nothing demotes it (there is no
stop()on focus denial here —stopAsync(isAudioFocusFailed = true)is only called from the focus-loss listener); isSwitchingPlayerstaystrue, so the new!isSwitchingPlayergates inPlaybackService.onUpdateNotificationandLegacyPlaybackService.handlePlaybackStatewill never remove the notification on pause afterwards → the FGS leaks.
Focus can still be denied even after going foreground (another app refusing to yield), so this is reachable. Consider resetting isSwitchingPlayer and demoting the early promotion on the focus-denied return path.
| private fun startForegroundEarly() { | ||
| if (Util.isAutomotive(this)) return | ||
| val session = playbackManager.mediaSessionManager.getMedia3Session() | ||
| try { | ||
| val title = playbackManager.getCurrentEpisode()?.title ?: getString(LR.string.loading) | ||
| val builder = NotificationCompat.Builder(this, Settings.NotificationChannel.NOTIFICATION_CHANNEL_ID_PLAYBACK.id) | ||
| .setContentTitle(title) | ||
| .setSmallIcon(IR.drawable.notification) | ||
| .setOnlyAlertOnce(true) | ||
| .setShowWhen(false) | ||
| .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) | ||
| if (session != null) { | ||
| builder.setStyle(MediaStyleNotificationHelper.MediaStyle(session)) | ||
| } | ||
| ServiceCompat.startForeground(this, Settings.NotificationId.PLAYING.value, builder.build(), ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK) | ||
| playbackManager.mediaSessionManager.setServiceForeground(true) | ||
| LogBuffer.i(LogBuffer.TAG_PLAYBACK, "startForeground early (preparing, media3)") | ||
| } catch (e: Exception) { | ||
| LogBuffer.e(LogBuffer.TAG_PLAYBACK, "startForeground early failed (media3): $e") | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && e is ForegroundServiceStartNotAllowedException) { | ||
| val currentValue = settings.getTimesToShowBatteryWarning() | ||
| settings.setTimesToShowBatteryWarning(2 + currentValue) | ||
| eventHorizon.track(PlaybackForegroundServiceErrorEvent) | ||
| } | ||
| if (session?.connectedControllers.isNullOrEmpty()) { | ||
| stopSelf() | ||
| } |
There was a problem hiding this comment.
Calling ServiceCompat.startForeground(...) yourself inside a Media3 MediaLibraryService is worth double-checking. Media3 manages the service's foreground state internally (via onUpdateNotification / the notification provider). Manually promoting to foreground out-of-band can desync Media3's internal foreground bookkeeping, which historically leads to symptoms like the service not being demoted/stopped when playback ends, or a duplicate/overwritten notification. It looks intentional here to satisfy the startForegroundService() 5s contract, but please confirm on-device that (a) the notification built here (plain NotificationCompat.Builder) is cleanly replaced by the Media3 DefaultMediaNotificationProvider notification once playback starts, and (b) the service still stops correctly at end-of-playback.
Also, this method plus LegacyPlaybackService.startForegroundEarly() are near-identical (same notification shape, same ForegroundServiceStartNotAllowedException → setTimesToShowBatteryWarning(2 + …) + PlaybackForegroundServiceErrorEvent handling). Consider extracting the shared catch/battery-warning logic to avoid the two copies drifting.
| settings.setTimesToShowBatteryWarning(2 + currentValue) | ||
| eventHorizon.track(com.automattic.eventhorizon.PlaybackForegroundServiceErrorEvent) | ||
| } | ||
| stopSelf() |
There was a problem hiding this comment.
Minor inconsistency with the Media3 path: here stopSelf() is called unconditionally on failure, whereas PlaybackService.startForegroundEarly() only calls stopSelf() when session?.connectedControllers.isNullOrEmpty(). If a Bluetooth/Auto controller is already connected and driving playback when this early-start fails, an unconditional stopSelf() could tear the session down. Consider guarding this the same way the Media3 path does (only stop when there's nothing actively connected/playing).
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| val mediaSession = playbackManager.mediaSessionManager.mediaSession ?: return | ||
| try { | ||
| val notification = notificationDrawer.buildPreparingNotification(mediaSession.sessionToken) | ||
| ServiceCompat.startForeground(this, Settings.NotificationId.PLAYING.value, notification, ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK) |
| if (session != null) { | ||
| builder.setStyle(MediaStyleNotificationHelper.MediaStyle(session)) | ||
| } | ||
| ServiceCompat.startForeground(this, Settings.NotificationId.PLAYING.value, builder.build(), ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK) |
|
Version |
3 participants
Description
It's been reported that our app is not compatible with upcomfing Android 17-related audio playback restrictions as there were some scenarios when we didn't ensure that our playback service runs in the foreground. A couple of such scenarios:
This PR attempts to fix those scenarios. I have added the feature flag
FOREGROUND_BEFORE_PLAYBACKand put my changes behind it. Previously we requested focus first and only promoted the service reactively, so background plays (Bluetooth, widgets, notifications, auto-play, Auto/Wear) got their focus request denied and were silently stopped. The core change is a suspend gate (ensureForegroundServiceStarted) called at the top ofPlaybackManager.play()that starts the service via startForegroundService and waits on a sharedisServiceForegroundsignal both services update, so the FGS is guaranteed live before focus regardless of feature-flag path (legacy or Media3) or entry point. It also keeps the service foreground across auto-play episode switches so a background advance never needs a fresh start exemption, and fails open — if the service genuinely can't start it falls back to the old behavior rather than blocking playback.See: p1783932483344609-slack-C028JAG44VD
Testing Instructions
You'll need to spin up an emulator with Android 17.
adb shell cmd audio set-enable-hardening enable.Background play scenarios (the ones that used to break)
5. Lock the screen, then hit play on a Bluetooth headset → audio should actually start.
6. Background the app, tap play on a home-screen widget → audio starts, notification shows.
7. Trigger a new-episode notification, background the app, tap "Play" on it → audio starts.
8. Play something, screen off, let the episode finish → next episode auto-plays without dropping.
9. From Android Auto (or adb shell am a play-from-search), say/pick something to play → it plays.
10. After each: check adb dumpsys audio and logcat show no AudioHardening suppression for our package.
Same thing on the legacy path
11. Turn
MEDIA3_SESSIONoff, force-stop the app, then repeat steps 5–10 → same results.Regression / normal behavior
12. Normal foreground play/pause/skip still works, no weird "Loading" notification lingering.
13. Pause, swipe the notification away (if hide-on-pause off), then resume from the widget → still plays.
14. Get a transient interruption (nav prompt or a phone call) → playback ducks/pauses then resumes fine.
15. Cast to a Chromecast, play, then disconnect → local playback resumes normally.
16. Flip the flag off, repeat a couple of plays → behaves exactly like before (sanity that it's inert).
Teardown
17. Reset enforcement:
adb shell cmd audio set-enable-hardening disable.Screenshots or Screencast
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...