From c43de22193680b79fe38966c7c9efced3a06aa39 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 9 Mar 2020 12:32:42 -0700 Subject: [PATCH] chore(wk, ff): simplify target management (#1279) --- src/firefox/ffBrowser.ts | 169 ++++++++------------------------------- src/firefox/ffPage.ts | 52 +++++++++--- src/server/firefox.ts | 2 +- src/webkit/wkBrowser.ts | 16 ++-- src/webkit/wkPage.ts | 16 ++-- 5 files changed, 86 insertions(+), 169 deletions(-) diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index b92d260f0a..f6596cc857 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -24,19 +24,19 @@ import { Page, PageBinding, PageEvent } from '../page'; import * as platform from '../platform'; import { ConnectionTransport, SlowMoTransport } from '../transport'; import * as types from '../types'; -import { ConnectionEvents, FFConnection, FFSession, FFSessionEvents } from './ffConnection'; +import { ConnectionEvents, FFConnection } from './ffConnection'; import { headersArray } from './ffNetworkManager'; import { FFPage } from './ffPage'; import { Protocol } from './protocol'; -const kAttachedToTarget = Symbol('kAttachedToTarget'); - export class FFBrowser extends platform.EventEmitter implements Browser { _connection: FFConnection; - _targets: Map; + readonly _ffPages: Map; readonly _defaultContext: FFBrowserContext; readonly _contexts: Map; private _eventListeners: RegisteredListener[]; + readonly _firstPagePromise: Promise; + private _firstPageCallback = () => {}; static async connect(transport: ConnectionTransport, attachToDefaultContext: boolean, slowMo?: number): Promise { const connection = new FFConnection(SlowMoTransport.wrap(transport, slowMo)); @@ -48,7 +48,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser { constructor(connection: FFConnection) { super(); this._connection = connection; - this._targets = new Map(); + this._ffPages = new Map(); this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({})); this._contexts = new Map(); @@ -61,6 +61,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser { helper.addEventListener(this._connection, 'Browser.attachedToTarget', this._onAttachedToTarget.bind(this)), helper.addEventListener(this._connection, 'Browser.detachedFromTarget', this._onDetachedFromTarget.bind(this)), ]; + this._firstPagePromise = new Promise(f => this._firstPageCallback = f); } isConnected(): boolean { @@ -112,53 +113,26 @@ export class FFBrowser extends platform.EventEmitter implements Browser { return createPageInNewContext(this, options); } - async _waitForTarget(predicate: (target: Target) => boolean, options: { timeout?: number; } = {}): Promise { - const { - timeout = 30000 - } = options; - const existingTarget = this._allTargets().find(predicate); - if (existingTarget) - return existingTarget; - let resolve: (t: Target) => void; - const targetPromise = new Promise(x => resolve = x); - this.on(kAttachedToTarget, check); - try { - if (!timeout) - return await targetPromise; - return await helper.waitWithTimeout(targetPromise, 'target', timeout); - } finally { - this.removeListener(kAttachedToTarget, check); - } - - function check(target: Target) { - if (predicate(target)) - resolve(target); - } - } - - _allTargets() { - return Array.from(this._targets.values()); - } - _onDetachedFromTarget(payload: Protocol.Browser.detachedFromTargetPayload) { - const {targetId} = payload; - const target = this._targets.get(targetId)!; - this._targets.delete(targetId); - target._didClose(); + const ffPage = this._ffPages.get(payload.targetId)!; + this._ffPages.delete(payload.targetId); + ffPage.didClose(); } async _onAttachedToTarget(payload: Protocol.Browser.attachedToTargetPayload) { const {targetId, browserContextId, openerId, type} = payload.targetInfo; + assert(type === 'page'); const context = browserContextId ? this._contexts.get(browserContextId)! : this._defaultContext; - const target = new Target(this, context, type, '', openerId); - this._targets.set(targetId, target); - target._initPagePromise(this._connection.createSession(payload.sessionId, type)); + const session = this._connection.createSession(payload.sessionId, type); + const opener = openerId ? this._ffPages.get(openerId)! : null; + const ffPage = new FFPage(session, context, opener); + this._ffPages.set(targetId, ffPage); - const pageEvent = new PageEvent(target.pageOrError()); - target.context().emit(Events.BrowserContext.Page, pageEvent); - this.emit(kAttachedToTarget, target); + const pageEvent = new PageEvent(ffPage.pageOrError()); + context.emit(Events.BrowserContext.Page, pageEvent); + + ffPage.pageOrError().then(() => this._firstPageCallback()); - const opener = target.opener(); if (!opener) return; const openerPage = await opener.pageOrError(); @@ -179,83 +153,6 @@ export class FFBrowser extends platform.EventEmitter implements Browser { } } -class Target { - _pagePromise?: Promise; - _ffPage: FFPage | null = null; - private readonly _browser: FFBrowser; - private readonly _context: FFBrowserContext; - private readonly _type: 'page' | 'browser'; - _url: string; - private readonly _openerId: string | undefined; - private _session?: FFSession; - - constructor(browser: FFBrowser, context: FFBrowserContext, type: 'page' | 'browser', url: string, openerId: string | undefined) { - this._browser = browser; - this._context = context; - this._type = type; - this._url = url; - this._openerId = openerId; - } - - _didClose() { - if (this._session) - this._session.dispose(); - if (this._ffPage) - this._ffPage.didClose(); - } - - opener(): Target | null { - return this._openerId ? this._browser._targets.get(this._openerId)! : null; - } - - type(): 'page' | 'browser' { - return this._type; - } - - url() { - return this._url; - } - - context(): FFBrowserContext { - return this._context; - } - - async pageOrError(): Promise { - if (this._type !== 'page') - throw new Error(`Cannot create page for "${this._type}" target`); - return this._pagePromise!; - } - - _initPagePromise(session: FFSession) { - this._session = session; - this._pagePromise = new Promise(async f => { - this._ffPage = new FFPage(session, this._context, async () => { - const openerTarget = this.opener(); - if (!openerTarget) - return null; - const result = await openerTarget.pageOrError(); - if (result instanceof Page && !result.isClosed()) - return result; - return null; - }); - const page = this._ffPage._page; - session.once(FFSessionEvents.Disconnected, () => page._didDisconnect()); - let pageOrError: Page | Error; - try { - await this._ffPage._initialize(); - pageOrError = page; - } catch (e) { - pageOrError = e; - } - f(pageOrError); - }); - } - - browser() { - return this._browser; - } -} - export class FFBrowserContext extends BrowserContextBase { readonly _browser: FFBrowser; readonly _browserContextId: string | null; @@ -281,13 +178,12 @@ export class FFBrowserContext extends BrowserContextBase { await this.setHTTPCredentials(this._options.httpCredentials); } + _ffPages(): FFPage[] { + return Array.from(this._browser._ffPages.values()).filter(ffPage => ffPage._browserContext === this); + } + _existingPages(): Page[] { - const pages: Page[] = []; - for (const target of this._browser._allTargets()) { - if (target.context() === this && target._ffPage) - pages.push(target._ffPage._page); - } - return pages; + return this._ffPages().map(ffPage => ffPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } setDefaultNavigationTimeout(timeout: number) { @@ -299,24 +195,23 @@ export class FFBrowserContext extends BrowserContextBase { } async pages(): Promise { - const targets = this._browser._allTargets().filter(target => target.context() === this && target.type() === 'page'); - const pages = await Promise.all(targets.map(target => target.pageOrError())); - return pages.filter(page => page instanceof Page && !page.isClosed()) as Page[]; + const pagesOrErrors = await Promise.all(this._ffPages().map(ffPage => ffPage.pageOrError())); + return pagesOrErrors.filter(pageOrError => pageOrError instanceof Page && !pageOrError.isClosed()) as Page[]; } async newPage(): Promise { assertBrowserContextIsNotOwned(this); - const {targetId} = await this._browser._connection.send('Browser.newPage', { + const { targetId } = await this._browser._connection.send('Browser.newPage', { browserContextId: this._browserContextId || undefined }); - const target = this._browser._targets.get(targetId)!; - const result = await target.pageOrError(); - if (result instanceof Page) { - if (result.isClosed()) + const ffPage = this._browser._ffPages.get(targetId)!; + const pageOrError = await ffPage.pageOrError(); + if (pageOrError instanceof Page) { + if (pageOrError.isClosed()) throw new Error('Page has been closed.'); - return result; + return pageOrError; } - throw result; + throw pageOrError; } async cookies(urls?: string | string[]): Promise { diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index 96e093a519..e3201d6781 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -26,7 +26,7 @@ import { kScreenshotDuringNavigationError } from '../screenshotter'; import * as types from '../types'; import { getAccessibilityTree } from './ffAccessibility'; import { FFBrowserContext } from './ffBrowser'; -import { FFSession } from './ffConnection'; +import { FFSession, FFSessionEvents } from './ffConnection'; import { FFExecutionContext } from './ffExecutionContext'; import { RawKeyboardImpl, RawMouseImpl } from './ffInput'; import { FFNetworkManager, headersArray } from './ffNetworkManager'; @@ -40,17 +40,22 @@ export class FFPage implements PageDelegate { readonly _session: FFSession; readonly _page: Page; readonly _networkManager: FFNetworkManager; - private readonly _openerResolver: () => Promise; + readonly _browserContext: FFBrowserContext; + private _pagePromise: Promise; + private _pageCallback: (pageOrError: Page | Error) => void = () => {}; + private _initialized = false; + private readonly _opener: FFPage | null; private readonly _contextIdToContext: Map; private _eventListeners: RegisteredListener[]; private _workers = new Map(); - constructor(session: FFSession, browserContext: FFBrowserContext, openerResolver: () => Promise) { + constructor(session: FFSession, browserContext: FFBrowserContext, opener: FFPage | null) { this._session = session; - this._openerResolver = openerResolver; + this._opener = opener; this.rawKeyboard = new RawKeyboardImpl(session); this.rawMouse = new RawMouseImpl(session); this._contextIdToContext = new Map(); + this._browserContext = browserContext; this._page = new Page(this, browserContext); this._networkManager = new FFNetworkManager(session, this._page); this._page.on(Events.Page.FrameDetached, frame => this._removeContextsForFrame(frame)); @@ -75,16 +80,33 @@ export class FFPage implements PageDelegate { helper.addEventListener(this._session, 'Page.dispatchMessageFromWorker', this._onDispatchMessageFromWorker.bind(this)), helper.addEventListener(this._session, 'Page.crashed', this._onCrashed.bind(this)), ]; + this._pagePromise = new Promise(f => this._pageCallback = f); + session.once(FFSessionEvents.Disconnected, () => this._page._didDisconnect()); + this._initialize(); } async _initialize() { - await Promise.all([ - this._session.send('Page.addScriptToEvaluateOnNewDocument', { - script: '', - worldName: UTILITY_WORLD_NAME, - }), - new Promise(f => this._session.once('Page.ready', f)), - ]); + try { + await Promise.all([ + this._session.send('Page.addScriptToEvaluateOnNewDocument', { + script: '', + worldName: UTILITY_WORLD_NAME, + }), + new Promise(f => this._session.once('Page.ready', f)), + ]); + this._pageCallback(this._page); + } catch (e) { + this._pageCallback(e); + } + this._initialized = true; + } + + _initializedPage(): Page | null { + return this._initialized ? this._page : null; + } + + async pageOrError(): Promise { + return this._pagePromise; } _onExecutionContextCreated(payload: Protocol.Runtime.executionContextCreatedPayload) { @@ -249,6 +271,7 @@ export class FFPage implements PageDelegate { } didClose() { + this._session.dispose(); helper.removeEventListeners(this._eventListeners); this._networkManager.dispose(); this._page._didClose(); @@ -289,7 +312,12 @@ export class FFPage implements PageDelegate { } async opener(): Promise { - return await this._openerResolver(); + if (!this._opener) + return null; + const result = await this._opener.pageOrError(); + if (result instanceof Page && !result.isClosed()) + return result; + return null; } async reload(): Promise { diff --git a/src/server/firefox.ts b/src/server/firefox.ts index 7f04903f9c..5ebedff888 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -83,7 +83,7 @@ export class Firefox implements BrowserType { const browser = await platform.connectToWebsocket(browserServer.wsEndpoint()!, transport => { return FFBrowser.connect(transport, true); }); - await helper.waitWithTimeout(browser._waitForTarget(t => t.type() === 'page'), 'first page', timeout); + await helper.waitWithTimeout(browser._firstPagePromise, '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 532a1ca329..be6a39a5c5 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -139,7 +139,7 @@ export class WKBrowser extends platform.EventEmitter implements Browser { const wkPage = this._wkPages.get(pageProxyId); if (!wkPage) return; - wkPage.didClose(false); + wkPage.didClose(); wkPage.dispose(); this._wkPages.delete(pageProxyId); } @@ -202,16 +202,12 @@ export class WKBrowserContext extends BrowserContextBase { await this.setHTTPCredentials(this._options.httpCredentials); } + _wkPages(): WKPage[] { + return Array.from(this._browser._wkPages.values()).filter(wkPage => wkPage._browserContext === this); + } + _existingPages(): Page[] { - const pages: Page[] = []; - for (const wkPage of this._browser._wkPages.values()) { - if (wkPage._browserContext !== this) - continue; - const page = wkPage._initializedPage(); - if (page) - pages.push(page); - } - return pages; + return this._wkPages().map(wkPage => wkPage._initializedPage()).filter(pageOrNull => !!pageOrNull) as Page[]; } async pages(): Promise { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index 8e3b5aac15..6eff11d7d2 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -78,8 +78,8 @@ export class WKPage implements PageDelegate { this._pagePromise = new Promise(f => this._pagePromiseCallback = f); } - _initializedPage(): Page | undefined { - return this._initialized ? this._page : undefined; + _initializedPage(): Page | null { + return this._initialized ? this._page : null; } private async _initializePageProxySession() { @@ -179,21 +179,19 @@ export class WKPage implements PageDelegate { this._provisionalPage = null; } else if (this._session.sessionId === targetId) { this._session.dispose(); + helper.removeEventListeners(this._sessionListeners); if (crashed) - this.didClose(crashed); + this._page._didCrash(); } } - didClose(crashed: boolean) { - helper.removeEventListeners(this._sessionListeners); - if (crashed) - this._page._didCrash(); - else - this._page._didClose(); + didClose() { + this._page._didClose(); } dispose() { this._pageProxySession.dispose(); + helper.removeEventListeners(this._sessionListeners); helper.removeEventListeners(this._eventListeners); if (this._session) this._session.dispose();