From 2f45ebbb723139a44f0b67d30419a883ade19215 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Sat, 4 Jan 2020 10:12:40 -0800 Subject: [PATCH] chore(webkit): remove WKTarget._type, simplify initialization (#376) --- src/webkit/wkBrowser.ts | 53 ++++++++++++++-------------------------- src/webkit/wkLauncher.ts | 2 +- src/webkit/wkTarget.ts | 9 +++---- 3 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 9a58b2328b..c465a4d553 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { EventEmitter } from 'events'; import { helper, RegisteredListener, debugError, assert } from '../helper'; import * as browser from '../browser'; import * as network from '../network'; @@ -30,11 +29,13 @@ import { ConnectionTransport } from '../transport'; export class WKBrowser extends browser.Browser { readonly _connection: WKConnection; - private _defaultContext: BrowserContext; - private _contexts = new Map(); - _targets = new Map(); - private _eventListeners: RegisteredListener[]; - private _privateEvents = new EventEmitter(); + private readonly _defaultContext: BrowserContext; + private readonly _contexts = new Map(); + private readonly _targets = new Map(); + private readonly _eventListeners: RegisteredListener[]; + + private _firstTargetCallback?: () => void; + private readonly _firstTargetPromise: Promise; constructor(transport: ConnectionTransport) { super(); @@ -53,6 +54,8 @@ export class WKBrowser extends browser.Browser { helper.addEventListener(this._connection, WKConnectionEvents.DidCommitProvisionalTarget, this._onProvisionalTargetCommitted.bind(this)), ]; + this._firstTargetPromise = new Promise(resolve => this._firstTargetCallback = resolve); + // Intercept provisional targets during cross-process navigation. this._connection.send('Target.setPauseOnStart', { pauseOnStart: true }).catch(e => { debugError(e); @@ -77,31 +80,13 @@ export class WKBrowser extends browser.Browser { return this._defaultContext; } - async _waitForTarget(predicate: (arg0: WKTarget) => boolean, options: { timeout?: number; } | undefined = {}): Promise { - const { - timeout = 30000 - } = options; - const existingTarget = Array.from(this._targets.values()).find(predicate); - if (existingTarget) - return existingTarget; - let resolve : (a: WKTarget) => void; - const targetPromise = new Promise(x => resolve = x); - this._privateEvents.on(BrowserEvents.TargetCreated, check); - try { - if (!timeout) - return await targetPromise; - return await helper.waitWithTimeout(targetPromise, 'target', timeout); - } finally { - this._privateEvents.removeListener(BrowserEvents.TargetCreated, check); - } - - function check(target: WKTarget) { - if (predicate(target)) - resolve(target); - } + async _waitForFirstPageTarget(timeout: number): Promise { + assert(!this._targets.size); + await helper.waitWithTimeout(this._firstTargetPromise, 'target', timeout); } _onTargetCreated(session: WKTargetSession, targetInfo: Protocol.Target.TargetInfo) { + assert(targetInfo.type === 'page', 'Only page targets are expected in WebKit, received: ' + targetInfo.type); let context = null; if (targetInfo.browserContextId) { // FIXME: we don't know about the default context id, so assume that all targets from @@ -121,7 +106,10 @@ export class WKBrowser extends browser.Browser { if (oldTarget) oldTarget._initializeSession(session); } - this._privateEvents.emit(BrowserEvents.TargetCreated, target); + if (this._firstTargetCallback) { + this._firstTargetCallback(); + this._firstTargetCallback = null; + } if (!targetInfo.oldTargetId && targetInfo.openerId) { const opener = this._targets.get(targetInfo.openerId); if (!opener) @@ -170,7 +158,7 @@ export class WKBrowser extends browser.Browser { _createBrowserContext(browserContextId: string | undefined, options: BrowserContextOptions): BrowserContext { const context = new BrowserContext({ pages: async (): Promise => { - const targets = Array.from(this._targets.values()).filter(target => target._browserContext === context && target._type === 'page'); + const targets = Array.from(this._targets.values()).filter(target => target._browserContext === context && !target._session.isProvisional()); const pages = await Promise.all(targets.map(target => target.page())); return pages.filter(page => !!page); }, @@ -230,8 +218,3 @@ export class WKBrowser extends browser.Browser { return context; } } - -const BrowserEvents = { - TargetCreated: Symbol('BrowserEvents.TargetCreated'), - TargetDestroyed: Symbol('BrowserEvents.TargetDestroyed'), -}; diff --git a/src/webkit/wkLauncher.ts b/src/webkit/wkLauncher.ts index 559235c202..c4050b124c 100644 --- a/src/webkit/wkLauncher.ts +++ b/src/webkit/wkLauncher.ts @@ -99,7 +99,7 @@ export class WKLauncher { try { const transport = new PipeTransport(launchedProcess.stdio[3] as NodeJS.WritableStream, launchedProcess.stdio[4] as NodeJS.ReadableStream); browser = new WKBrowser(SlowMoTransport.wrap(transport, slowMo)); - await browser._waitForTarget(t => t._type === 'page', {timeout}); + await browser._waitForFirstPageTarget(timeout); return new BrowserServer(browser, launchedProcess, ''); } catch (e) { if (browser) diff --git a/src/webkit/wkTarget.ts b/src/webkit/wkTarget.ts index 27706154e1..82ec88bdb2 100644 --- a/src/webkit/wkTarget.ts +++ b/src/webkit/wkTarget.ts @@ -25,19 +25,16 @@ import { WKBrowser } from './wkBrowser'; export class WKTarget { readonly _browserContext: BrowserContext; readonly _targetId: string; - readonly _type: 'page' | 'service-worker' | 'worker'; - private readonly _session: WKTargetSession; + readonly _session: WKTargetSession; private _pagePromise: Promise | null = null; private _browser: WKBrowser; _wkPage: WKPage | null = null; constructor(browser: WKBrowser, session: WKTargetSession, targetInfo: Protocol.Target.TargetInfo, browserContext: BrowserContext) { - const {targetId, type} = targetInfo; this._browser = browser; this._session = session; this._browserContext = browserContext; - this._targetId = targetId; - this._type = type; + this._targetId = targetInfo.targetId; /** @type {?Promise} */ this._pagePromise = null; } @@ -80,7 +77,7 @@ export class WKTarget { } async page(): Promise { - if (this._type === 'page' && !this._pagePromise) { + if (!this._pagePromise) { this._wkPage = new WKPage(this._browser, this._browserContext); // Reference local page variable as |this._frameManager| may be // cleared on swap.