diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 82139d2facac7..d126cb820a6a9 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -762,12 +762,13 @@ class FrameSession { return; } - // ... or an oopif. - const childFrameSession = this._crPage._sessions.get(event.targetId!); + // ... or an oopif / cross-process navigation. + const targetId = event.targetId!; + const childFrameSession = this._crPage._sessions.get(targetId); if (!childFrameSession) return; - // Usually, we get frameAttached in this session first and mark child as swappedIn. + // FIX PART 1: Synchronous early exit if already swapped if (childFrameSession._swappedIn) { childFrameSession.dispose(); return; @@ -777,11 +778,25 @@ class FrameSession { // In this case we don't know whether this is a remote frame detach, // or just a remote -> local transition. In the latter case, frameAttached // is already inflight, so let's make a safe roundtrip to ensure it arrives. - this._client.send('Page.enable').catch(e => null).then(() => { - // Child was not swapped in - that means frameAttached did not happen and - // this is remote detach rather than remote -> local swap. - if (!childFrameSession._swappedIn) - this._page.frameManager.frameDetached(event.targetId!); + this._client.send('Page.enable').catch(() => null).then(() => { + // FIX PART 2: Re-check _swappedIn after the microtask delay + if (childFrameSession._swappedIn) { + childFrameSession.dispose(); + return; + } + + // FIX PART 3: The core race-condition guard. + // If the session currently mapped to this targetId is NO LONGER + // the childFrameSession that triggered this detachment, it means + // a new session (from a cross-process navigation) has already + // taken over this targetId. We must NOT call frameDetached. + if (this._crPage._sessions.get(targetId) !== childFrameSession) { + childFrameSession.dispose(); + return; + } + + // Safe to proceed with normal detachment + this._page.frameManager.frameDetached(targetId); childFrameSession.dispose(); }); } diff --git a/tests/library/chromium/oopif.spec.ts b/tests/library/chromium/oopif.spec.ts index a8971fe638a4c..24824c5665557 100644 --- a/tests/library/chromium/oopif.spec.ts +++ b/tests/library/chromium/oopif.spec.ts @@ -388,6 +388,34 @@ it('should allow to re-connect to OOPIFs with CDP when iframes were there alread await hostBrowser.close(); }); +// @see https://github.com/microsoft/playwright/issues/41348 +it('should not throw TargetClosedException on cross-origin redirect after click', async ({ page, server }) => { + // 1. Set up a same-origin endpoint that 302 redirects to a cross-origin URL + server.setRedirect('/redirect', server.CROSS_PROCESS_PREFIX + '/target.html'); + + // 2. Start on a same-origin page with a form + await page.goto(server.PREFIX + '/empty.html'); + await page.setContent(` +
+ `); + + // 3. Click the button. This triggers a POST -> 302 -> Cross-Origin Navigation. + // In Playwright 1.60, this specific sequence caused a race condition in + // crPage.ts _onDetachedFromTarget, throwing TargetClosedException. + await Promise.all([ + page.waitForNavigation(), + page.click('button'), + ]); + + // 4. Verify we successfully landed on the cross-origin page without crashing + expect(page.url()).toBe(server.CROSS_PROCESS_PREFIX + '/target.html'); + + // 5. Ensure the page is still alive and functional + await page.evaluate(() => document.title); +}); + async function assertOOPIFCount(browser: Browser, count: number) { if (browser.browserType().name() !== 'chromium') return;