From 65d45c18c34833ffdc5761331956561e0193d724 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 13 Jul 2020 16:03:24 -0700 Subject: [PATCH] feat(rpc): introduce Waiter for various waitFor implementations (#2935) Use it for waitForEvent and waitForLoadState. --- src/frames.ts | 15 ++-- src/rpc/channels.ts | 7 +- src/rpc/client/browserContext.ts | 30 ++++--- src/rpc/client/frame.ts | 43 +++++++++- src/rpc/client/page.ts | 61 ++++----------- src/rpc/client/waiter.ts | 91 ++++++++++++++++++++++ src/rpc/server/browserContextDispatcher.ts | 10 +-- src/rpc/server/elementHandlerDispatcher.ts | 1 - src/rpc/server/frameDispatcher.ts | 16 ++-- test/environments.js | 3 +- 10 files changed, 182 insertions(+), 95 deletions(-) create mode 100644 src/rpc/client/waiter.ts diff --git a/src/frames.ts b/src/frames.ts index cbb5363164..526da04a46 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -57,7 +57,8 @@ type NavigationEvent = { documentInfo?: DocumentInfo, // Undefined for same-document navigations. error?: Error, }; -export const kLifecycleEvent = Symbol('lifecycle'); +export const kAddLifecycleEvent = Symbol('addLifecycle'); +export const kRemoveLifecycleEvent = Symbol('removeLifecycle'); export class FrameManager { private _page: Page; @@ -380,7 +381,7 @@ export class Frame { for (const event of events) { // Checking whether we have already notified about this event. if (!this._subtreeLifecycleEvents.has(event)) { - this._eventEmitter.emit(kLifecycleEvent, event); + this._eventEmitter.emit(kAddLifecycleEvent, event); if (this === mainFrame && this._url !== 'about:blank') this._page._logger.info(` "${event}" event fired`); if (this === mainFrame && event === 'load') @@ -389,6 +390,10 @@ export class Frame { this._page.emit(Events.Page.DOMContentLoaded); } } + for (const event of this._subtreeLifecycleEvents) { + if (!events.has(event)) + this._eventEmitter.emit(kRemoveLifecycleEvent, event); + } this._subtreeLifecycleEvents = events; } @@ -429,7 +434,7 @@ export class Frame { } if (!this._subtreeLifecycleEvents.has(waitUntil)) - await helper.waitForEvent(progress, this._eventEmitter, kLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; + await helper.waitForEvent(progress, this._eventEmitter, kAddLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; const request = event.documentInfo ? event.documentInfo.request : undefined; return request ? request._finalRequest().response() : null; @@ -453,7 +458,7 @@ export class Frame { throw navigationEvent.error; if (!this._subtreeLifecycleEvents.has(waitUntil)) - await helper.waitForEvent(progress, this._eventEmitter, kLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; + await helper.waitForEvent(progress, this._eventEmitter, kAddLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; const request = navigationEvent.documentInfo ? navigationEvent.documentInfo.request : undefined; return request ? request._finalRequest().response() : null; @@ -467,7 +472,7 @@ export class Frame { async _waitForLoadState(progress: Progress, state: types.LifecycleEvent): Promise { const waitUntil = verifyLifecycle(state); if (!this._subtreeLifecycleEvents.has(waitUntil)) - await helper.waitForEvent(progress, this._eventEmitter, kLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; + await helper.waitForEvent(progress, this._eventEmitter, kAddLifecycleEvent, (e: types.LifecycleEvent) => e === waitUntil).promise; } async frameElement(): Promise { diff --git a/src/rpc/channels.ts b/src/rpc/channels.ts index 6df884c811..930781fdf8 100644 --- a/src/rpc/channels.ts +++ b/src/rpc/channels.ts @@ -92,7 +92,6 @@ export interface BrowserContextChannel extends Channel { setHTTPCredentials(params: { httpCredentials: types.Credentials | null }): Promise; setNetworkInterceptionEnabled(params: { enabled: boolean }): Promise; setOffline(params: { offline: boolean }): Promise; - waitForEvent(params: { event: string }): Promise; on(event: 'crBackgroundPage', callback: (params: PageChannel) => void): this; on(event: 'crServiceWorker', callback: (params: WorkerChannel) => void): this; @@ -169,6 +168,8 @@ export type PageInitializer = { export type PageAttribution = { isPage?: boolean }; export interface FrameChannel extends Channel { + on(event: 'loadstate', callback: (params: { add?: types.LifecycleEvent, remove?: types.LifecycleEvent }) => void): this; + evalOnSelector(params: { selector: string; expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise; evalOnSelectorAll(params: { selector: string; expression: string, isFunction: boolean, arg: any} & PageAttribution): Promise; addScriptTag(params: { url?: string | undefined, path?: string | undefined, content?: string | undefined, type?: string | undefined} & PageAttribution): Promise; @@ -199,14 +200,14 @@ export interface FrameChannel extends Channel { type(params: { selector: string, text: string, delay?: number, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise; uncheck(params: { selector: string, force?: boolean, noWaitAfter?: boolean } & types.TimeoutOptions & PageAttribution): Promise; waitForFunction(params: { expression: string, isFunction: boolean, arg: any } & types.WaitForFunctionOptions & PageAttribution): Promise; - waitForLoadState(params: { state: types.LifecycleEvent } & types.TimeoutOptions & PageAttribution): Promise; waitForNavigation(params: types.WaitForNavigationOptions & PageAttribution): Promise; waitForSelector(params: { selector: string } & types.WaitForElementOptions & PageAttribution): Promise; } export type FrameInitializer = { url: string, name: string, - parentFrame: FrameChannel | null + parentFrame: FrameChannel | null, + loadStates: types.LifecycleEvent[], }; diff --git a/src/rpc/client/browserContext.ts b/src/rpc/client/browserContext.ts index 872357209e..a82978f587 100644 --- a/src/rpc/client/browserContext.ts +++ b/src/rpc/client/browserContext.ts @@ -16,7 +16,7 @@ */ import * as frames from './frame'; -import { Page, BindingCall, waitForEvent } from './page'; +import { Page, BindingCall } from './page'; import * as types from '../../types'; import * as network from './network'; import { BrowserContextChannel, BrowserContextInitializer } from '../channels'; @@ -26,6 +26,8 @@ import { Browser } from './browser'; import { Events } from '../../events'; import { TimeoutSettings } from '../../timeoutSettings'; import { BrowserType } from './browserType'; +import { Waiter } from './waiter'; +import { TimeoutError } from '../../errors'; export class BrowserContext extends ChannelOwner { _pages = new Set(); @@ -33,7 +35,6 @@ export class BrowserContext extends ChannelOwner(); - private _pendingWaitForEvents = new Map<(error: Error) => void, string>(); _timeoutSettings = new TimeoutSettings(); _ownerPage: Page | undefined; private _isClosedOrClosing = false; @@ -87,6 +88,7 @@ export class BrowserContext extends ChannelOwner { - const hasTimeout = optionsOrPredicate && !(optionsOrPredicate instanceof Function); - let reject: () => void; - const result = await Promise.race([ - waitForEvent(this, event, optionsOrPredicate, this._timeoutSettings.timeout(hasTimeout ? optionsOrPredicate as any : {})), - new Promise((f, r) => { reject = r; this._pendingWaitForEvents.set(reject, event); }) - ]); - this._pendingWaitForEvents.delete(reject!); + async waitForEvent(event: string, optionsOrPredicate: types.WaitForEventOptions = {}): Promise { + const timeout = this._timeoutSettings.timeout(optionsOrPredicate instanceof Function ? {} : optionsOrPredicate); + const predicate = optionsOrPredicate instanceof Function ? optionsOrPredicate : optionsOrPredicate.predicate; + const waiter = new Waiter(); + waiter.rejectOnTimeout(timeout, new TimeoutError(`Timeout while waiting for event "${event}"`)); + if (event !== Events.BrowserContext.Close) + waiter.rejectOnEvent(this, Events.BrowserContext.Close, new Error('Context closed')); + const result = await waiter.waitForEvent(this, event, predicate as any); + waiter.dispose(); return result; } @@ -192,13 +195,6 @@ export class BrowserContext extends ChannelOwner any; export class Frame extends ChannelOwner { + _eventEmitter: EventEmitter; + _loadStates: Set; _parentFrame: Frame | null = null; _url = ''; _name = ''; @@ -50,23 +55,45 @@ export class Frame extends ChannelOwner { constructor(parent: ChannelOwner, type: string, guid: string, initializer: FrameInitializer) { super(parent, type, guid, initializer); + this._eventEmitter = new EventEmitter(); + this._eventEmitter.setMaxListeners(0); this._parentFrame = Frame.fromNullable(initializer.parentFrame); if (this._parentFrame) this._parentFrame._childFrames.add(this); this._name = initializer.name; this._url = initializer.url; + this._loadStates = new Set(initializer.loadStates); + this._channel.on('loadstate', event => { + if (event.add) { + this._loadStates.add(event.add); + this._eventEmitter.emit('loadstate', event.add); + } + if (event.remove) + this._loadStates.delete(event.remove); + }); } async goto(url: string, options: GotoOptions = {}): Promise { - return Response.fromNullable(await this._channel.goto({ url, ...options, isPage: this._page!._isPageCall })); + return network.Response.fromNullable(await this._channel.goto({ url, ...options, isPage: this._page!._isPageCall })); } async waitForNavigation(options: types.WaitForNavigationOptions = {}): Promise { - return Response.fromNullable(await this._channel.waitForNavigation({ ...options, isPage: this._page!._isPageCall })); + return network.Response.fromNullable(await this._channel.waitForNavigation({ ...options, isPage: this._page!._isPageCall })); } async waitForLoadState(state: types.LifecycleEvent = 'load', options: types.TimeoutOptions = {}): Promise { - await this._channel.waitForLoadState({ state, ...options, isPage: this._page!._isPageCall }); + state = verifyLoadState(state); + if (this._loadStates.has(state)) + return; + const timeout = this._page!._timeoutSettings.navigationTimeout(options); + const apiName = this._page!._isPageCall ? 'page.waitForLoadState' : 'frame.waitForLoadState'; + const waiter = new Waiter(); + waiter.rejectOnEvent(this._page!, Events.Page.Close, new Error('Navigation failed because page was closed!')); + waiter.rejectOnEvent(this._page!, Events.Page.Crash, new Error('Navigation failed because page crashed!')); + waiter.rejectOnEvent(this._page!, Events.Page.FrameDetached, new Error('Navigating frame was detached!'), frame => frame === this); + waiter.rejectOnTimeout(timeout, new TimeoutError(`Timeout ${timeout}ms exceeded during ${apiName}.`)); + await waiter.waitForEvent(this._eventEmitter, 'loadstate', s => s === state); + waiter.dispose(); } async frameElement(): Promise { @@ -228,3 +255,11 @@ export class Frame extends ChannelOwner { return await this._channel.title(); } } + +function verifyLoadState(waitUntil: types.LifecycleEvent): types.LifecycleEvent { + if (waitUntil as unknown === 'networkidle0') + waitUntil = 'networkidle'; + if (!types.kLifecycleEvents.has(waitUntil)) + throw new Error(`Unsupported waitUntil option ${String(waitUntil)}`); + return waitUntil; +} diff --git a/src/rpc/client/page.ts b/src/rpc/client/page.ts index d63a44db31..8ff502e117 100644 --- a/src/rpc/client/page.ts +++ b/src/rpc/client/page.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { EventEmitter } from 'events'; import { TimeoutError } from '../../errors'; import { Events } from '../../events'; import { assert, assertMaxArguments, helper, Listener } from '../../helper'; @@ -38,6 +37,7 @@ import { Request, Response, Route, RouteHandler } from './network'; import { FileChooser } from './fileChooser'; import { Buffer } from 'buffer'; import { Coverage } from './coverage'; +import { Waiter } from './waiter'; export class Page extends ChannelOwner { private _browserContext: BrowserContext; @@ -57,8 +57,7 @@ export class Page extends ChannelOwner { pdf?: (options?: types.PDFOptions) => Promise; readonly _bindings = new Map(); - private _pendingWaitForEvents = new Map<(error: Error) => void, string>(); - private _timeoutSettings: TimeoutSettings; + readonly _timeoutSettings: TimeoutSettings; _isPageCall = false; static from(page: PageChannel): Page { @@ -166,26 +165,13 @@ export class Page extends ChannelOwner { private _onClose() { this._closed = true; this._browserContext._pages.delete(this); - this._rejectPendingOperations(false); this.emit(Events.Page.Close); } private _onCrash() { - this._rejectPendingOperations(true); this.emit(Events.Page.Crash); } - private _rejectPendingOperations(isCrash: boolean) { - for (const [listener, event] of this._pendingWaitForEvents) { - if (event === Events.Page.Close && !isCrash) - continue; - if (event === Events.Page.Crash && isCrash) - continue; - listener(new Error(isCrash ? 'Page crashed' : 'Page closed')); - } - this._pendingWaitForEvents.clear(); - } - context(): BrowserContext { return this._browserContext; } @@ -214,6 +200,7 @@ export class Page extends ChannelOwner { } setDefaultNavigationTimeout(timeout: number) { + this._timeoutSettings.setDefaultNavigationTimeout(timeout); this._channel.setDefaultNavigationTimeoutNoReply({ timeout }); } @@ -340,12 +327,16 @@ export class Page extends ChannelOwner { } async waitForEvent(event: string, optionsOrPredicate: types.WaitForEventOptions = {}): Promise { - let reject: () => void; - const result = await Promise.race([ - waitForEvent(this, event, optionsOrPredicate, this._timeoutSettings.timeout(optionsOrPredicate instanceof Function ? {} : optionsOrPredicate)), - new Promise((f, r) => { reject = r; this._pendingWaitForEvents.set(reject, event); }) - ]); - this._pendingWaitForEvents.delete(reject!); + const timeout = this._timeoutSettings.timeout(optionsOrPredicate instanceof Function ? {} : optionsOrPredicate); + const predicate = optionsOrPredicate instanceof Function ? optionsOrPredicate : optionsOrPredicate.predicate; + const waiter = new Waiter(); + waiter.rejectOnTimeout(timeout, new TimeoutError(`Timeout while waiting for event "${event}"`)); + if (event !== Events.Page.Crash) + waiter.rejectOnEvent(this, Events.Page.Crash, new Error('Page crashed')); + if (event !== Events.Page.Close) + waiter.rejectOnEvent(this, Events.Page.Close, new Error('Page closed')); + const result = await waiter.waitForEvent(this, event, predicate as any); + waiter.dispose(); return result; } @@ -543,29 +534,3 @@ export class BindingCall extends ChannelOwner { - let predicate: Function | undefined; - let timeout = defaultTimeout; - if (typeof optionsOrPredicate === 'function') { - predicate = optionsOrPredicate; - } else if (optionsOrPredicate.predicate) { - if (optionsOrPredicate.timeout !== undefined) - timeout = optionsOrPredicate.timeout; - predicate = optionsOrPredicate.predicate; - } - let callback: (a: any) => void; - const result = new Promise(f => callback = f); - const listener = helper.addEventListener(emitter, event, param => { - if (predicate && !predicate(param)) - return; - callback(param); - helper.removeEventListeners([listener]); - }); - if (timeout === 0) - return result; - return Promise.race([ - result, - new Promise((f, r) => setTimeout(() => r(new TimeoutError('Timeout while waiting for event')), timeout)) - ]); -} diff --git a/src/rpc/client/waiter.ts b/src/rpc/client/waiter.ts new file mode 100644 index 0000000000..62eff42f83 --- /dev/null +++ b/src/rpc/client/waiter.ts @@ -0,0 +1,91 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { EventEmitter } from 'events'; + +export class Waiter { + private _dispose: (() => void)[] = []; + private _failures: Promise[] = []; + + async waitForEvent(emitter: EventEmitter, event: string, predicate?: (arg: T) => boolean): Promise { + const { promise, dispose } = waitForEvent(emitter, event, predicate); + return this._wait(promise, dispose); + } + + rejectOnEvent(emitter: EventEmitter, event: string, error: Error, predicate?: (arg: T) => boolean) { + const { promise, dispose } = waitForEvent(emitter, event, predicate); + this._rejectOn(promise.then(() => { throw error; }), dispose); + } + + rejectOnTimeout(timeout: number, error: Error) { + if (!timeout) + return; + const { promise, dispose } = waitForTimeout(timeout); + this._rejectOn(promise.then(() => { throw error; }), dispose); + } + + dispose() { + for (const dispose of this._dispose) + dispose(); + } + + private async _wait(promise: Promise, dispose?: () => void): Promise { + try { + const result = await Promise.race([promise, ...this._failures]); + if (dispose) + dispose(); + return result; + } catch (e) { + if (dispose) + dispose(); + this.dispose(); + throw e; + } + } + + private _rejectOn(promise: Promise, dispose?: () => void) { + this._failures.push(promise); + if (dispose) + this._dispose.push(dispose); + } +} + +function waitForEvent(emitter: EventEmitter, event: string, predicate?: (arg: T) => boolean): { promise: Promise, dispose: () => void } { + let listener: (eventArg: any) => void; + const promise = new Promise((resolve, reject) => { + listener = (eventArg: any) => { + try { + if (predicate && !predicate(eventArg)) + return; + emitter.removeListener(event, listener); + resolve(eventArg); + } catch (e) { + emitter.removeListener(event, listener); + reject(e); + } + }; + emitter.addListener(event, listener); + }); + const dispose = () => emitter.removeListener(event, listener); + return { promise, dispose }; +} + +function waitForTimeout(timeout: number): { promise: Promise, dispose: () => void } { + let timeoutId: number; + const promise = new Promise(resolve => timeoutId = setTimeout(resolve, timeout)); + const dispose = () => clearTimeout(timeoutId); + return { promise, dispose }; +} diff --git a/src/rpc/server/browserContextDispatcher.ts b/src/rpc/server/browserContextDispatcher.ts index c3d7e3fe6e..49ee7c583a 100644 --- a/src/rpc/server/browserContextDispatcher.ts +++ b/src/rpc/server/browserContextDispatcher.ts @@ -17,11 +17,10 @@ import * as types from '../../types'; import { BrowserContextBase, BrowserContext } from '../../browserContext'; import { Events } from '../../events'; -import { Dispatcher, DispatcherScope, lookupNullableDispatcher, lookupDispatcher } from './dispatcher'; +import { Dispatcher, DispatcherScope, lookupDispatcher } from './dispatcher'; import { PageDispatcher, BindingCallDispatcher, WorkerDispatcher } from './pageDispatcher'; import { PageChannel, BrowserContextChannel, BrowserContextInitializer, CDPSessionChannel } from '../channels'; import { RouteDispatcher, RequestDispatcher } from './networkDispatchers'; -import { Page } from '../../page'; import { CRBrowserContext } from '../../chromium/crBrowser'; import { CDPSessionDispatcher } from './cdpSessionDispatcher'; import { Events as ChromiumEvents } from '../../chromium/events'; @@ -121,13 +120,6 @@ export class BrowserContextDispatcher extends Dispatcher { - const result = await this._context.waitForEvent(params.event); - if (result instanceof Page) - return lookupNullableDispatcher(result); - return result; - } - async close(): Promise { await this._context.close(); } diff --git a/src/rpc/server/elementHandlerDispatcher.ts b/src/rpc/server/elementHandlerDispatcher.ts index 16e82f781e..e06f4de11e 100644 --- a/src/rpc/server/elementHandlerDispatcher.ts +++ b/src/rpc/server/elementHandlerDispatcher.ts @@ -38,7 +38,6 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme constructor(scope: DispatcherScope, elementHandle: ElementHandle) { super(scope, elementHandle); this._elementHandle = elementHandle; - this._elementHandle = elementHandle; } async ownerFrame(): Promise { diff --git a/src/rpc/server/frameDispatcher.ts b/src/rpc/server/frameDispatcher.ts index d8b7b697b8..12ca6fc514 100644 --- a/src/rpc/server/frameDispatcher.ts +++ b/src/rpc/server/frameDispatcher.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Frame } from '../../frames'; +import { Frame, kAddLifecycleEvent, kRemoveLifecycleEvent } from '../../frames'; import * as types from '../../types'; import { ElementHandleChannel, FrameChannel, FrameInitializer, JSHandleChannel, ResponseChannel, PageAttribution } from '../channels'; import { Dispatcher, DispatcherScope, lookupNullableDispatcher, existingDispatcher } from './dispatcher'; @@ -34,9 +34,16 @@ export class FrameDispatcher extends Dispatcher impleme super(scope, frame, 'frame', { url: frame.url(), name: frame.name(), - parentFrame: lookupNullableDispatcher(frame.parentFrame()) + parentFrame: lookupNullableDispatcher(frame.parentFrame()), + loadStates: Array.from(frame._subtreeLifecycleEvents), }); this._frame = frame; + frame._eventEmitter.on(kAddLifecycleEvent, (event: types.LifecycleEvent) => { + this._dispatchEvent('loadstate', { add: event }); + }); + frame._eventEmitter.on(kRemoveLifecycleEvent, (event: types.LifecycleEvent) => { + this._dispatchEvent('loadstate', { remove: event }); + }); } async goto(params: { url: string } & types.GotoOptions & PageAttribution): Promise { @@ -44,11 +51,6 @@ export class FrameDispatcher extends Dispatcher impleme return lookupNullableDispatcher(await target.goto(params.url, params)); } - async waitForLoadState(params: { state?: 'load' | 'domcontentloaded' | 'networkidle' } & types.TimeoutOptions & PageAttribution): Promise { - const target = params.isPage ? this._frame._page : this._frame; - await target.waitForLoadState(params.state, params); - } - async waitForNavigation(params: types.WaitForNavigationOptions & PageAttribution): Promise { const target = params.isPage ? this._frame._page : this._frame; return lookupNullableDispatcher(await target.waitForNavigation(params)); diff --git a/test/environments.js b/test/environments.js index 31f481c7e1..02c053a7f1 100644 --- a/test/environments.js +++ b/test/environments.js @@ -125,7 +125,8 @@ class TraceTestEnvironment { this._session = null; } - async beforeEach() { + async beforeEach(state, testRun) { + const t = testRun.test(); const inspector = require('inspector'); const fs = require('fs'); const util = require('util');