diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index db00e29a01059..0536acccaaeaf 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -41,6 +41,12 @@ import type { BrowserOptions } from '../browser'; import type { SdkObject } from '../instrumentation'; import type * as channels from '../channels'; +// How long connect waits for a pre-existing page's renderer to answer before leaving it to +// initialize in the background. +const kUnresponsivePageTimeout = 3000; + +export type ContextUpdateKind = 'extraHTTPHeaders' | 'offline' | 'httpCredentials' | 'requestInterception' | 'geolocation' | 'userAgent' | 'initScripts' | 'playwrightBinding'; + export class CRBrowser extends Browser { readonly _connection: CRConnection; _session: CRSession; @@ -159,7 +165,10 @@ export class CRBrowser extends Browser { } async _waitForAllPagesToBeInitialized() { - await Promise.all([...this._crPages.values()].map(crPage => crPage._page.waitForInitializedOrError())); + // A pre-existing tab whose renderer is frozen or asleep (Memory Saver, sleeping tabs, a hung + // renderer) never answers page-level CDP. Waiting on it would hold the connection forever, + // so wait only for pages whose renderer responds. + await Promise.all([...this._crPages.values()].map(crPage => crPage._waitForInitializedIfResponsive(kUnresponsivePageTimeout))); } _onAttachedToTarget({ targetInfo, sessionId, waitingForDebugger }: Protocol.Target.attachedToTargetPayload) { @@ -342,6 +351,12 @@ export class CRBrowserContext extends BrowserContext { static CREvents = CREvents; declare readonly _browser: CRBrowser; + // Counts, per kind, the context-wide updates that are pushed to initialized pages only, so that a + // page still initializing can tell which ones it missed. See CRPage._syncContextUpdatesMissedDuringInitialization. + readonly _updateCounts: Record = { + extraHTTPHeaders: 0, offline: 0, httpCredentials: 0, requestInterception: 0, + geolocation: 0, userAgent: 0, initScripts: 0, playwrightBinding: 0, + }; constructor(browser: CRBrowser, browserContextId: string | undefined, options: types.BrowserContextOptions) { super(browser, options, browserContextId); @@ -484,11 +499,13 @@ export class CRBrowserContext extends BrowserContext { async setGeolocation(geolocation?: types.Geolocation): Promise { verifyGeolocation(geolocation); this._options.geolocation = geolocation; + ++this._updateCounts.geolocation; for (const page of this.pages()) await (page.delegate as CRPage).updateGeolocation(); } async doUpdateExtraHTTPHeaders(): Promise { + ++this._updateCounts.extraHTTPHeaders; for (const page of this.pages()) await (page.delegate as CRPage).updateExtraHTTPHeaders(); for (const sw of this.serviceWorkers()) @@ -497,6 +514,7 @@ export class CRBrowserContext extends BrowserContext { async setUserAgent(userAgent: string | undefined): Promise { this._options.userAgent = userAgent; + ++this._updateCounts.userAgent; await Promise.all([ ...this.pages().map(page => (page.delegate as CRPage).updateUserAgent()), ...this.serviceWorkers().map(sw => sw.updateUserAgent()), @@ -504,6 +522,7 @@ export class CRBrowserContext extends BrowserContext { } async doUpdateOffline(): Promise { + ++this._updateCounts.offline; for (const page of this.pages()) await (page.delegate as CRPage).updateOffline(); for (const sw of this.serviceWorkers()) @@ -512,6 +531,7 @@ export class CRBrowserContext extends BrowserContext { async doSetHTTPCredentials(httpCredentials?: HttpCredentials[]): Promise { this._options.httpCredentials = httpCredentials; + ++this._updateCounts.httpCredentials; for (const page of this.pages()) await (page.delegate as CRPage).updateHttpCredentials(); for (const sw of this.serviceWorkers()) @@ -519,16 +539,19 @@ export class CRBrowserContext extends BrowserContext { } async doAddInitScript(initScript: InitScript) { + ++this._updateCounts.initScripts; for (const page of this.pages()) await (page.delegate as CRPage).addInitScript(initScript); } async doRemoveInitScripts(initScripts: InitScript[]) { + ++this._updateCounts.initScripts; for (const page of this.pages()) await (page.delegate as CRPage).removeInitScripts(initScripts); } async doUpdateRequestInterception(): Promise { + ++this._updateCounts.requestInterception; for (const page of this.pages()) await (page.delegate as CRPage).updateRequestInterception(); for (const sw of this.serviceWorkers()) @@ -544,6 +567,7 @@ export class CRBrowserContext extends BrowserContext { } override async doExposePlaywrightBinding() { + ++this._updateCounts.playwrightBinding; for (const page of this._crPages()) await page.exposePlaywrightBinding(); } diff --git a/packages/playwright-core/src/server/chromium/crPage.ts b/packages/playwright-core/src/server/chromium/crPage.ts index 17d2933f545c7..aec8d6f5ef74e 100644 --- a/packages/playwright-core/src/server/chromium/crPage.ts +++ b/packages/playwright-core/src/server/chromium/crPage.ts @@ -16,6 +16,7 @@ */ import { assert } from '@isomorphic/assert'; +import { ManualPromise } from '@isomorphic/manualPromise'; import { rewriteErrorMessage } from '@utils/stackTrace'; import { eventsHelper } from '@utils/eventsHelper'; import * as dialog from '../dialog'; @@ -38,6 +39,7 @@ import { isSessionClosedError } from '../protocolError'; import { startAutomaticVideoRecording } from '../videoRecorder'; import { nullProgress } from '../progress'; +import type { ContextUpdateKind } from './crBrowser'; import type { CRSession } from './crConnection'; import type { Protocol } from './protocol'; import type { RegisteredListener } from '@utils/eventsHelper'; @@ -86,6 +88,8 @@ export class CRPage implements PageDelegate { // of their Page.windowOpen events is not guaranteed to match the order // of new popup targets. readonly _nextWindowOpenPopupFeatures: string[][] = []; + // Context-wide update counts when this page started initializing, see CRBrowserContext._updateCounts. + private readonly _initialUpdateCounts: Record; static mainFrameSession(page: Page): FrameSession { const crPage = page.delegate as CRPage; @@ -102,6 +106,7 @@ export class CRPage implements PageDelegate { this._pdf = new CRPDF(client); this._coverage = new CRCoverage(client); this._browserContext = browserContext; + this._initialUpdateCounts = { ...browserContext._updateCounts }; this._page = new Page(this, browserContext); // Create a unique utility world for this Playwright instance, just in case there // are multiple instances of Playwright connected to the same browser page. @@ -122,11 +127,48 @@ export class CRPage implements PageDelegate { this._page.setEmulatedSizeFromWindowOpen({ viewport: viewportSize, screen: viewportSize }); } - this._mainFrameSession._initialize(bits.hasUIWindow).then( + this._mainFrameSession._initialize(bits.hasUIWindow).then(() => this._syncContextUpdatesMissedDuringInitialization()).then( () => this._page.reportAsNew(this._opener?._page, undefined), error => this._page.reportAsNew(this._opener?._page, error)); } + // Context-wide updates only reach initialized pages, while initialization reads the context + // state once, up front. A page that initializes slowly - for example a pre-existing tab that + // was asleep when connectOverCDP attached - would otherwise miss updates made in between. + // Re-apply only the kinds that actually changed: some updates are not no-ops when repeated + // (a user agent override, clearing geolocation) and must not touch a page nobody asked about. + private async _syncContextUpdatesMissedDuringInitialization() { + const missed = (kind: ContextUpdateKind) => this._browserContext._updateCounts[kind] !== this._initialUpdateCounts[kind]; + const updates: Record Promise> = { + extraHTTPHeaders: () => this.updateExtraHTTPHeaders(), + offline: () => this.updateOffline(), + httpCredentials: () => this.updateHttpCredentials(), + requestInterception: () => this.updateRequestInterception(), + geolocation: () => this.updateGeolocation(), + userAgent: () => this.updateUserAgent(), + initScripts: () => this._forAllFrameSessions(frame => frame._syncInitScripts()), + playwrightBinding: () => this.exposePlaywrightBinding(), + }; + await Promise.all((Object.keys(updates) as ContextUpdateKind[]).filter(missed).map(kind => updates[kind]())); + } + + // Resolves once the page is initialized, or right away if its renderer has not answered a + // single page-level command within `timeout`. A frozen or sleeping tab never answers, and + // keeps initializing in the background; it is reported through the page event if it wakes. + async _waitForInitializedIfResponsive(timeout: number) { + const initialized = this._page.waitForInitializedOrError(); + let timer: NodeJS.Timeout | undefined; + const unresponsive = new Promise(f => timer = setTimeout(() => f(false), timeout)); + const responsive = await Promise.race([ + initialized.then(() => true), + this._mainFrameSession._rendererResponded.then(() => true), + unresponsive, + ]); + clearTimeout(timer); + if (responsive) + await initialized; + } + private async _forAllFrameSessions(cb: (frame: FrameSession) => Promise) { const frameSessions = Array.from(this._sessions.values()); await Promise.all(frameSessions.map(frameSession => { @@ -415,6 +457,8 @@ class FrameSession { _metricsOverride: Protocol.Emulation.setDeviceMetricsOverrideParameters | undefined; private _workerSessions = new Map(); private _initScriptIds = new Map(); + // Page.getFrameTree is answered by the renderer, so a reply proves the renderer is responsive. + readonly _rendererResponded = new ManualPromise(); private _bufferedAttachedToTargetEvents: Protocol.Target.attachedToTargetPayload[] | undefined; constructor(crPage: CRPage, client: CRSession, targetId: string, parentSession: FrameSession | null) { @@ -499,6 +543,7 @@ class FrameSession { const promises: Promise[] = [ this._client.send('Page.enable'), this._client.send('Page.getFrameTree').then(({ frameTree }) => { + this._rendererResponded.resolve(); if (this._isMainFrame()) { this._handleFrameTree(frameTree); this._addRendererListeners(); @@ -1090,6 +1135,12 @@ class FrameSession { await Promise.all(ids.map(identifier => this._client.send('Page.removeScriptToEvaluateOnNewDocument', { identifier }).catch(() => {}))); // target can be closed } + async _syncInitScripts(): Promise { + const wanted = new Set(this._crPage._page.allInitScripts()); + await this._removeEvaluatesOnNewDocument([...this._initScriptIds.keys()].filter(script => !wanted.has(script))); + await Promise.all([...wanted].filter(script => !this._initScriptIds.has(script)).map(script => this._evaluateOnNewDocument(script, 'main'))); + } + async exposePlaywrightBinding() { await this._client.send('Runtime.addBinding', { name: PageBinding.kBindingName }); } diff --git a/tests/library/chromium/connect-over-cdp.spec.ts b/tests/library/chromium/connect-over-cdp.spec.ts index 64b4c1be9d543..f4a81c1b89d85 100644 --- a/tests/library/chromium/connect-over-cdp.spec.ts +++ b/tests/library/chromium/connect-over-cdp.spec.ts @@ -781,3 +781,53 @@ test('should connect over CDP using a ConnectionTransport', async ({ browserType await browserServer.close(); } }); + +test('should not wait for a pre-existing page whose renderer never responds', async ({ browserType, server }, testInfo) => { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ args: ['--remote-debugging-port=' + port] }); + try { + const first = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`); + const responsive = await first.contexts()[0].newPage(); + await responsive.goto(server.EMPTY_PAGE); + // A renderer that is busy forever stands in for a tab frozen by Memory Saver: it never answers page-level CDP. + const frozen = await first.contexts()[0].newPage(); + await frozen.goto('data:text/html,'); + await new Promise(f => setTimeout(f, 500)); + + const second = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`, { timeout: 15000 }); + const urls = second.contexts()[0].pages().map(page => page.url()); + expect(urls).toContain(server.EMPTY_PAGE); + expect(urls.filter(url => url.startsWith('data:'))).toEqual([]); + await second.close(); + await first.close(); + } finally { + await browserServer.close(); + } +}); + +test('should report a pre-existing page once its renderer responds, with context updates made meanwhile', async ({ browserType, server }, testInfo) => { + const port = 9339 + testInfo.workerIndex; + const browserServer = await browserType.launch({ args: ['--remote-debugging-port=' + port] }); + try { + const first = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`); + const sleeping = await first.contexts()[0].newPage(); + // Busy for well past the grace period connect gives an unresponsive page, then wakes up. + await sleeping.goto('data:text/html,'); + await new Promise(f => setTimeout(f, 500)); + + const second = await browserType.connectOverCDP(`http://127.0.0.1:${port}/`, { timeout: 15000 }); + const context = second.contexts()[0]; + expect(context.pages().filter(page => page.url().startsWith('data:'))).toEqual([]); + const pagePromise = context.waitForEvent('page', { timeout: 15000 }); + // Added while the page is still unresponsive, so it only reaches the page once it wakes. + await context.addInitScript(() => (window as any).__addedWhileAsleep = 'yes'); + const woken = await pagePromise; + expect(woken.url()).toMatch(/^data:/); + await woken.goto(server.EMPTY_PAGE); + expect(await woken.evaluate(() => (window as any).__addedWhileAsleep)).toBe('yes'); + await second.close(); + await first.close(); + } finally { + await browserServer.close(); + } +});