fix(service-worker): assign initializing client's app version, when a request is for worker script (#58131)

rozpuszczalny · thePunderWoman · commit 4546d4fb3914 · 2025-04-02T14:23:52.000Z
When a new version of app is available in a service worker, and a client with old version exists, web workers initialized from a client with old version will now be properly assigned with the same version. Before this change, a web worker was assigned with the newest version. Fixes #57971 PR Close #58131
diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts
@@ -700,7 +700,14 @@ export class Driver implements Debuggable, UpdateSource {
     //
     // NOTE: For navigation requests, we care about the `resultingClientId`. If it is undefined or
     //       the empty string (which is the case for sub-resource requests), we look at `clientId`.
-    const clientId = event.resultingClientId || event.clientId;
+    //
+    // NOTE: If a request is a worker script, we should use the `clientId`, as worker is a part
+    //       of requesting client.
+    const isWorkerScriptRequest =
+      event.request.destination === 'worker' && event.resultingClientId && event.clientId;
+    const clientId = isWorkerScriptRequest
+      ? event.clientId
+      : event.resultingClientId || event.clientId;
     if (clientId) {
       // Check if there is an assigned client id.
       if (this.clientVersionMap.has(clientId)) {
@@ -729,6 +736,18 @@ export class Driver implements Debuggable, UpdateSource {
           appVersion = this.lookupVersionByHash(this.latestHash, 'assignVersion');
         }
 
+        if (isWorkerScriptRequest) {
+          if (!this.clientVersionMap.has(event.resultingClientId)) {
+            // New worker hasn't been seen before; set this client to requesting client version
+            this.clientVersionMap.set(event.resultingClientId, hash);
+            await this.sync();
+          } else if (this.clientVersionMap.get(event.resultingClientId)! !== hash) {
+            throw new Error(
+              `Version mismatch between worker client ${event.resultingClientId} and requesting client ${clientId}`,
+            );
+          }
+        }
+
         // TODO: make sure the version is valid.
         return appVersion;
       } else {
@@ -763,6 +782,17 @@ export class Driver implements Debuggable, UpdateSource {
           throw new Error(`Invariant violated (assignVersion): latestHash was null`);
         }
 
+        if (isWorkerScriptRequest) {
+          if (!this.clientVersionMap.has(event.resultingClientId)) {
+            // New worker hasn't been seen before; set this client to latest hash as well
+            this.clientVersionMap.set(event.resultingClientId, this.latestHash);
+          } else if (this.clientVersionMap.get(event.resultingClientId)! !== this.latestHash) {
+            throw new Error(
+              `Version mismatch between worker client ${event.resultingClientId} and requesting client ${clientId}`,
+            );
+          }
+        }
+
         // Pin this client ID to the current latest version, indefinitely.
         this.clientVersionMap.set(clientId, this.latestHash);
         await this.sync();
diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts
@@ -472,6 +472,24 @@ import {envIsSupported} from '../testing/utils';
       expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo v2');
     });
 
+    it('returns old content for worker initialized from old version', async () => {
+      expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
+      expect(await makeRequest(scope, '/baz.txt')).toEqual('this is baz');
+      await driver.initialized;
+
+      const client = scope.clients.getMock('default')!;
+      expect(client.messages).toEqual([]);
+
+      scope.updateServerState(serverUpdate);
+      expect(await driver.checkForUpdate()).toEqual(true);
+
+      // Worker request came from default client, old version should be served
+      expect(await makeWorkerRequest(scope, '/foo.txt')).toEqual('this is foo');
+      // Worker has been initialized from default client, further requests from worker should be served from old version
+      expect(await makeRequest(scope, '/foo.txt', 'worker')).toEqual('this is foo');
+      expect(await makeRequest(scope, '/baz.txt', 'worker')).toEqual('this is baz');
+    });
+
     it('handles empty client ID', async () => {
       // Initialize the SW.
       expect(await makeNavigationRequest(scope, '/foo/file1', '')).toEqual('this is foo');
@@ -2657,6 +2675,26 @@ async function makeRequest(
   return null;
 }
 
+async function makeWorkerRequest(
+  scope: SwTestHarness,
+  url: string,
+  clientId = 'default',
+  resultingClientId = 'worker',
+  init?: Object,
+): Promise<string | null> {
+  const [resPromise, done] = scope.handleFetch(
+    new MockRequest(url, {...init, destination: 'worker'}),
+    clientId,
+    resultingClientId,
+  );
+  await done;
+  const res = await resPromise;
+  if (res !== undefined && res.ok) {
+    return res.text();
+  }
+  return null;
+}
+
 function makeNavigationRequest(
   scope: SwTestHarness,
   url: string,
diff --git a/packages/service-worker/worker/testing/fetch.ts b/packages/service-worker/worker/testing/fetch.ts
@@ -122,7 +122,10 @@ export class MockRequest extends MockBody implements Request {
 
   url: string;
 
-  constructor(input: string | Request, init: RequestInit = {}) {
+  constructor(
+    input: string | Request,
+    init: RequestInit & {destination?: RequestDestination} = {},
+  ) {
     super((init.body as string | null) ?? null);
     if (typeof input !== 'string') {
       throw 'Not implemented';
@@ -150,6 +153,9 @@ export class MockRequest extends MockBody implements Request {
     if (init.method !== undefined) {
       this.method = init.method;
     }
+    if (init.destination !== undefined) {
+      this.destination = init.destination;
+    }
   }
 
   clone(): Request {
diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts
@@ -192,7 +192,11 @@ export class SwTestHarnessImpl
     this.skippedWaiting = true;
   }
 
-  handleFetch(req: Request, clientId = ''): [Promise<Response | undefined>, Promise<void>] {
+  handleFetch(
+    req: Request,
+    clientId = '',
+    resultingClientId?: string,
+  ): [Promise<Response | undefined>, Promise<void>] {
     if (!this.eventHandlers.has('fetch')) {
       throw new Error('No fetch handler registered');
     }
@@ -203,9 +207,12 @@ export class SwTestHarnessImpl
       this.clients.add(clientId, isNavigation ? req.url : this.scopeUrl);
     }
 
-    const event = isNavigation
-      ? new MockFetchEvent(req, '', clientId)
-      : new MockFetchEvent(req, clientId, '');
+    const event =
+      clientId && resultingClientId
+        ? new MockFetchEvent(req, clientId, resultingClientId)
+        : isNavigation
+          ? new MockFetchEvent(req, '', clientId)
+          : new MockFetchEvent(req, clientId, '');
     this.eventHandlers.get('fetch')!.call(this, event);
 
     return [event.response, event.ready];
