Add legacy service preservation and feature flag toggle infrastructure by sztomek · Pull Request #5145 · Automattic/pocket-casts-android

sztomek · 2026-03-18T10:35:49Z

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 the MEDIA3_SESSION feature 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

If this is a user-facing change, I have added an entry in CHANGELOG.md
Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
I have considered whether it makes sense to add tests for my changes
All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
Any jetpack compose components I added or changed are covered by compose previews
I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

dangermattic · 2026-03-18T10:36:36Z

	2 Warnings
⚠️	Class `LegacyPlaybackService` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `MediaControllerCallback` is missing tests, but `unit-tests-exemption` label was set to ignore this.

Generated by 🚫 Danger

Copilot

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 LegacyPlaybackService and added an automotive-specific LegacyAutoPlaybackService subclass.
Added PlaybackServiceToggle to enable/disable the appropriate service components based on the MEDIA3_SESSION feature 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.

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackServiceToggle.kt

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt

proguard-rules.pro

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/PlaybackServiceToggle.kt

automotive/src/main/java/au/com/shiftyjelly/pocketcasts/LegacyAutoPlaybackService.kt

sztomek · 2026-03-27T16:04:16Z

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt

+            }
+
+            Timber.d("Playback Notification State Change $state")
+            when (state) {


Only the relevant PlaybackStateCompat states are handled; the unhandled cases are intentional no-ops. This is the standard pattern used throughout the codebase.

wpmobilebot · 2026-03-30T10:36:40Z

Version 8.9 has now entered code-freeze, so the milestone of this PR has been updated to 8.10.

Copilot

Pull request overview

This PR is part of the Media3 migration stack, extracting the existing PlaybackService (MediaBrowserServiceCompat) behavior into a preserved LegacyPlaybackService and adding a small utility to toggle which service component is enabled at runtime via the MEDIA3_SESSION feature flag.

Changes:

Add LegacyPlaybackService (legacy MediaBrowserServiceCompat implementation) and an automotive-specific LegacyAutoPlaybackService subclass.
Introduce PlaybackServiceToggle to enable/disable service components based on Feature.MEDIA3_SESSION.
Add an R8/ProGuard keep rule for LegacyPlaybackService.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
proguard-rules.pro	Adds a keep rule for the legacy playback service class.
modules/services/repositories/.../PlaybackServiceToggle.kt	Adds feature-flag-driven component enable/disable utility for swapping service implementations.
modules/services/repositories/.../LegacyPlaybackService.kt	Introduces preserved legacy playback service logic (notifications, browsing, sleep timer).
automotive/.../LegacyAutoPlaybackService.kt	Adds an automotive-specific legacy service subclass mirroring the existing automotive service behavior.

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt

proguard-rules.pro

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-01T20:32:00Z

automotive/src/main/java/au/com/shiftyjelly/pocketcasts/LegacyAutoPlaybackService.kt

+    override fun onCreate() {
+        super.onCreate()
+        settings.setAutomotiveConnectedToMediaSession(false)
+
+        RefreshPodcastsTask.runNow(this, applicationScope)
+


LegacyPlaybackService.onCreate() can early-return after calling stopSelf() when mediaSession is null (flag mismatch). This subclass continues executing after super.onCreate() regardless, which can trigger settings writes and RefreshPodcastsTask.runNow() even though the service is shutting down. Add a guard (e.g., return early if mediaController/notificationManager wasn’t initialized or if playbackManager.mediaSessionManager.mediaSession is null) to avoid running automotive side effects in the mismatched/stopSelf path.

Add SupervisorJob to LegacyPlaybackService coroutine context and cancel it in onDestroy() so coroutines are properly cleaned up. Fix misleading proguard comment to match the actual rule behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The class was introduced in this branch but the manifest entry was deferred to 06d, causing lint to fail on branches 05 through 06c. Register it now with android:enabled="false" — PlaybackServiceToggle enables the correct service at runtime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-02T11:32:22Z

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt

+            return
+        }
+
+        if (sleepTimerDisposable == null || sleepTimerDisposable!!.isDisposed) {


sleepTimerDisposable!!.isDisposed introduces a forced unwrap on a nullable Disposable. This can be rewritten with safe calls to avoid potential NPEs and align with the codebase’s preference to avoid !! (e.g., check sleepTimerDisposable?.isDisposed != false).

Suggested change

if (sleepTimerDisposable == null || sleepTimerDisposable!!.isDisposed) {

if (sleepTimerDisposable?.isDisposed != false) {

Copilot · 2026-04-02T11:32:22Z

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt

+    override fun onDestroy() {
+        super.onDestroy()
+        isForeground = false
+
+        disposables.clear()
+        sleepTimerDisposable?.dispose()
+        job.cancel()
+


mediaController registers a MediaControllerCompat.Callback, but the service never unregisters it in onDestroy(). Because the callback is an inner class holding a reference to the service, this can keep the service instance reachable and/or deliver callbacks after teardown. Unregister the callback (and clear the controller reference) during onDestroy() (and consider unregistering the previous callback if mediaController is ever replaced).

Copilot · 2026-04-02T11:32:22Z

automotive/src/main/java/au/com/shiftyjelly/pocketcasts/LegacyAutoPlaybackService.kt

+class LegacyAutoPlaybackService : LegacyPlaybackService() {
+
+    @Inject @ApplicationScope
+    lateinit var applicationScope: CoroutineScope
+
+    override fun onCreate() {
+        super.onCreate()
+        settings.setAutomotiveConnectedToMediaSession(false)
+
+        RefreshPodcastsTask.runNow(this, applicationScope)
+
+        Timber.d("Legacy auto playback service created")
+    }


LegacyAutoPlaybackService doesn’t override onLoadChildren(). The existing AutoPlaybackService adds automotive-specific behavior there (IO dispatcher + podcastManager.refreshPodcastsIfRequired("Automotive")). If this legacy service is intended to be a drop-in replacement behind a feature flag, it likely needs the same override (and dependency injection) to avoid behavior differences/regressions when the legacy path is enabled.

wpmobilebot · 2026-04-02T11:32:56Z

Project manifest changes for automotive

The following changes in the automotive's merged AndroidManifest.xml file were detected (build variant: release):

--- ./build/reports/diff_manifest/automotive/release/base_manifest.txt	2026-04-02 11:32:52.675287290 +0000
+++ ./build/reports/diff_manifest/automotive/release/head_manifest.txt	2026-04-02 11:32:55.455295561 +0000
@@ -105,6 +105,16 @@
                 <action android:name="android.media.browse.MediaBrowserService" />
             </intent-filter>
         </service>
+        <service
+            android:name="au.com.shiftyjelly.pocketcasts.LegacyAutoPlaybackService"
+            android:enabled="false"
+            android:exported="true"
+            android:foregroundServiceType="mediaPlayback"
+            android:label="@string/app_name" >
+            <intent-filter>
+                <action android:name="android.media.browse.MediaBrowserService" />
+            </intent-filter>
+        </service>
 
         <!-- services which are periodic or trigerred by the app automatically triggered, eg: can run in the background -->
         <service

Go to https://buildkite.com/automattic/pocket-casts-android/builds/16227/canvas?sid=019d4df3-a54a-4491-b18a-b7b388a9af6f, click on the Artifacts tab and audit the files.

automotive/src/main/AndroidManifest.xml

            </intent-filter>
        </service>

+        <service


sztomek added this to the 8.10 milestone Mar 18, 2026

sztomek requested a review from a team as a code owner March 18, 2026 10:35

sztomek removed the request for review from a team March 18, 2026 10:35

sztomek added the do not merge label Mar 18, 2026

sztomek requested a review from MiSikora March 18, 2026 10:35

sztomek added the [Type] Enhancement Improve an existing feature. label Mar 18, 2026

Copilot AI review requested due to automatic review settings March 18, 2026 10:35

sztomek added the [Area] Playback Episode playback issue label Mar 18, 2026

Copilot started reviewing on behalf of sztomek March 18, 2026 10:36 View session

Copilot AI reviewed Mar 18, 2026

View reviewed changes

github-advanced-security bot found potential problems Mar 18, 2026

View reviewed changes

MiSikora approved these changes Mar 19, 2026

View reviewed changes

sztomek modified the milestones: 8.10, 8.9 Mar 20, 2026

sztomek force-pushed the media3/04-session-callbacks branch from d85dfab to 4841ed3 Compare March 27, 2026 16:03

sztomek force-pushed the media3/05-legacy-service branch from 7992866 to 08ef9b3 Compare March 27, 2026 16:04

wpmobilebot modified the milestones: 8.9, 8.10 Mar 30, 2026

sztomek force-pushed the media3/04-session-callbacks branch from 4841ed3 to 497b54c Compare March 31, 2026 17:21

Copilot AI review requested due to automatic review settings April 1, 2026 06:51

sztomek force-pushed the media3/05-legacy-service branch from 08ef9b3 to da8949c Compare April 1, 2026 06:51

Copilot started reviewing on behalf of sztomek April 1, 2026 06:51 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

.../src/main/java/au/com/shiftyjelly/pocketcasts/repositories/playback/LegacyPlaybackService.kt Show resolved Hide resolved

proguard-rules.pro Outdated Show resolved Hide resolved

sztomek force-pushed the media3/05-legacy-service branch from da8949c to bc1dd94 Compare April 1, 2026 17:55

sztomek force-pushed the media3/04-session-callbacks branch from f992988 to 376454a Compare April 1, 2026 20:26

Copilot AI review requested due to automatic review settings April 1, 2026 20:28

sztomek force-pushed the media3/05-legacy-service branch from bc1dd94 to 3327e0e Compare April 1, 2026 20:28

Copilot started reviewing on behalf of sztomek April 1, 2026 20:29 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

sztomek force-pushed the media3/04-session-callbacks branch from 376454a to 8eceb80 Compare April 2, 2026 10:56

sztomek and others added 3 commits April 2, 2026 12:56

Add legacy service preservation and feature flag toggle infrastructure

a1ea69e

Update proguard comment to mention both service classes

09906b3

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

sztomek force-pushed the media3/05-legacy-service branch from 3327e0e to 09906b3 Compare April 2, 2026 10:56

sztomek removed the do not merge label Apr 2, 2026

Copilot AI review requested due to automatic review settings April 2, 2026 11:28

Copilot started reviewing on behalf of sztomek April 2, 2026 11:29 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

github-advanced-security bot found potential problems Apr 2, 2026

View reviewed changes

automotive/src/main/AndroidManifest.xml

</intent-filter>

</service>

<service

Check warning

Code scanning / Android Lint

Exported service does not require permission Warning

Exported service does not require permission

sztomek added the unit-tests-exemption label Apr 2, 2026

	if (sleepTimerDisposable == null \|\| sleepTimerDisposable!!.isDisposed) {
	if (sleepTimerDisposable?.isDisposed != false) {

Conversation

sztomek commented Mar 18, 2026

Description

Testing Instructions

Checklist

Uh oh!

dangermattic commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

sztomek Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Apr 2, 2026

Project manifest changes for automotive

Uh oh!

Check warning

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dangermattic commented Mar 18, 2026 •

edited

Loading