Try better approach to ensuring webview service worker is up to date#312020
Try better approach to ensuring webview service worker is up to date#312020mjbvz merged 2 commits intomicrosoft:mainfrom
Conversation
This should fix a case where the very first reload after a service worker update used the old service worker
There was a problem hiding this comment.
Pull request overview
This PR updates the webview preload service worker registration flow to more reliably ensure the latest service worker is active after updates, addressing cases where a reload could still be served by an older worker.
Changes:
- Simplifies service worker “version” negotiation by removing the explicit version message handshake and instead forcing an explicit
registration.update()when an older controller is present. - Adjusts caching behavior to avoid caching ranged responses and to mark partial (206) responses as
no-store. - Removes the message-channel-based routing used to forward worker/sharedworker-originated resource/localhost loads to the host.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/webview/browser/pre/service-worker.js | Removes version/message-channel logic and changes caching + range handling in fetch processing. |
| src/vs/workbench/contrib/webview/browser/pre/index.html | Updates SW registration strategy (updateViaCache + explicit update + controllerchange waiting) and updates CSP hash accordingly. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/webview/browser/pre/service-worker.js:452
- With the removal of the
client.type === 'worker' || 'sharedworker'fallback branch, worker-originated fetches that don't resolve awebviewIdno longer send anyload-resourcemessage to the host, so the request will always time out (30s) and return 408. Please reintroduce a routing mechanism for these worker clients (e.g. the previous outer-iframe message channel) or otherwise ensure worker requests can be associated with the correct webview.
range,
});
}
}
return promise.then(entry => resolveResourceEntry(entry, cached));
src/vs/workbench/contrib/webview/browser/pre/service-worker.js:516
- Similar to
processResourceRequest: if this request comes from a'worker'/'sharedworker'client whoseclient.urldoes not include?id=...,webviewIdwill be falsy but this code still creates alocalhostRequestStoreentry and (now) sends no message to any outer iframe client. That guarantees a 30s delay before falling back tofetch(event.request)inresolveRedirect. Either add a worker-safe routing fallback here or fail fast to avoid the timeout path.
id: requestId,
});
}
}
return promise.then(resolveRedirect);
- Files reviewed: 2/2 changed files
- Comments generated: 2
| // Refs https://github.com/microsoft/vscode/issues/244143 | ||
| // With PlzDedicatedWorker, worker subresources and blob wokers | ||
| // With PlzDedicatedWorker, worker subresources and blob workers | ||
| // will use clients different from the window client. | ||
| // Since we cannot different a worker main resource from a worker subresource | ||
| // we will use message channel to the outer iframe provided at the time | ||
| // of service worker controller version initialization. | ||
| if (!webviewId && client.type !== 'worker' && client.type !== 'sharedworker') { | ||
| console.error('Could not resolve webview id'); |
There was a problem hiding this comment.
This code path explicitly allows 'worker'/'sharedworker' clients to have no webviewId, but there is no longer any fallback routing for those requests. For worker clients where client.url does not include ?id=... (e.g. blob workers / worker subresources in #244143), requests will later time out because no load-resource message is ever sent.
This issue also appears in the following locations of the same file:
- line 447
- line 511
This should simplify the code and fix a case where the very first reload after a service worker update used the old service worker