From 39c96cd2b14e2e8c4baa98bec7124d2ddf37ecca Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 22 Apr 2025 14:21:46 +0100 Subject: [PATCH] fix(chromium): `response.body()` for worker main script With `PlzDedicatedWorker` being the default, worker main script starts in the page target and finishes in the worker target. To retrieve the response body, we should update `request.session` in the same way we do that for OOPIF main requests. --- .../src/server/chromium/crNetworkManager.ts | 14 +++++--- tests/page/workers.spec.ts | 33 ++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crNetworkManager.ts b/packages/playwright-core/src/server/chromium/crNetworkManager.ts index 1ea01c17a9271..beb0032f6bcc0 100644 --- a/packages/playwright-core/src/server/chromium/crNetworkManager.ts +++ b/packages/playwright-core/src/server/chromium/crNetworkManager.ts @@ -490,7 +490,7 @@ export class CRNetworkManager { // @see https://crbug.com/750469 if (!request) return; - this._maybeUpdateOOPIFMainRequest(sessionInfo, request); + this._maybeUpdateRequestSession(sessionInfo, request); // Under certain conditions we never get the Network.responseReceived // event from protocol. @see https://crbug.com/883475 @@ -525,7 +525,7 @@ export class CRNetworkManager { // @see https://crbug.com/750469 if (!request) return; - this._maybeUpdateOOPIFMainRequest(sessionInfo, request); + this._maybeUpdateRequestSession(sessionInfo, request); const response = request.request._existingResponse(); if (response) { response.setTransferSize(null); @@ -540,11 +540,17 @@ export class CRNetworkManager { (this._page?._frameManager || this._serviceWorker)!.requestFailed(request.request, !!event.canceled); } - private _maybeUpdateOOPIFMainRequest(sessionInfo: SessionInfo, request: InterceptableRequest) { + private _maybeUpdateRequestSession(sessionInfo: SessionInfo, request: InterceptableRequest) { // OOPIF has a main request that starts in the parent session but finishes in the child session. // We check for the main request by matching loaderId and requestId, and if it now belongs to // a child session, migrate it there. - if (request.session !== sessionInfo.session && !sessionInfo.isMain && request._documentId === request._requestId) + // + // Same goes for the main worker script with PlzDedicatedWorker enabled, which is the default. + // Here we check the `workerFrame`. + // + // In theory, we can always update the session. However, we try to be conservative here + // to make sure we understand all the scenarios where the session should be updated. + if (request.session !== sessionInfo.session && !sessionInfo.isMain && (request._documentId === request._requestId || sessionInfo.workerFrame)) request.session = sessionInfo.session; } } diff --git a/tests/page/workers.spec.ts b/tests/page/workers.spec.ts index 3ca56a686667d..a4e5b99afc9f2 100644 --- a/tests/page/workers.spec.ts +++ b/tests/page/workers.spec.ts @@ -182,7 +182,10 @@ it('should report network activity on worker creation', async function({ page, s }); it('should report worker script as network request', { - annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/33107' }, + annotation: [ + { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/33107' }, + { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35678' }, + ], }, async function({ page, server }) { await page.goto(server.EMPTY_PAGE); const [request1, request2] = await Promise.all([ @@ -192,6 +195,34 @@ it('should report worker script as network request', { ]); expect.soft(request1.url()).toBe(server.PREFIX + '/worker/worker.js'); expect.soft(request1).toBe(request2); + const response = await request1.response(); + const text = await response.text(); + expect(text).toContain(`console.log('hello from the worker');`); +}); + +it('should report worker script as network request after redirect', { + annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/35678' }, +}, async ({ page, server, browserName }) => { + it.fixme(browserName === 'chromium', 'Chromium does not report the redirect because it is not plumbed to the worker target'); + + await page.goto(server.EMPTY_PAGE); + server.setRedirect('/worker.js', '/worker2.js'); + server.setRoute('/worker2.js', (req, res) => { + res.setHeader('Content-Type', 'text/javascript'); + res.end(`console.log('hello from the worker');`); + }); + const [request] = await Promise.all([ + page.waitForEvent('request', r => r.url().includes('worker.js')), + page.waitForEvent('console', msg => msg.text().includes('hello from the worker')), + page.evaluate(() => (window as any).w = new Worker('/worker.js')), + ]); + expect(request.url()).toBe(server.PREFIX + '/worker.js'); + const redirect = request.redirectedTo(); + expect(redirect).toBeTruthy(); + expect(redirect.url()).toBe(server.PREFIX + '/worker2.js'); + const response = await redirect.response(); + const text = await response.text(); + expect(text).toContain(`console.log('hello from the worker');`); }); it('should dispatch console messages when page has workers', async function({ page, server }) {