Repository navigation
Close a content script's port when the worker hangs up, even while an extension page holds it - #1203
Merged
Merged
Conversation
… extension page holds it
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.
Suppose an extension's service worker deliberately calls
port.disconnect()on a content script's port while one of the extension's own pages is open in the default session. Today the content script'sonDisconnectfires only once that page closes. The proxy counts the open page as another receiving end of the port, and on the worker's hang-upendWorkerLegcloses only the worker's leg.Chrome closes the whole channel in this case.
disconnect()reachesMessageService::ClosePortImplwithclose_channel=true, which goes toCloseChannelImpl, and that notifies the opener (extensions/browser/api/messaging/message_service.cc). A context that goes away closes withclose_channel=falseinstead. So the refusal and worker-stop paths keependWorkerLeg, and only the deliberate hang-up now callscloseShimPort(port, { notifyWorker: false }).There is no changelog line. The page-leg fan-out (122cb76) is in no release, so no user has met this bug.
This also evens out CI's e2e shards (#1201, #1201). Five
extension-proxytests opened such a page first, and each waited out a 15 s probe deadline.Proof
New unit test
closes when the worker hangs up, though a page still holds it, run withbun test --isolate packages/electron-extensions/runtime-proxy.Before, with
main'sruntime-proxy.ts:After:
Then
op run --env-file .env.e2e.op -- bun run test:e2e tests/extension-proxy.e2e.tson macOS. All 19 tests passed both times, and the whole file went from 2.3 min to 1.1 min::363both accounts' popups reach the one worker:431every extension context has chrome and no browser:556a page navigating away disconnects the port:1009storage is one store:1100storage.onChanged fires in the worker and the shim sessionRisk
The change reaches only content-script ports that have page legs, which in the shipped catalog means Bitwarden. Ports with no page legs already took the
closeShimPort(notifyWorker: false)path. Revert by reverting this PR.