Service worker: the MTProto port goes only to a tab that answers, never to the PiP window a request comes from

A request made while the worker had no MTProto port handed the port to the window that made it. With
the client moved into a Document Picture-in-Picture window, every video is requested from that window,
which has no listener: the port went there, nobody read it, and since the worker counted that window as
connected, the real tab's next hello got no port either - streaming stayed stuck.

- A window is connected once it answers hello, as the startup check already required: only the app's
  own tabs do. A request with no port runs that same check again over every window, so in PiP the port
  goes to the tab the app runs in
- A window is asked once while its answer is awaited, and one look at the window list runs at a time:
  a window that never answers costs one pending hello, not one per request
- A hello that fails no longer connects its window
- The test reuses the shared deferred helper and one startup helper, and covers the PiP window and the
  single pending hello

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: morethanwords

src/lib/serviceWorker/index.service.ts

@@ -188,37 +188,44 @@ const {
   onClosedWindows: onDownloadClosedWindows
 } = handleDownload(serviceMessagePort);
 
+const helloAwaiting: Set<string> = new Set();
+let checkingWindows: Promise<void>;
+
 // * service worker can be killed, so won't get 'hello' event
-async function startupCheck() {
+// * a window is connected once it answers: only the app's tabs do, never a window with no app in it (the
+// * Document PiP window the app moves into), which would take the MTProto port with nobody to read it
+async function helloWindows(from: string) {
   const windowClients = await getWindowClients();
   const length = windowClients.length;
-  log(`got ${length} windows from the start`);
+  log(`got ${length} windows from`, from);
   windowClients.forEach((windowClient) => {
     if(windowClient.frameType === 'none') {
       log.warn('skipping bugged Safari starting window', windowClient.id);
       return;
     }
 
+    if(helloAwaiting.has(windowClient.id)) {
+      return;
+    }
+
     log('checking window', windowClient.id);
-    try {
-      const promise = serviceMessagePort.invoke(
-        'hello',
-        undefined,
-        undefined,
-        windowClient
-      );
-
-      // * a busy or frozen tab answers long after the worker has started, and is no less alive for it:
-      // * nothing asks it again, so dropping a late answer left the worker with no MTProto port
-      promise.finally(() => {
-        onWindowConnected(windowClient, 'startup check');
-      });
-    } catch(err) {
+    helloAwaiting.add(windowClient.id);
+    // * a busy or frozen tab answers long after it is asked, and is no less alive for it:
+    // * dropping a late answer left the worker with no MTProto port
+    serviceMessagePort.invoke('hello', undefined, undefined, windowClient).then(() => {
+      onWindowConnected(windowClient, from);
+    }, (err) => {
       log.error('failed to send hello to window', windowClient.id, err);
-    }
+    }).finally(() => {
+      helloAwaiting.delete(windowClient.id);
+    });
   });
 }
-startupCheck();
+
+const checkWindows = (from: string) => checkingWindows ??= helloWindows(from).finally(() => {
+  checkingWindows = undefined;
+});
+checkWindows('startup check');
 
 const connectedWindows: Map<string, WindowClient> = new Map();
 (self as any).connectedWindows = connectedWindows;
@@ -271,22 +278,6 @@ watchCacheStoragesLifetime({
 
 watchMtprotoOnDev({connectedWindows, onWindowConnected});
 
-/**
- * A tab the worker does not count as connected - its startup answer was lost, or it was taken for gone -
- * still asks for files, and with no MTProto port they hang until the worker restarts. Asking says it is
- * alive, so take it back. Costs nothing while the port is there.
- */
-const reconnectWindowIfNeeded = async(clientId: string) => {
-  if(_mtprotoMessagePort || !clientId) {
-    return;
-  }
-
-  const client = await ctx.clients.get(clientId);
-  if(client?.type === 'window' && !_mtprotoMessagePort) {
-    onWindowConnected(client as WindowClient, 'fetch');
-  }
-};
-
 const onFetch = (event: FetchEvent): void => {
   // Web manifests must reach the network so installed PWA metadata can update.
   if(
@@ -302,7 +293,12 @@ const onFetch = (event: FetchEvent): void => {
     return;
   }
 
-  reconnectWindowIfNeeded(event.clientId);
+  // * a tab the worker does not count as connected - its startup answer was lost, or it was taken for gone -
+  // * still asks for files, and with no MTProto port they hang until the worker restarts. The request may come
+  // * from the PiP window rather than the tab its app runs in, so every window is asked again
+  if(!_mtprotoMessagePort) {
+    checkWindows('fetch');
+  }
 
   try {
     // const [, url, scope, params] = /http[:s]+\/\/.*?(\/(.*?)(?:$|\/(.*)$))/.exec(event.request.url) || [];

src/tests/serviceWorkerMtprotoPort.test.ts

@@ -1,9 +1,11 @@
 /*
  * The service worker streams every file through a port of its own to the MTProto worker, handed over
  * by the first tab it counts as connected. A tab it does not count - one that answered the startup check
- * late, one it took for gone - must still get that port, or every video hangs until the worker restarts.
+ * late, one it took for gone - must still get that port, or every video hangs until the worker restarts;
+ * and a window with no app in it to answer, the Document PiP window, must never get it.
  */
 import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest';
+import deferred from '@/tests/helpers/deferred';
 
 const mocks = vi.hoisted(() => ({
   invoke: vi.fn(),
@@ -59,15 +61,25 @@ vi.mock('@lib/passcode/deferredIsUsingPasscode', () => ({default: {
 }}));
 
 const tab = {id: 'tab-1', type: 'window', frameType: 'top-level'};
+// * the Document PiP window the app moves into: a window of the origin, with no app of its own to answer
+const pip = {id: 'pip-1', type: 'window', frameType: 'auxiliary'};
 
-const startWorker = () => import('@lib/serviceWorker/index.service');
+const startWorker = async() => {
+  await import('@lib/serviceWorker/index.service');
+  await vi.advanceTimersByTimeAsync(0);
+};
 
-const fetchFromTab = (clientId: string) => (self as any).onfetch({
+const fetchFrom = (client: {id: string}) => (self as any).onfetch({
   request: {url: 'https://web.telegram.org/k/stream/%7B%7D', mode: 'cors', headers: new Headers()},
-  clientId,
+  clientId: client.id,
   respondWith: () => {}
 });
 
+const answerHelloFrom = (...windows: any[]) => mocks.invoke.mockImplementation((type: string, payload: any, withAck: any, target: any) => {
+  return type === 'hello' && windows.includes(target) ? Promise.resolve() : new Promise(() => {});
+});
+
+const hellosSentTo = (target: any) => mocks.invoke.mock.calls.filter(([type, , , source]) => type === 'hello' && source === target);
 const portsSentTo = (target: any) => mocks.invokeVoid.mock.calls.filter(([type, , source]) => type === 'port' && source === target);
 
 describe('the service worker\'s MTProto port', () => {
@@ -77,40 +89,36 @@ describe('the service worker\'s MTProto port', () => {
     mocks.invoke.mockReset().mockReturnValue(new Promise(() => {}));
     mocks.invokeVoid.mockReset();
     mocks.getWindowClients.mockReset().mockResolvedValue([]);
-    (self as any).clients = {get: vi.fn(async(id: string) => id === tab.id ? tab : undefined)};
   });
 
   afterEach(() => {
     vi.useRealTimers();
-    delete (self as any).clients;
     delete (self as any).onfetch;
   });
 
   test('goes to a tab that answers the startup check late', async() => {
-    let answer: () => void;
-    mocks.invoke.mockImplementation((type: string) => type === 'hello' ?
-      new Promise<void>((resolve) => answer = resolve) :
-      new Promise(() => {}));
+    const answer = deferred();
+    mocks.invoke.mockImplementation((type: string) => type === 'hello' ? answer.promise : new Promise(() => {}));
     mocks.getWindowClients.mockResolvedValue([tab]);
 
     await startWorker();
-    await vi.advanceTimersByTimeAsync(0);
-    expect(mocks.invoke).toHaveBeenCalledWith('hello', undefined, undefined, tab);
+    expect(hellosSentTo(tab)).toHaveLength(1);
 
     // * the tab is busy or frozen while the worker starts, and answers well after it
     await vi.advanceTimersByTimeAsync(30e3);
     expect(portsSentTo(tab)).toHaveLength(0);
-    answer();
+    answer.resolve();
     await vi.advanceTimersByTimeAsync(0);
 
     expect(portsSentTo(tab), 'the late answer was dropped').toHaveLength(1);
   });
 
   test('goes to a tab the worker does not count with its next request', async() => {
     await startWorker();
-    await vi.advanceTimersByTimeAsync(0);
+    answerHelloFrom(tab);
+    mocks.getWindowClients.mockResolvedValue([tab]);
 
-    fetchFromTab(tab.id);
+    fetchFrom(tab);
     await vi.advanceTimersByTimeAsync(0);
 
     expect(portsSentTo(tab), 'the request hangs with no port').toHaveLength(1);
@@ -119,30 +127,43 @@ describe('the service worker\'s MTProto port', () => {
 
   test('costs a connected tab\'s requests nothing and is not sent twice', async() => {
     await startWorker();
+    answerHelloFrom(tab);
+    mocks.getWindowClients.mockResolvedValue([tab]);
+    fetchFrom(tab);
     await vi.advanceTimersByTimeAsync(0);
-    fetchFromTab(tab.id);
-    await vi.advanceTimersByTimeAsync(0);
-    const getClient = (self as any).clients.get;
-    getClient.mockClear();
+    mocks.getWindowClients.mockClear();
 
-    fetchFromTab(tab.id);
-    fetchFromTab(tab.id);
+    fetchFrom(tab);
+    fetchFrom(tab);
     await vi.advanceTimersByTimeAsync(0);
 
-    expect(getClient).not.toHaveBeenCalled();
+    expect(mocks.getWindowClients).not.toHaveBeenCalled();
     expect(portsSentTo(tab)).toHaveLength(1);
   });
 
-  test('is not handed to anything but a window', async() => {
-    const worker = {id: 'worker-1', type: 'sharedworker'};
-    (self as any).clients.get = vi.fn(async() => worker);
+  test('goes to the tab whose app answers, not to the PiP window the request comes from', async() => {
     await startWorker();
+    answerHelloFrom(tab);
+    mocks.getWindowClients.mockResolvedValue([pip, tab]);
+
+    fetchFrom(pip);
     await vi.advanceTimersByTimeAsync(0);
 
-    fetchFromTab(worker.id);
-    fetchFromTab('');
+    expect(portsSentTo(pip), 'nobody reads the port there').toHaveLength(0);
+    expect(portsSentTo(tab)).toHaveLength(1);
+  });
+
+  test('asks a window once while its answer is awaited', async() => {
+    await startWorker();
+    mocks.getWindowClients.mockResolvedValue([pip]);
+
+    fetchFrom(pip);
+    await vi.advanceTimersByTimeAsync(0);
+    fetchFrom(pip);
+    fetchFrom(pip);
     await vi.advanceTimersByTimeAsync(0);
 
+    expect(hellosSentTo(pip)).toHaveLength(1);
     expect(mocks.invokeVoid.mock.calls.filter(([type]) => type === 'port')).toHaveLength(0);
   });
 });