fix(chromium): fix crash if connecting to a browser with a serviceworker (#5803)

src/dispatchers/browserContextDispatcher.ts

@@ -44,7 +44,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
         this._dispatchEvent('crBackgroundPage', { page: new PageDispatcher(this._scope, page) });
       context.on(CRBrowserContext.CREvents.BackgroundPage, page => this._dispatchEvent('crBackgroundPage', { page: new PageDispatcher(this._scope, page) }));
       for (const serviceWorker of (context as CRBrowserContext).serviceWorkers())
-        this._dispatchEvent('crServiceWorker', new WorkerDispatcher(this._scope, serviceWorker));
+        this._dispatchEvent('crServiceWorker', { worker: new WorkerDispatcher(this._scope, serviceWorker)});
       context.on(CRBrowserContext.CREvents.ServiceWorker, serviceWorker => this._dispatchEvent('crServiceWorker', { worker: new WorkerDispatcher(this._scope, serviceWorker) }));
     }
   }

src/server/chromium/crBrowser.ts

@@ -50,6 +50,9 @@ export class CRBrowser extends Browser {
     const browser = new CRBrowser(connection, options);
     browser._devtools = devtools;
     const session = connection.rootSession;
+    if ((options as any).__testHookOnConnectToBrowser)
+      await (options as any).__testHookOnConnectToBrowser();
+
     const version = await session.send('Browser.getVersion');
     browser._isMac = version.userAgent.includes('Macintosh');
     browser._version = version.product.substring(version.product.indexOf('/') + 1);
@@ -59,32 +62,11 @@ export class CRBrowser extends Browser {
     }
     browser._defaultContext = new CRBrowserContext(browser, undefined, options.persistent);
 
-    const existingTargetAttachPromises: Promise<any>[] = [];
-    // First page, background pages and their service workers in the persistent context
-    // are created automatically and may be initialized before we enable auto-attach.
-    function attachToExistingPage({targetInfo}: Protocol.Target.targetCreatedPayload) {
-      if (targetInfo.type !== 'page' && targetInfo.type !== 'background_page' && targetInfo.type !== 'service_worker')
-        return;
-      // TODO: should we handle the error during 'Target.attachToTarget'? Can the target disappear?
-      existingTargetAttachPromises.push(session.send('Target.attachToTarget', {targetId: targetInfo.targetId, flatten: true}));
-    }
-    session.on('Target.targetCreated', attachToExistingPage);
-
-    const startDiscover = session.send('Target.setDiscoverTargets', { discover: true });
-    const autoAttachAndStopDiscover = session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }).then(() => {
-      // All targets collected before setAutoAttach response will not be auto-attached, the rest will be.
-      // TODO: We should fix this upstream and remove this tricky logic.
-      session.off('Target.targetCreated', attachToExistingPage);
-      return session.send('Target.setDiscoverTargets', { discover: false });
-    });
     await Promise.all([
-      startDiscover,
-      autoAttachAndStopDiscover,
+      session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }),
       (browser._defaultContext as CRBrowserContext)._initialize(),
     ]);
 
-    // Wait for initial targets to arrive.
-    await Promise.all(existingTargetAttachPromises);
     return browser;
   }
 

src/server/firefox/ffBrowser.ts

@@ -35,6 +35,8 @@ export class FFBrowser extends Browser {
   static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise<FFBrowser> {
     const connection = new FFConnection(transport, options.protocolLogger, options.browserLogsCollector);
     const browser = new FFBrowser(connection, options);
+    if ((options as any).__testHookOnConnectToBrowser)
+      await (options as any).__testHookOnConnectToBrowser();
     const promises: Promise<any>[] = [
       connection.send('Browser.enable', { attachToDefaultContext: !!options.persistent }),
       browser._initVersion(),

src/server/webkit/wkBrowser.ts

@@ -39,6 +39,8 @@ export class WKBrowser extends Browser {
 
   static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise<WKBrowser> {
     const browser = new WKBrowser(transport, options);
+    if ((options as any).__testHookOnConnectToBrowser)
+      await (options as any).__testHookOnConnectToBrowser();
     const promises: Promise<any>[] = [
       browser._browserSession.send('Playwright.enable'),
     ];

test/chromium/chromium.spec.ts

@@ -157,4 +157,41 @@ describe('chromium', (suite, { browserName }) => {
       await browserServer.close();
     }
   });
+
+  it('should connect to existing service workers', async ({browserType, testWorkerIndex, browserOptions, server}) => {
+    const port = 9339 + testWorkerIndex;
+    const browserServer = await browserType.launch({
+      ...browserOptions,
+      args: ['--remote-debugging-port=' + port]
+    });
+    try {
+      const json = await new Promise<string>((resolve, reject) => {
+        http.get(`http://localhost:${port}/json/version/`, resp => {
+          let data = '';
+          resp.on('data', chunk => data += chunk);
+          resp.on('end', () => resolve(data));
+        }).on('error', reject);
+      });
+      const cdpBrowser1 = await browserType.connectOverCDP({
+        wsEndpoint: JSON.parse(json).webSocketDebuggerUrl,
+      });
+      const context = cdpBrowser1.contexts()[0] as ChromiumBrowserContext;
+      const page = await cdpBrowser1.contexts()[0].newPage();
+      const [worker] = await Promise.all([
+        context.waitForEvent('serviceworker'),
+        page.goto(server.PREFIX + '/serviceworkers/empty/sw.html')
+      ]);
+      expect(await worker.evaluate(() => self.toString())).toBe('[object ServiceWorkerGlobalScope]');
+      await cdpBrowser1.close();
+
+      const cdpBrowser2 = await browserType.connectOverCDP({
+        wsEndpoint: JSON.parse(json).webSocketDebuggerUrl,
+      });
+      const context2 = cdpBrowser2.contexts()[0] as ChromiumBrowserContext;
+      expect(context2.serviceWorkers().length).toBe(1);
+      await cdpBrowser2.close();
+    } finally {
+      await browserServer.close();
+    }
+  });
 });

test/defaultbrowsercontext-2.spec.ts

@@ -229,3 +229,11 @@ it('should respect selectors', async ({playwright, launchPersistent}) => {
   expect(await page.innerHTML('css=div')).toBe('hello');
   expect(await page.innerHTML('defaultContextCSS=div')).toBe('hello');
 });
+
+it('should connect to a browser with the default page', (test, { mode }) => {
+  test.skip(mode !== 'default');
+}, async ({browserType, browserOptions, createUserDataDir}) => {
+  const options = { ...browserOptions, __testHookOnConnectToBrowser: () => new Promise(f => setTimeout(f, 3000)) };
+  const context = await browserType.launchPersistentContext(await createUserDataDir(), options);
+  expect(context.pages().length).toBe(1);
+});