Revert "feat: do not wait for first page in non-persistent mode (#939)"

This reverts commit a567123596.

Reason for revert: WK-Win fails to start if we start talking over the
pipe too early.
This commit is contained in:
Andrey Lushnikov 2020-02-13 13:44:13 -08:00
parent c15534ff01
commit 71892b4aaa
8 changed files with 18 additions and 19 deletions

View File

@ -46,6 +46,7 @@ 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;
}

View File

@ -39,6 +39,7 @@ 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;
}

View File

@ -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, helper } from '../helper';
import { assert } from '../helper';
import { CRBrowser } from '../chromium/crBrowser';
import * as platform from '../platform';
import { TimeoutError } from '../errors';
@ -65,10 +65,8 @@ export class Chromium implements BrowserType {
}
async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise<BrowserContext> {
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();

View File

@ -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, helper } from '../helper';
import { assert } from '../helper';
import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType';
import { ConnectOptions, LaunchType } from '../browser';
import { BrowserServer } from './browserServer';
@ -75,10 +75,8 @@ export class Firefox implements BrowserType {
}
async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise<BrowserContext> {
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();

View File

@ -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, helper } from '../helper';
import { assert } from '../helper';
import { kBrowserCloseMessageId } from '../webkit/wkConnection';
import { LaunchOptions, BrowserArgOptions, BrowserType } from './browserType';
import { ConnectionTransport } from '../transport';
@ -77,10 +77,8 @@ export class WebKit implements BrowserType {
}
async launchPersistent(userDataDir: string, options?: LaunchOptions): Promise<BrowserContext> {
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();

View File

@ -43,6 +43,8 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
static async connect(transport: ConnectionTransport, slowMo: number = 0): Promise<WKBrowser> {
const browser = new WKBrowser(SlowMoTransport.wrap(transport, slowMo));
// TODO: figure out the timeout.
await browser._waitForFirstPageTarget(30000);
return browser;
}
@ -93,9 +95,9 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
return createPageInNewContext(this, options);
}
async _waitForFirstPageTarget(): Promise<void> {
async _waitForFirstPageTarget(timeout: number): Promise<void> {
assert(!this._pageProxies.size);
return this._firstPageProxyPromise;
await helper.waitWithTimeout(this._firstPageProxyPromise, 'firstPageProxy', timeout);
}
_onPageProxyCreated(event: Protocol.Browser.pageProxyCreatedPayload) {

View File

@ -153,15 +153,17 @@ 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 (context) => (await browser.targets()).filter(t => t.type() === 'page' && t.context() === context);
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 context = await newContext();
await context.newPage();
expect((await targets(context)).length).toBe(1);
expect((await targets()).length).toBe(3);
expect((await context.pages()).length).toBe(1);
await context.close();
expect((await targets(context)).length).toBe(0);
expect((await targets()).length).toBe(2);
});
});

View File

@ -57,12 +57,11 @@ 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', 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();
browser.on('targetcreated', () => events.push('CREATED'));
browser.on('targetchanged', () => events.push('CHANGED'));
browser.on('targetdestroyed', () => events.push('DESTROYED'));
const page = await browser.newPage();
await page.goto(server.EMPTY_PAGE);
await page.close();
expect(events).toEqual(['CREATED', 'CHANGED', 'DESTROYED']);