Repository navigation
Drop the service worker warm-up now that listeners register in time - #304
Merged
Merged
Conversation
document_start pinged the worker with { contentScriptQuery: 'warmup' } so
it would be awake by the time document_end connected. Nothing ever
handled that message - there is no listener for it anywhere - so it always
rejected and the rejection was swallowed. Booting the worker was the whole
point.
That was a workaround for listeners registering after an await, which is
fixed: every backend now registers during the worker's initial
synchronous run, so the connection that wakes it is routed rather than
dropped. The warm-up no longer has a job.
Measured against a worker left idle for 60s, twice, with the warm-up gone:
the theme still applies pre-paint, stored preview marks come back 252-253ms
after load, and a preview click writes through and persists. Both the read
and write paths survive a cold wake with nothing pre-booting the worker.
Refs #301
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #301
What this changes
Deletes
warmServiceWorker()fromdocument_start.ts, and the test that asserted it fired.document_startno longer communicates with the service worker at all - it is now purelycaptureUrlCartParam.Why it is safe to delete
The warm-up pinged the worker with
{ contentScriptQuery: 'warmup' }so it would be awake by thetime
document_endconnected. Nothing ever handled that message - there is no listener for itanywhere in the codebase - so it always rejected and the rejection was swallowed. Booting the
worker was its entire purpose.
It existed to paper over listeners registering after an
await: in MV3, a message or connectionthat wakes a sleeping worker is only routed to listeners registered during the worker's initial
synchronous run. That root cause was fixed in #303, which moved every backend's registration into
that synchronous run. With the cause gone, the warm-up has no job.
This is the second half of #301. The first half - the ordering fix itself - landed in #303 while
chasing an unrelated preview-history bug, so only the removal was left.
Verification
Measured against a worker left idle 60-65s with the warm-up removed, so nothing pre-booted it:
sendMessageBoth APIs were exercised deliberately.
sendMessagemattered most: #301 measured it rejectingoutright against a cold worker before the fix, and it is now answered on a cold wake, twice.
tsc --noEmitclean, 40 files / 776 tests pass, pre-commit hooks pass. The test count drops byone because the warm-up assertion was replaced with one asserting
document_startdoes not messagethe worker at all, guarding against the ping creeping back.
Not verified
findmusic_autologin.tsruns onfindmusic.club, has no warm-up of its own, and makes foursendMessagecalls - item 2 of #301. Message dispatch does not depend on the content script'sorigin, so the cold
sendMessageresults above cover the mechanism, but the autologin flow itselfneeds a login I could not drive and has not been run end to end. Worth one manual pass before
considering that item closed.