The wiring checks moved from source text to the compiled service in
c62f19548, and they see calls but not values. That let three rewrites of
the onUpdateNotification override pass that the earlier text checks
caught: passing true instead of Media3's answer, and computing
foregroundRequired but handing Media3 the original answer (which would
bring back the background stop this branch fixes).
The test now also reads the override's instructions: Media3's answer is
loaded once, goes straight into AutoplayHandoff.foregroundRequired, and
what MediaSessionService.onUpdateNotification receives is that result
(directly or through a local), not a constant.
Measured in review on 2cac33588 plus this change: testDebugUnitTest 66/66;
the harmless rewrites (a local val, autoplay.let { }) still pass; the
rewrites above now fail. Still not covered: the release callback's value
and an extra condition on onDestroy's cancel.
Two claims in AutoplayHandoff's header, from the audit (F9).
Correction to e7ba6bcc3: its message and this header said that Media3 only
logs a refused foreground start. That holds on the path where Media3 asks
the service about its notification: MediaSessionService
.onUpdateNotificationInternal catches the refusal and logs "Failed to start
foreground". The path that applies a notification once its artwork has
loaded (MediaNotificationManager.onNotificationUpdated) starts the
foreground without that catch. Both read in the Media3 1.4.1 source.
The header also said it was not measured whether Android accepts a repeated
start request from a service already in the foreground. Android's
ActiveServices answers that by reading (android-16.0.0_r1; an Android 14
copy has the same recompute rule), and the header now says what it reads:
the allowance is worked out again only while it is denied, and
ActiveServices clears it when the service leaves the foreground or is torn
down, on a first start long after a plain start, and on a repeated start of
a short or time-limited service type, which mediaPlayback is not. Read, not
measured on a device; GrapheneOS NOT MEASURED.
Comments only; the JVM tests pass on this commit.
The audit (F4) read that with the app in the background the SponsorBlock
skip loop keeps the first video's segments: it gets its item through
collectAsStateWithLifecycle, which stops following NowPlaying while the
activity is stopped, and by the audit's check of the compiled dispatcher
its delay loop goes on running. Each later video (autoplay, or the queue)
would then be skipped to the ends of the first one's segments, and a
segment ending within half a second of a later video's end would end it
and start the next autoplay. Before e7ba6bcc3 the background chain
reportedly stopped after one video; with it the chain can run on, which
makes this reachable.
The decision moves to segmentToSkip (SponsorBlockSkip.kt), which reads
NowPlaying at the moment of each decision and applies segments only while
it names the video they belong to. NowPlaying moves with the player: the
service claims it for the next video before handing that video over, and
for a queue item when the player moves to it. With the app in front the
composition still restarts the loop for a new item; the check also covers
the moment between NowPlaying moving and that restart.
Measured: 66 of 66 JVM tests pass on this commit's tree. Removing the
NowPlaying check, which gives the behaviour before this commit, fails
SponsorBlockSkipTest.aVideosSegmentsAreNotSkippedOnTheNextOne. On a device
NOT MEASURED, including whether the loop runs while the activity is
stopped.
The audit (F6) read that the 30 second bound on an autoplay attempt is
shorter than strawcore's own limits. An attempt makes at most two
extraction calls, and strawcore abandons each one at 60 seconds
(EXTRACT_TIMEOUT in rust/strawcore/src/runtime.rs), so on a slow link the
bound could cancel a lookup that before e7ba6bcc3, which added the bound,
ran to its end and played the next video, as it still can with the app on
screen.
It asked whether the bound should only release the hold and let the lookup
finish. It does not here: a lookup that finishes after the hold is
released hands the next video to a service that has left the foreground,
which in the background is the refused foreground start that e7ba6bcc3
exists to avoid (read; not reproduced on a device). The bound moves to 125
seconds instead, past the 120 seconds two extractions can take, and stays a
guard against an attempt stuck outside strawcore. The 5 seconds over 120
are a chosen margin, not a measured time.
Cost: an attempt stuck outside strawcore holds the foreground, with the
ended video's notification, for up to 125 seconds instead of 30. The audit
measured the two lookups at 0.66 to 0.93 seconds on a wired link; on a
mobile link they are NOT MEASURED.
Measured: the JVM tests pass on this commit; the service's attempt is still
in flight at 124,999 ms of virtual time and gives up at 125,000 ms.
The audit (F8) read that the staleness fence on the autoplay hand-over
compares URLs only. A play request on the ended video (from the
notification or the minibar) replays it without moving NowPlaying, so a
lookup that finished afterwards replaced the replay with the next video,
and setPlayingFrom set play-when-ready even if the user had paused since
(read in the Media3 and straw sources; not reproduced on a device).
The hand-over now also needs the player to be still ended with
play-when-ready set, the state it is in when a video ends with nobody
touching it. A play request from a controller replays an ended video
(Media3 1.4.1 handles it with Util.handlePlayButtonAction, which seeks to
the start and plays), and a pause or a stop clears one of the two; in each
case the user's action stands, nothing is handed over, and Media3 is asked
again whether the service stays in the foreground, as for any attempt that
ends without a hand-over. ExoPlayer leaves play-when-ready set at the end of
a video unless it was told to pause at the end of each item, which straw
does not do (read in the ExoPlayer 1.4.1 source).
The attempt is not cancelled when the user acts: its lookup runs on until it
ends, holding the foreground until then. Cancelling it on a user action
needs a way to tell the user's commands from the player events the
hand-over itself causes (a session callback on controller commands, for
example); that is a new mechanism, left out of this fix round.
Measured: the JVM tests pass on this commit; the new test covers a replay
(READY and BUFFERING), a replay then pause, a pause while ended and a stop.
Device behaviour NOT MEASURED.
The audit of b0a5e7213 found the playback service's autoplay wiring guarded
by matches on its source text. A comment, a dead branch or "|| true" passed
them and a harmless rewrite failed them, and nothing tested how the service
builds and drives its handoff: the 30 second bound, the main-thread
hand-over, the end of a video starting an attempt, the staleness fence, the
allowlist gate.
What the service decides about autoplay moves, unchanged in behaviour, to
ServiceAutoplay.kt: how it builds its AutoplayHandoff (hand-overs on
Dispatchers.Main, the 30 second bound, a release that asks Media3 again with
false), which player state starts an attempt, the allowlist gate on the
next video, and the fence on the hand-over with the check that the player
took the video. ServiceAutoplayTest runs them with Dispatchers.Main replaced
by a test dispatcher on virtual time (kotlinx-coroutines-test, a new
test-only dependency).
What stays in PlaybackService is its connection to Media3: the
onUpdateNotification override, building the handoff, attaching the
end-of-video listener, tryAutoplay, and the cancel in onDestroy.
AutoplayWiringTest now reads those from the compiled class instead of the
source text (CompiledReach: what a method calls, following the class's own
helpers, lambdas and nested classes). It sees calls, not values.
AutoplayHandoffTest now dispatches the way the main thread does (a launched
attempt runs later, never inside start) on virtual time, and adds: the
release asks Media3 after the hold is dropped; a failed lookup or hand-over
does not reach the scope's exception handler; an attempt gives up at its
bound and not a millisecond before. One test keeps Dispatchers.Unconfined on
purpose, for a dispatcher that resumes the replaced attempt inside
job.cancel().
Correction to b0a5e7213: its message says "The service code cannot run in a
JVM test, so this checks the source". The service class cannot run there,
but the decisions in it can once they are taken out of it, which is what
this commit does; and the source check that commit added was passed by a
comment or a dead branch, as the audit measured.
Measured in throwaway build containers (mutants one at a time, the test JVM
under ulimit -v 8388608, MALLOC_ARENA_MAX=2, timeout 900). At 737eb50 the
shipped suite (46 tests) let the audit's mutants H06, H11, H15, H28, S03,
S04, S05, S08, S09, S10, S11, S12, S13 and S14 survive and failed on its
harmless rewrites F01, F02 and F03. On this tree: 62 of 62 JVM tests pass;
the service mutants passing Media3's answer straight through, building the
release with no session or a no-op update, commenting out or removing the
onDestroy cancel, and wrapping it in "if (false)" all fail
AutoplayWiringTest; commenting out the update argument does not compile.
The full mutant run is at the head of this round.
The owner retired the log service the app sent to, so the "Send logs"
button in Settings and the send from the crash handler go, with
everything that existed only for them: LogShipper, the crash-handler
install in StrawApp, the build-time token (its read in
strawApp/build.gradle.kts, the BuildConfig field and the CI env line),
LogDump.captureScrubbedLinesBlocking and the second, lighter scrub
profile that only the shipper used.
Kept, because other code uses it: LogDump.capture and scrubLine (the
"Export logs" share sheet and the on-screen error strings), strawLogI,
and BuildConfig itself. LogDumpScrubTest now covers the one profile
left (still 10 tests).
Measured by the builder with this same patch applied to e7ba6bcc3 (the
parent of the test-only commit b0a5e7213), not yet on this commit's
parent: 46 of 46 unit tests pass; an old-versus-new
comparison of the scrubber over 48,212 log lines found 0 differences;
assembleDebug passes with the token unset and with a placeholder set;
a search of the built APK finds no trace of the log host, the ingest
URL, the token or the button text, where a control build of the parent
commit has them.
A mutant that made the hand-over always answer yes survived the suite:
setPlayingFrom returns early when it cannot build an item, and a yes then
releases the hold without asking Media3 to look again. The service code
cannot run in a JVM test, so this checks the source, as the other wiring
checks do.
Autoplay starts when the last item ends and then looks up the next video
over the network. Media3 1.4.1 takes the playback service out of the
foreground as soon as the player ends (its rule is play-when-ready and
READY or BUFFERING), so the next video was handed to a service that had
already left the foreground. Android 12 and later do not let an app in
the background start a foreground service (the documented platform
restriction; not reproduced on a device here), and Media3 only logs that
refusal, so with the screen off or another app in front the next video
played from a service that was not in the foreground. The report is that
it then stops a few seconds in; how the system stops it was not measured
on a device.
The service now answers Media3's foreground question through
AutoplayHandoff: while an attempt is in flight it keeps the foreground,
and it lets go when the next video is handed over, or when the attempt
finds nothing, fails, is refused by the player or runs past 30 seconds,
in which case it asks Media3 to look again straight away. One attempt
runs at a time; a newer one replaces an older one without losing the
hold, and the service drops any attempt when it is destroyed.
The lookup and hand-over themselves are unchanged: same candidate pick,
same allowlist gate, same staleness fence, same setPlayingFrom.
