From 7bcdfcb7e9e75feca0a8de692926ea25ca8fbed0 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Thu, 21 Sep 2023 11:13:12 +0200 Subject: [PATCH] fix: handle missing detach events for restored bfcache targets (#10967) --- .../src/cdp/ChromeTargetManager.ts | 12 +++----- test/TestExpectations.json | 6 ++++ .../bfcache/worker-iframe-container.html | 11 +++++++ test/assets/cached/bfcache/worker-iframe.html | 3 ++ test/assets/cached/bfcache/worker.mjs | 1 + test/src/bfcache.spec.ts | 30 ++++++++++++++++++- 6 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 test/assets/cached/bfcache/worker-iframe-container.html create mode 100644 test/assets/cached/bfcache/worker-iframe.html create mode 100644 test/assets/cached/bfcache/worker.mjs diff --git a/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts b/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts index fee446f3..28dc8557 100644 --- a/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts +++ b/packages/puppeteer-core/src/cdp/ChromeTargetManager.ts @@ -24,6 +24,7 @@ import {debugError} from '../common/util.js'; import {assert} from '../util/assert.js'; import {Deferred} from '../util/Deferred.js'; +import {type CdpCDPSession} from './CDPSession.js'; import {type Connection} from './Connection.js'; import {CdpTarget, InitializationStatus} from './Target.js'; import { @@ -358,10 +359,7 @@ export class ChromeTargetManager // `this.#connection.isAutoAttached(targetInfo.targetId)`. In the future, we // should determine if a target is auto-attached or not with the help of // CDP. - if ( - targetInfo.type === 'service_worker' && - this.#connection.isAutoAttached(targetInfo.targetId) - ) { + if (targetInfo.type === 'service_worker') { this.#finishInitializationIfReady(targetInfo.targetId); await silentDetach(); if (this.#attachedTargetsByTargetId.has(targetInfo.targetId)) { @@ -393,18 +391,16 @@ export class ChromeTargetManager return; } - if (!isExistingTarget) { - target._initialize(); - } - this.#setupAttachmentListeners(session); if (isExistingTarget) { + (session as CdpCDPSession)._setTarget(target); this.#attachedTargetsBySessionId.set( session.id(), this.#attachedTargetsByTargetId.get(targetInfo.targetId)! ); } else { + target._initialize(); this.#attachedTargetsByTargetId.set(targetInfo.targetId, target); this.#attachedTargetsBySessionId.set(session.id(), target); } diff --git a/test/TestExpectations.json b/test/TestExpectations.json index 865bfee7..c4c61af2 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -1397,6 +1397,12 @@ "parameters": ["chrome", "webDriverBiDi"], "expectations": ["PASS"] }, + { + "testIdPattern": "[bfcache.spec] BFCache can navigate to a BFCached page containing an OOPIF and a worker", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["cdp", "firefox"], + "expectations": ["SKIP"] + }, { "testIdPattern": "[browser.spec] Browser specs Browser.isConnected should set the browser connected state", "platforms": ["darwin", "linux", "win32"], diff --git a/test/assets/cached/bfcache/worker-iframe-container.html b/test/assets/cached/bfcache/worker-iframe-container.html new file mode 100644 index 00000000..857914bb --- /dev/null +++ b/test/assets/cached/bfcache/worker-iframe-container.html @@ -0,0 +1,11 @@ +BFCachednext + diff --git a/test/assets/cached/bfcache/worker-iframe.html b/test/assets/cached/bfcache/worker-iframe.html new file mode 100644 index 00000000..9233f557 --- /dev/null +++ b/test/assets/cached/bfcache/worker-iframe.html @@ -0,0 +1,3 @@ + diff --git a/test/assets/cached/bfcache/worker.mjs b/test/assets/cached/bfcache/worker.mjs new file mode 100644 index 00000000..72a8036e --- /dev/null +++ b/test/assets/cached/bfcache/worker.mjs @@ -0,0 +1 @@ +console.log('HELLO'); diff --git a/test/src/bfcache.spec.ts b/test/src/bfcache.spec.ts index 136a1cbd..a9113ed5 100644 --- a/test/src/bfcache.spec.ts +++ b/test/src/bfcache.spec.ts @@ -15,8 +15,10 @@ */ import expect from 'expect'; +import {PageEvent} from 'puppeteer-core'; import {launch} from './mocha-utils.js'; +import {waitEvent} from './utils.js'; describe('BFCache', function () { it('can navigate to a BFCached page', async () => { @@ -25,7 +27,7 @@ describe('BFCache', function () { }); try { - page.setDefaultTimeout(2000); + page.setDefaultTimeout(3000); await page.goto(httpsServer.PREFIX + '/cached/bfcache/index.html'); @@ -44,4 +46,30 @@ describe('BFCache', function () { await close(); } }); + + it('can navigate to a BFCached page containing an OOPIF and a worker', async () => { + const {httpsServer, page, close} = await launch({ + ignoreHTTPSErrors: true, + }); + try { + page.setDefaultTimeout(3000); + const [worker1] = await Promise.all([ + waitEvent(page, PageEvent.WorkerCreated), + page.goto( + httpsServer.PREFIX + '/cached/bfcache/worker-iframe-container.html' + ), + ]); + expect(await worker1.evaluate('1 + 1')).toBe(2); + await Promise.all([page.waitForNavigation(), page.locator('a').click()]); + + const [worker2] = await Promise.all([ + waitEvent(page, PageEvent.WorkerCreated), + page.waitForNavigation(), + page.goBack(), + ]); + expect(await worker2.evaluate('1 + 1')).toBe(2); + } finally { + await close(); + } + }); });