Add legacy service preservation and feature flag toggle infrastructure#5145
Open
sztomek wants to merge 1 commit intomedia3/04-session-callbacksfrom
Open
Add legacy service preservation and feature flag toggle infrastructure#5145sztomek wants to merge 1 commit intomedia3/04-session-callbacksfrom
sztomek wants to merge 1 commit intomedia3/04-session-callbacksfrom
Conversation
Collaborator
Generated by 🚫 Danger |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is part of the Media3 migration, preserving the existing MediaBrowserServiceCompat playback service implementation under new “legacy” service classes and adding a feature-flag-based toggle to switch between legacy and Media3 service components at runtime.
Changes:
- Extracted legacy playback service logic into
LegacyPlaybackServiceand added an automotive-specificLegacyAutoPlaybackServicesubclass. - Added
PlaybackServiceToggleto enable/disable the appropriate service components based on theMEDIA3_SESSIONfeature flag. - Added a ProGuard keep rule intended to retain the legacy service during minification.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| proguard-rules.pro | Adds a keep rule for the new legacy playback service class. |
| modules/services/repositories/.../PlaybackServiceToggle.kt | Introduces runtime component enable/disable logic based on a feature flag. |
| modules/services/repositories/.../LegacyPlaybackService.kt | New legacy MediaBrowserServiceCompat implementation extracted from existing playback service logic. |
| automotive/.../LegacyAutoPlaybackService.kt | Automotive-specific legacy service subclass with refresh/pause behavior. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+9
to
+19
| object PlaybackServiceToggle { | ||
|
|
||
| fun ensureCorrectServiceEnabled( | ||
| context: Context, | ||
| media3Services: List<String> = listOf(PlaybackService::class.java.name), | ||
| legacyServices: List<String> = listOf(LegacyPlaybackService::class.java.name), | ||
| ) { | ||
| val useMedia3 = FeatureFlag.isEnabled(Feature.MEDIA3_SESSION) | ||
| media3Services.forEach { setComponentEnabled(context, it, useMedia3) } | ||
| legacyServices.forEach { setComponentEnabled(context, it, !useMedia3) } | ||
| } |
Comment on lines
+277
to
+282
| val removeNotification = state != PlaybackStateCompat.STATE_PAUSED || settings.hideNotificationOnPause.value | ||
| if (removeNotification || isForegroundService) { | ||
| val isTransientLoss = playbackManager.playbackStateRelay.blockingFirst().transientLoss | ||
| if (isTransientLoss) { | ||
| return | ||
| } |
Comment on lines
+321
to
+330
| private fun observeSleepTimer() { | ||
| sleepTimer.stateFlow | ||
| .onEach { state -> | ||
| onSleepTimerStateChange(state) | ||
| } | ||
| .catch { throwable -> | ||
| Timber.e(throwable, "Error observing SleepTimer state") | ||
| } | ||
| .launchIn(this) | ||
| } |
| # https://github.com/shiftyjelly/pocketcasts-android/issues/1656 | ||
| # https://github.com/shiftyjelly/pocketcasts-android/pulls/2921 | ||
| -keep,allowobfuscation,allowshrinking class au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackService { *; } | ||
| -keep,allowobfuscation,allowshrinking class au.com.shiftyjelly.pocketcasts.repositories.playback.LegacyPlaybackService { *; } |
Comment on lines
+21
to
+36
| private fun setComponentEnabled(context: Context, className: String, enabled: Boolean) { | ||
| val componentName = ComponentName(context.packageName, className) | ||
| val desiredState = if (enabled) { | ||
| PackageManager.COMPONENT_ENABLED_STATE_ENABLED | ||
| } else { | ||
| PackageManager.COMPONENT_ENABLED_STATE_DISABLED | ||
| } | ||
|
|
||
| val currentState = context.packageManager.getComponentEnabledSetting(componentName) | ||
| if (currentState != desiredState) { | ||
| context.packageManager.setComponentEnabledSetting( | ||
| componentName, | ||
| desiredState, | ||
| PackageManager.DONT_KILL_APP, | ||
| ) | ||
| } |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import timber.log.Timber | ||
|
|
||
| @AndroidEntryPoint | ||
| class LegacyAutoPlaybackService : LegacyPlaybackService() { |
Check failure
Code scanning / Android Lint
Class is not registered in the manifest Error
| } | ||
|
|
||
| Timber.d("Playback Notification State Change $state") | ||
| when (state) { |
Check warning
Code scanning / Android Lint
Missing @IntDef in Switch Warning
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is the 5th step of the media3 migration. This PR preserves the existing PlaybackService logic by extracting it into
LegacyPlaybackService(a MediaBrowserServiceCompat) and its thin LegacyAutoPlaybackService subclass, so the old service can continue to run alongside the new Media3 implementation. A PlaybackServiceToggle utility was added to resolve the correct service class based on theMEDIA3_SESSIONfeature flag, enabling a safe runtime switch between legacy and Media3 paths.A ProGuard keep rule was also added to ensure the legacy service class isn't stripped during minification.
Testing Instructions
Just review the code since the new components are not wired up yet
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xml