Skip to content

Commit 9f867c3

Browse files
authored
Revert service worker usage of MessageChannel (#140351)
* Revert service worker usage of MessageChannel Reverts 66b6adf While I'm not 100% about this, I think 66b6adf causes resourses to occasionally not load. I believe this can happen if the service worker is unitilized while the webview remains active. I can't reproduce this myself so it may be related to memory pressure or resource usage, however relying on the service worker not being reinitilized does seem like a potentially bad idea https://stackoverflow.com/questions/34775105/what-causes-the-global-context-of-a-service-worker-to-be-reset Will investigate if there's another way to achive this since using MessagePort did clean up the code and slightly improve performance * Bump webview commit versions
1 parent 942f56e commit 9f867c3

File tree

7 files changed

+180
-154
lines changed

7 files changed

+180
-154
lines changed

product.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"licenseFileName": "LICENSE.txt",
2626
"reportIssueUrl": "https://github.com/microsoft/vscode/issues/new",
2727
"urlProtocol": "code-oss",
28-
"webviewContentExternalBaseUrlTemplate": "https://{{uuid}}.vscode-webview.net/insider/69df0500a8963fc469161c038a14a39384d5a303/out/vs/workbench/contrib/webview/browser/pre/",
28+
"webviewContentExternalBaseUrlTemplate": "https://{{uuid}}.vscode-webview.net/insider/d372f9187401bd145a0a6e15ba369e2d82d02005/out/vs/workbench/contrib/webview/browser/pre/",
2929
"extensionAllowedProposedApi": [
3030
"ms-vscode.vscode-js-profile-flame",
3131
"ms-vscode.vscode-js-profile-table",

src/vs/workbench/contrib/webview/browser/pre/main.js

+50-21
Original file line numberDiff line numberDiff line change
@@ -206,27 +206,37 @@ const workerReady = new Promise((resolve, reject) => {
206206

207207
const swPath = `service-worker.js?v=${expectedWorkerVersion}&vscode-resource-base-authority=${searchParams.get('vscode-resource-base-authority')}`;
208208

209-
/**
210-
* @param {MessageEvent} event
211-
*/
212-
const swMessageHandler = async (event) => {
213-
if (event.data.channel !== 'init') {
214-
console.log('Unknown message received in webview from service worker');
215-
return;
216-
}
217-
navigator.serviceWorker.removeEventListener('message', swMessageHandler);
218-
219-
// Forward the port back to VS Code
220-
hostMessaging.onMessage('did-init-service-worker', () => resolve());
221-
hostMessaging.postMessage('init-service-worker', {}, event.ports);
222-
};
223-
navigator.serviceWorker.addEventListener('message', swMessageHandler);
224-
225209
navigator.serviceWorker.register(swPath)
226210
.then(() => navigator.serviceWorker.ready)
227-
.then(() => {
228-
const initServiceWorker = (/** @type {ServiceWorker} */ worker) => {
229-
worker.postMessage({ channel: 'init' });
211+
.then(async registration => {
212+
/**
213+
* @param {MessageEvent} event
214+
*/
215+
const versionHandler = async (event) => {
216+
if (event.data.channel !== 'version') {
217+
return;
218+
}
219+
220+
navigator.serviceWorker.removeEventListener('message', versionHandler);
221+
if (event.data.version === expectedWorkerVersion) {
222+
return resolve();
223+
} else {
224+
console.log(`Found unexpected service worker version. Found: ${event.data.version}. Expected: ${expectedWorkerVersion}`);
225+
console.log(`Attempting to reload service worker`);
226+
227+
// If we have the wrong version, try once (and only once) to unregister and re-register
228+
// Note that `.update` doesn't seem to work desktop electron at the moment so we use
229+
// `unregister` and `register` here.
230+
return registration.unregister()
231+
.then(() => navigator.serviceWorker.register(swPath))
232+
.then(() => navigator.serviceWorker.ready)
233+
.finally(() => { resolve(); });
234+
}
235+
};
236+
navigator.serviceWorker.addEventListener('message', versionHandler);
237+
238+
const postVersionMessage = (/** @type {ServiceWorker} */ controller) => {
239+
controller.postMessage({ channel: 'version' });
230240
};
231241

232242
// At this point, either the service worker is ready and
@@ -236,13 +246,13 @@ const workerReady = new Promise((resolve, reject) => {
236246
const currentController = navigator.serviceWorker.controller;
237247
if (currentController?.scriptURL.endsWith(swPath)) {
238248
// service worker already loaded & ready to receive messages
239-
initServiceWorker(currentController);
249+
postVersionMessage(currentController);
240250
} else {
241251
// either there's no controlling service worker, or it's an old one:
242252
// wait for it to change before posting the message
243253
const onControllerChange = () => {
244254
navigator.serviceWorker.removeEventListener('controllerchange', onControllerChange);
245-
initServiceWorker(navigator.serviceWorker.controller);
255+
postVersionMessage(navigator.serviceWorker.controller);
246256
};
247257
navigator.serviceWorker.addEventListener('controllerchange', onControllerChange);
248258
}
@@ -376,7 +386,26 @@ const initData = {
376386
themeName: undefined,
377387
};
378388

389+
hostMessaging.onMessage('did-load-resource', (_event, data) => {
390+
navigator.serviceWorker.ready.then(registration => {
391+
assertIsDefined(registration.active).postMessage({ channel: 'did-load-resource', data }, data.data?.buffer ? [data.data.buffer] : []);
392+
});
393+
});
394+
395+
hostMessaging.onMessage('did-load-localhost', (_event, data) => {
396+
navigator.serviceWorker.ready.then(registration => {
397+
assertIsDefined(registration.active).postMessage({ channel: 'did-load-localhost', data });
398+
});
399+
});
379400

401+
navigator.serviceWorker.addEventListener('message', event => {
402+
switch (event.data.channel) {
403+
case 'load-resource':
404+
case 'load-localhost':
405+
hostMessaging.postMessage(event.data.channel, event.data);
406+
return;
407+
}
408+
});
380409
/**
381410
* @param {HTMLDocument?} document
382411
* @param {HTMLElement?} body

src/vs/workbench/contrib/webview/browser/pre/service-worker.js

+82-36
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
const sw = /** @type {ServiceWorkerGlobalScope} */ (/** @type {any} */ (self));
1111

12-
const VERSION = 3;
12+
const VERSION = 4;
1313

1414
const resourceCacheName = `vscode-resource-cache-${VERSION}`;
1515

@@ -127,41 +127,33 @@ const notFound = () =>
127127
const methodNotAllowed = () =>
128128
new Response('Method Not Allowed', { status: 405, });
129129

130-
const vscodeMessageChannel = new MessageChannel();
131-
132-
sw.addEventListener('message', event => {
130+
sw.addEventListener('message', async (event) => {
133131
switch (event.data.channel) {
134-
case 'init':
132+
case 'version':
135133
{
136134
const source = /** @type {Client} */ (event.source);
137135
sw.clients.get(source.id).then(client => {
138-
client?.postMessage({
139-
channel: 'init',
140-
version: VERSION
141-
}, [vscodeMessageChannel.port2]);
136+
if (client) {
137+
client.postMessage({
138+
channel: 'version',
139+
version: VERSION
140+
});
141+
}
142142
});
143143
return;
144144
}
145-
default:
146-
console.log('Unknown message');
147-
return;
148-
}
149-
});
150-
151-
vscodeMessageChannel.port1.onmessage = (event) => {
152-
switch (event.data.channel) {
153145
case 'did-load-resource':
154146
{
155147
/** @type {ResourceResponse} */
156-
const response = event.data;
148+
const response = event.data.data;
157149
if (!resourceRequestStore.resolve(response.id, response)) {
158150
console.log('Could not resolve unknown resource', response.path);
159151
}
160152
return;
161153
}
162154
case 'did-load-localhost':
163155
{
164-
const data = event.data;
156+
const data = event.data.data;
165157
if (!localhostRequestStore.resolve(data.id, data.location)) {
166158
console.log('Could not resolve unknown localhost', data.origin);
167159
}
@@ -171,7 +163,7 @@ vscodeMessageChannel.port1.onmessage = (event) => {
171163
console.log('Unknown message');
172164
return;
173165
}
174-
};
166+
});
175167

176168
sw.addEventListener('fetch', (event) => {
177169
const requestUrl = new URL(event.request.url);
@@ -193,7 +185,7 @@ sw.addEventListener('fetch', (event) => {
193185
});
194186

195187
sw.addEventListener('install', (event) => {
196-
event.waitUntil(sw.skipWaiting());
188+
event.waitUntil(sw.skipWaiting()); // Activate worker immediately
197189
});
198190

199191
sw.addEventListener('activate', (event) => {
@@ -205,6 +197,18 @@ sw.addEventListener('activate', (event) => {
205197
* @param {URL} requestUrl
206198
*/
207199
async function processResourceRequest(event, requestUrl) {
200+
const client = await sw.clients.get(event.clientId);
201+
if (!client) {
202+
console.error('Could not find inner client for request');
203+
return notFound();
204+
}
205+
206+
const webviewId = getWebviewIdForClient(client);
207+
if (!webviewId) {
208+
console.error('Could not resolve webview id');
209+
return notFound();
210+
}
211+
208212
const shouldTryCaching = (event.request.method === 'GET');
209213

210214
/**
@@ -254,6 +258,12 @@ async function processResourceRequest(event, requestUrl) {
254258
return response.clone();
255259
};
256260

261+
const parentClients = await getOuterIframeClient(webviewId);
262+
if (!parentClients.length) {
263+
console.log('Could not find parent client for request');
264+
return notFound();
265+
}
266+
257267
/** @type {Response | undefined} */
258268
let cached;
259269
if (shouldTryCaching) {
@@ -267,15 +277,17 @@ async function processResourceRequest(event, requestUrl) {
267277
const scheme = firstHostSegment.split('+', 1)[0];
268278
const authority = firstHostSegment.slice(scheme.length + 1); // may be empty
269279

270-
vscodeMessageChannel.port1.postMessage({
271-
channel: 'load-resource',
272-
id: requestId,
273-
path: requestUrl.pathname,
274-
scheme,
275-
authority,
276-
query: requestUrl.search.replace(/^\?/, ''),
277-
ifNoneMatch: cached?.headers.get('ETag'),
278-
});
280+
for (const parentClient of parentClients) {
281+
parentClient.postMessage({
282+
channel: 'load-resource',
283+
id: requestId,
284+
path: requestUrl.pathname,
285+
scheme,
286+
authority,
287+
query: requestUrl.search.replace(/^\?/, ''),
288+
ifNoneMatch: cached?.headers.get('ETag'),
289+
});
290+
}
279291

280292
return promise.then(entry => resolveResourceEntry(entry, cached));
281293
}
@@ -292,6 +304,11 @@ async function processLocalhostRequest(event, requestUrl) {
292304
// that are not spawned by vs code
293305
return fetch(event.request);
294306
}
307+
const webviewId = getWebviewIdForClient(client);
308+
if (!webviewId) {
309+
console.error('Could not resolve webview id');
310+
return fetch(event.request);
311+
}
295312

296313
const origin = requestUrl.origin;
297314

@@ -312,13 +329,42 @@ async function processLocalhostRequest(event, requestUrl) {
312329
});
313330
};
314331

315-
const { requestId, promise } = localhostRequestStore.create();
332+
const parentClients = await getOuterIframeClient(webviewId);
333+
if (!parentClients.length) {
334+
console.log('Could not find parent client for request');
335+
return notFound();
336+
}
316337

317-
vscodeMessageChannel.port1.postMessage({
318-
channel: 'load-localhost',
319-
origin: origin,
320-
id: requestId,
321-
});
338+
const { requestId, promise } = localhostRequestStore.create();
339+
for (const parentClient of parentClients) {
340+
parentClient.postMessage({
341+
channel: 'load-localhost',
342+
origin: origin,
343+
id: requestId,
344+
});
345+
}
322346

323347
return promise.then(resolveRedirect);
324348
}
349+
350+
/**
351+
* @param {Client} client
352+
* @returns {string | null}
353+
*/
354+
function getWebviewIdForClient(client) {
355+
const requesterClientUrl = new URL(client.url);
356+
return requesterClientUrl.searchParams.get('id');
357+
}
358+
359+
/**
360+
* @param {string} webviewId
361+
* @returns {Promise<Client[]>}
362+
*/
363+
async function getOuterIframeClient(webviewId) {
364+
const allClients = await sw.clients.matchAll({ includeUncontrolled: true });
365+
return allClients.filter(client => {
366+
const clientUrl = new URL(client.url);
367+
const hasExpectedPathName = (clientUrl.pathname === `${rootPath}/` || clientUrl.pathname === `${rootPath}/index.html`);
368+
return hasExpectedPathName && clientUrl.searchParams.get('id') === webviewId;
369+
});
370+
}

0 commit comments

Comments
 (0)