From 9e85f8d856252605bc1c3f0f01b40569f3a8a39c Mon Sep 17 00:00:00 2001 From: Joel Einbinder Date: Tue, 31 Mar 2020 16:18:49 -0700 Subject: [PATCH] chore(waitForEvent): refactor waitForEvent into a single implementation (#1602) Moves the `waitForEvent` implementation into an `ExtendedEventEmitter` class. This is step one if we want to add `waitForEvent` to `Worker`, `Browser`, and `BrowserServer` objects. All of these only have a 'close' event, but I still feel we should be consistent with our event emitters. --- src/browserContext.ts | 23 ++++++++++------------- src/extendedEventEmitter.ts | 36 ++++++++++++++++++++++++++++++++++++ src/page.ts | 18 ++++++++++-------- 3 files changed, 56 insertions(+), 21 deletions(-) create mode 100644 src/extendedEventEmitter.ts diff --git a/src/browserContext.ts b/src/browserContext.ts index 568cd179fc..627050536c 100644 --- a/src/browserContext.ts +++ b/src/browserContext.ts @@ -18,10 +18,10 @@ import { helper } from './helper'; import * as network from './network'; import { Page, PageBinding } from './page'; -import * as platform from './platform'; import { TimeoutSettings } from './timeoutSettings'; import * as types from './types'; import { Events } from './events'; +import { ExtendedEventEmitter } from './extendedEventEmitter'; export type BrowserContextOptions = { viewport?: types.Size | null, @@ -62,7 +62,7 @@ export interface BrowserContext { close(): Promise; } -export abstract class BrowserContextBase extends platform.EventEmitter implements BrowserContext { +export abstract class BrowserContextBase extends ExtendedEventEmitter implements BrowserContext { readonly _timeoutSettings = new TimeoutSettings(); readonly _pageBindings = new Map(); readonly _options: BrowserContextOptions; @@ -78,6 +78,14 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement this._closePromise = new Promise(fulfill => this._closePromiseFulfill = fulfill); } + protected _abortPromiseForEvent(event: string) { + return event === Events.BrowserContext.Close ? super._abortPromiseForEvent(event) : this._closePromise; + } + + protected _timeoutForEvent(event: string): number { + return this._timeoutSettings.timeout(); + } + _browserClosed() { for (const page of this.pages()) page._didClose(); @@ -132,17 +140,6 @@ export abstract class BrowserContextBase extends platform.EventEmitter implement setDefaultTimeout(timeout: number) { this._timeoutSettings.setDefaultTimeout(timeout); } - - async waitForEvent(event: string, optionsOrPredicate?: Function | (types.TimeoutOptions & { predicate?: Function })): Promise { - if (!optionsOrPredicate) - optionsOrPredicate = {}; - if (typeof optionsOrPredicate === 'function') - optionsOrPredicate = { predicate: optionsOrPredicate }; - const { timeout = this._timeoutSettings.timeout(), predicate = () => true } = optionsOrPredicate; - - const abortPromise = (event === Events.BrowserContext.Close) ? new Promise(() => { }) : this._closePromise; - return helper.waitForEvent(this, event, (...args: any[]) => !!predicate(...args), timeout, abortPromise); - } } export function assertBrowserContextIsNotOwned(context: BrowserContextBase) { diff --git a/src/extendedEventEmitter.ts b/src/extendedEventEmitter.ts new file mode 100644 index 0000000000..49ae8f339b --- /dev/null +++ b/src/extendedEventEmitter.ts @@ -0,0 +1,36 @@ +/** + * 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 './platform'; +import { helper } from './helper'; + +export class ExtendedEventEmitter extends EventEmitter { + protected _abortPromiseForEvent(event: string) { + return new Promise(() => void 0); + } + + protected _timeoutForEvent(event: string): number { + throw new Error('unimplemented'); + } + + async waitForEvent(event: string, optionsOrPredicate: Function|{ predicate?: Function, timeout?: number } = {}): Promise { + const { + predicate = () => true, + timeout = this._timeoutForEvent(event) + } = typeof optionsOrPredicate === 'function' ? {predicate: optionsOrPredicate} : optionsOrPredicate; + return helper.waitForEvent(this, event, predicate, timeout, this._abortPromiseForEvent(event)); + } +} diff --git a/src/page.ts b/src/page.ts index c6dd32f7f0..b5c88dc898 100644 --- a/src/page.ts +++ b/src/page.ts @@ -29,6 +29,7 @@ import { BrowserContext, BrowserContextBase } from './browserContext'; import { ConsoleMessage, ConsoleMessageLocation } from './console'; import * as accessibility from './accessibility'; import * as platform from './platform'; +import { ExtendedEventEmitter } from './extendedEventEmitter'; export interface PageDelegate { readonly rawMouse: input.RawMouse; @@ -87,7 +88,7 @@ export type FileChooser = { multiple: boolean }; -export class Page extends platform.EventEmitter { +export class Page extends ExtendedEventEmitter { private _closed = false; private _closedCallback: () => void; private _closedPromise: Promise; @@ -142,6 +143,14 @@ export class Page extends platform.EventEmitter { this.coverage = delegate.coverage ? delegate.coverage() : null; } + protected _abortPromiseForEvent(event: string) { + return this._disconnectedPromise; + } + + protected _timeoutForEvent(event: string): number { + return this._timeoutSettings.timeout(); + } + _didClose() { assert(!this._closed, 'Page closed twice'); this._closed = true; @@ -303,13 +312,6 @@ export class Page extends platform.EventEmitter { return this.mainFrame().waitForNavigation(options); } - async waitForEvent(event: string, optionsOrPredicate: Function | (types.TimeoutOptions & { predicate?: Function }) = {}): Promise { - if (typeof optionsOrPredicate === 'function') - optionsOrPredicate = { predicate: optionsOrPredicate }; - const { timeout = this._timeoutSettings.timeout(), predicate = () => true } = optionsOrPredicate; - return helper.waitForEvent(this, event, (...args: any[]) => !!predicate(...args), timeout, this._disconnectedPromise); - } - async waitForRequest(urlOrPredicate: string | RegExp | ((r: network.Request) => boolean), options: types.TimeoutOptions = {}): Promise { const { timeout = this._timeoutSettings.timeout() } = options; return helper.waitForEvent(this, Events.Page.Request, (request: network.Request) => {