Resolve webview id for worker clients in service worker#315495
Resolve webview id for worker clients in service worker#315495yongsooim wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the webview service worker where worker/sharedworker clients could pass the guard but fail routing due to lacking an id search param, causing fetches (notably dynamic imports) to hang until timeout.
Changes:
- Resolve a webview id for worker/sharedworker clients by mapping the worker URL origin to a single owning webview iframe client.
- Update
processResourceRequestandprocessLocalhostRequestto use this worker resolution and fail closed when the owner cannot be resolved. - Add
getWebviewIdForWorkerClienthelper to perform the origin-to-iframe-id resolution with ambiguity detection.
|
@microsoft-github-policy-service agree |
|
I have refactored the service worker to address the code duplication concern raised by the AI reviewer. By extracting the path-checking logic into a helper function isWebviewIframe(url), we keep the logic DRY and reduce the risk of drift if entry points change. @yongsooim, feel free to pull these changes if you find them helpful! |
|
I have opened a companion draft PR (#315498) that extracts the isWebviewIframe helper into a standalone commit, addressing the code duplication concern from the Copilot review. @yongsooim feel free to pull that commit directly — happy to close #315498 if you prefer to squash it into your branch instead. |
|
Thanks @harshagarwalnyu for taking a look and for opening the companion PR. I folded a corrected version of that helper extraction directly into this PR in 871021f, keeping the existing |
Fixes #315494. This appears to be a regression from #311844.
processResourceRequestandprocessLocalhostRequestallowworker/sharedworkerclients past the guard, but neither functionroutes them: the dispatch block runs only when
webviewIdis non-null,and a worker client's
client.url(typicallyblob:https://<webview-origin>/<uuid>) has noidsearchParam. Therequest store entry is never resolved and the fetch hangs until
requestTimeout()fires; in the renderer this surfaces asTypeError: Failed to fetch dynamically imported module.This patch resolves the owning iframe for a worker client by matching
the worker URL's origin against window clients whose pathname is one of
the webview entry points, then reading
idfrom that iframe'ssearchParams. Because some webviews can intentionally share an origin,
the match is only accepted when that origin maps to a single webview id.
The relaxed
worker / sharedworkerexception in the guard is replaced:a worker client whose owning iframe cannot be resolved unambiguously is
treated the same as a window client without a webview id (
notFound()for resource requests,
fetch(event.request)for localhost), preservingpre-#311844 behavior for that edge.
Validation
node --check src/vs/workbench/contrib/webview/browser/pre/service-worker.jsgit diff --checkload-resourcemessageload-resourceload-localhostI did not run the full VS Code browser test suite from this sparse checkout.