diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index 755b85525cd3c..9ae90fbaabb64 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -46,7 +46,6 @@ export class CRBrowser extends platform.EventEmitter implements Browser { const connection = new CRConnection(SlowMoTransport.wrap(transport, slowMo)); const browser = new CRBrowser(connection); await connection.rootSession.send('Target.setDiscoverTargets', { discover: true }); - await browser.waitForTarget(t => t.type() === 'page'); return browser; } diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index 6fc992abb5757..51ed6087a247f 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -39,7 +39,6 @@ export class FFBrowser extends platform.EventEmitter implements Browser { const connection = new FFConnection(SlowMoTransport.wrap(transport, slowMo)); const browser = new FFBrowser(connection); await connection.send('Target.enable'); - await browser._waitForTarget(t => t.type() === 'page'); return browser; } diff --git a/src/server/chromium.ts b/src/server/chromium.ts index 833965f998668..4aea2046fed84 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -22,7 +22,7 @@ import * as util from 'util'; import { BrowserFetcher, OnProgressCallback, BrowserFetcherOptions } from '../server/browserFetcher'; import { DeviceDescriptors } from '../deviceDescriptors'; import * as types from '../types'; -import { assert } from '../helper'; +import { assert, helper } from '../helper'; import { CRBrowser } from '../chromium/crBrowser'; import * as platform from '../platform'; import { TimeoutError } from '../errors'; @@ -63,8 +63,10 @@ export class Chromium implements BrowserType { } async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise { + const { timeout = 30000 } = options || {}; const { browserServer, transport } = await this._launchServer(options, 'persistent', userDataDir); const browser = await CRBrowser.connect(transport!); + await helper.waitWithTimeout(browser.waitForTarget(t => t.type() === 'page'), 'first page', timeout); // Hack: for typical launch scenario, ensure that close waits for actual process termination. const browserContext = browser._defaultContext; browserContext.close = () => browserServer.close(); diff --git a/src/server/firefox.ts b/src/server/firefox.ts index d894d0f607c2a..d797329dfdb99 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -27,7 +27,7 @@ import * as os from 'os'; import * as path from 'path'; import * as util from 'util'; import { TimeoutError } from '../errors'; -import { assert } from '../helper'; +import { assert, helper } from '../helper'; import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType'; import { ConnectOptions, LaunchType } from '../browser'; import { BrowserServer } from './browserServer'; @@ -73,8 +73,10 @@ export class Firefox implements BrowserType { } async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise { + const { timeout = 30000 } = options || {}; const { browserServer, transport } = await this._launchServer(options, 'persistent', userDataDir); const browser = await FFBrowser.connect(transport!); + await helper.waitWithTimeout(browser._waitForTarget(t => t.type() === 'page'), 'first page', timeout); // Hack: for typical launch scenario, ensure that close waits for actual process termination. const browserContext = browser._defaultContext; browserContext.close = () => browserServer.close(); diff --git a/src/server/webkit.ts b/src/server/webkit.ts index 9a3297ef3c2c9..721c1cba6dacc 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -28,7 +28,7 @@ import * as path from 'path'; import * as platform from '../platform'; import * as util from 'util'; import * as os from 'os'; -import { assert } from '../helper'; +import { assert, helper } from '../helper'; import { kBrowserCloseMessageId } from '../webkit/wkConnection'; import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType'; import { ConnectionTransport } from '../transport'; @@ -75,8 +75,10 @@ export class WebKit implements BrowserType { } async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise { + const { timeout = 30000 } = options || {}; const { browserServer, transport } = await this._launchServer(options, 'persistent', userDataDir); const browser = await WKBrowser.connect(transport!); + await helper.waitWithTimeout(browser._waitForFirstPageTarget(), 'first page', timeout); // Hack: for typical launch scenario, ensure that close waits for actual process termination. const browserContext = browser._defaultContext; browserContext.close = () => browserServer.close(); diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index c17ec2faf048e..6d958999775db 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -43,8 +43,6 @@ export class WKBrowser extends platform.EventEmitter implements Browser { static async connect(transport: ConnectionTransport, slowMo: number = 0): Promise { const browser = new WKBrowser(SlowMoTransport.wrap(transport, slowMo)); - // TODO: figure out the timeout. - await browser._waitForFirstPageTarget(30000); return browser; } @@ -93,9 +91,9 @@ export class WKBrowser extends platform.EventEmitter implements Browser { return createPageInNewContext(this, options); } - async _waitForFirstPageTarget(timeout: number): Promise { + async _waitForFirstPageTarget(): Promise { assert(!this._pageProxies.size); - await helper.waitWithTimeout(this._firstPageProxyPromise, 'firstPageProxy', timeout); + return this._firstPageProxyPromise; } _onPageProxyCreated(event: Protocol.Browser.pageProxyCreatedPayload) { diff --git a/test/chromium/chromium.spec.js b/test/chromium/chromium.spec.js index 6b169fe38204d..3234e8ef5620a 100644 --- a/test/chromium/chromium.spec.js +++ b/test/chromium/chromium.spec.js @@ -153,17 +153,15 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI expect(browser.pageTarget(page).opener()).toBe(null); }); it('should close all belonging targets once closing context', async function({browser, newContext}) { - const targets = async () => (await browser.targets()).filter(t => t.type() === 'page'); - // There is one page in a default profile and one page created by test harness. - expect((await targets()).length).toBe(2); + const targets = async (context) => (await browser.targets()).filter(t => t.type() === 'page' && t.context() === context); const context = await newContext(); await context.newPage(); - expect((await targets()).length).toBe(3); + expect((await targets(context)).length).toBe(1); expect((await context.pages()).length).toBe(1); await context.close(); - expect((await targets()).length).toBe(2); + expect((await targets(context)).length).toBe(0); }); }); diff --git a/test/chromium/launcher.spec.js b/test/chromium/launcher.spec.js index 0c96e30090500..b1bbc910d8440 100644 --- a/test/chromium/launcher.spec.js +++ b/test/chromium/launcher.spec.js @@ -57,11 +57,12 @@ module.exports.describe = function({testRunner, expect, defaultBrowserOptions, p describe('Browser target events', function() { it('should work', async({server}) => { const browser = await playwright.launch(defaultBrowserOptions); + const context = await browser.newContext(); const events = []; - browser.on('targetcreated', () => events.push('CREATED')); - browser.on('targetchanged', () => events.push('CHANGED')); - browser.on('targetdestroyed', () => events.push('DESTROYED')); - const page = await browser.newPage(); + browser.on('targetcreated', target => target.context() === context && events.push('CREATED')); + browser.on('targetchanged', target => target.context() === context && events.push('CHANGED')); + browser.on('targetdestroyed', target => target.context() === context && events.push('DESTROYED')); + const page = await context.newPage(); await page.goto(server.EMPTY_PAGE); await page.close(); expect(events).toEqual(['CREATED', 'CHANGED', 'DESTROYED']);