From 7b24363ce8ddc3bc6a8f8affcc72f80b098fab11 Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Fri, 12 Mar 2021 05:06:09 -0800 Subject: [PATCH] fix(chromium): fix crash if connecting to a browser with a serviceworker --- src/dispatchers/browserContextDispatcher.ts | 2 +- src/server/chromium/crBrowser.ts | 26 +++------------ src/server/firefox/ffBrowser.ts | 2 ++ src/server/webkit/wkBrowser.ts | 2 ++ test/chromium/chromium.spec.ts | 37 +++++++++++++++++++++ test/defaultbrowsercontext-2.spec.ts | 8 +++++ 6 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/dispatchers/browserContextDispatcher.ts b/src/dispatchers/browserContextDispatcher.ts index 740ed0c275698..d350293b82b56 100644 --- a/src/dispatchers/browserContextDispatcher.ts +++ b/src/dispatchers/browserContextDispatcher.ts @@ -44,7 +44,7 @@ export class BrowserContextDispatcher extends Dispatcher 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) })); } } diff --git a/src/server/chromium/crBrowser.ts b/src/server/chromium/crBrowser.ts index 62eb24d7b90c0..081e06440ee16 100644 --- a/src/server/chromium/crBrowser.ts +++ b/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[] = []; - // 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; } diff --git a/src/server/firefox/ffBrowser.ts b/src/server/firefox/ffBrowser.ts index 90ab3f76966f3..f3f247404dfd1 100644 --- a/src/server/firefox/ffBrowser.ts +++ b/src/server/firefox/ffBrowser.ts @@ -35,6 +35,8 @@ export class FFBrowser extends Browser { static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { 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[] = [ connection.send('Browser.enable', { attachToDefaultContext: !!options.persistent }), browser._initVersion(), diff --git a/src/server/webkit/wkBrowser.ts b/src/server/webkit/wkBrowser.ts index 0939de31080a9..90f6f6920fc39 100644 --- a/src/server/webkit/wkBrowser.ts +++ b/src/server/webkit/wkBrowser.ts @@ -39,6 +39,8 @@ export class WKBrowser extends Browser { static async connect(transport: ConnectionTransport, options: BrowserOptions): Promise { const browser = new WKBrowser(transport, options); + if ((options as any).__testHookOnConnectToBrowser) + await (options as any).__testHookOnConnectToBrowser(); const promises: Promise[] = [ browser._browserSession.send('Playwright.enable'), ]; diff --git a/test/chromium/chromium.spec.ts b/test/chromium/chromium.spec.ts index 862f0a2630727..f131e30d1e8bd 100644 --- a/test/chromium/chromium.spec.ts +++ b/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((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(); + } + }); }); diff --git a/test/defaultbrowsercontext-2.spec.ts b/test/defaultbrowsercontext-2.spec.ts index a8713fa25937e..f7e873c9ad4d6 100644 --- a/test/defaultbrowsercontext-2.spec.ts +++ b/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); +});