From 87516cb3a3dd4b138ebe7fd91179f19ec616f8f4 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Tue, 30 Jun 2020 21:30:39 -0700 Subject: [PATCH] chore(rpc): make dispatcher creation and lookup explicit (#2783) --- src/rpc/dispatcher.ts | 21 +++++++-- src/rpc/server.ts | 6 +-- src/rpc/server/browserContextDispatcher.ts | 13 ++--- src/rpc/server/browserDispatcher.ts | 18 ++----- src/rpc/server/browserServerDispatcher.ts | 6 --- src/rpc/server/browserTypeDispatcher.ts | 14 ++---- src/rpc/server/consoleMessageDispatcher.ts | 10 +--- src/rpc/server/dialogDispatcher.ts | 6 --- src/rpc/server/downloadDispatcher.ts | 6 --- src/rpc/server/elementHandlerDispatcher.ts | 32 ++++--------- src/rpc/server/frameDispatcher.ts | 39 +++++++-------- src/rpc/server/jsHandleDispatcher.ts | 4 +- src/rpc/server/networkDispatchers.ts | 36 ++++---------- src/rpc/server/pageDispatcher.ts | 55 +++++++++------------- test/environments.js | 5 +- test/test.js | 4 -- 16 files changed, 97 insertions(+), 178 deletions(-) diff --git a/src/rpc/dispatcher.ts b/src/rpc/dispatcher.ts index 9e801eb5da..b2357a579a 100644 --- a/src/rpc/dispatcher.ts +++ b/src/rpc/dispatcher.ts @@ -15,10 +15,26 @@ */ import { EventEmitter } from 'events'; -import { helper } from '../helper'; +import { helper, debugAssert } from '../helper'; import { Channel } from './channels'; import { serializeError } from './serializers'; +export const dispatcherSymbol = Symbol('dispatcher'); + +export function lookupDispatcher(object: any): DispatcherType { + const result = object[dispatcherSymbol]; + debugAssert(result); + return result; +} + +export function existingDispatcher(object: any): DispatcherType { + return object[dispatcherSymbol]; +} + +export function lookupNullableDispatcher(object: any | null): DispatcherType | null { + return object ? lookupDispatcher(object) : null; +} + export class Dispatcher extends EventEmitter implements Channel { readonly _guid: string; readonly _type: string; @@ -32,7 +48,7 @@ export class Dispatcher extends EventEmitter implements Chann this._object = object; this._scope = scope; scope.dispatchers.set(this._guid, this); - (object as any)[scope.dispatcherSymbol] = this; + (object as any)[dispatcherSymbol] = this; this._scope.sendMessageToClient(this._guid, '__create__', { type, initializer }); } @@ -43,7 +59,6 @@ export class Dispatcher extends EventEmitter implements Chann export class DispatcherScope { readonly dispatchers = new Map>(); - readonly dispatcherSymbol = Symbol('dispatcher'); onmessage = (message: string) => {}; async sendMessageToClient(guid: string, method: string, params: any): Promise { diff --git a/src/rpc/server.ts b/src/rpc/server.ts index 1918200093..8250beac13 100644 --- a/src/rpc/server.ts +++ b/src/rpc/server.ts @@ -25,6 +25,6 @@ transport.onmessage = message => dispatcherScope.send(message); dispatcherScope.onmessage = message => transport.send(message); const playwright = new Playwright(__dirname, require('../../browsers.json')['browsers']); -BrowserTypeDispatcher.from(dispatcherScope, playwright.chromium!); -BrowserTypeDispatcher.from(dispatcherScope, playwright.firefox!); -BrowserTypeDispatcher.from(dispatcherScope, playwright.webkit!); +new BrowserTypeDispatcher(dispatcherScope, playwright.chromium!); +new BrowserTypeDispatcher(dispatcherScope, playwright.firefox!); +new BrowserTypeDispatcher(dispatcherScope, playwright.webkit!); diff --git a/src/rpc/server/browserContextDispatcher.ts b/src/rpc/server/browserContextDispatcher.ts index 46a139cd92..af6a499c82 100644 --- a/src/rpc/server/browserContextDispatcher.ts +++ b/src/rpc/server/browserContextDispatcher.ts @@ -17,7 +17,7 @@ import * as types from '../../types'; import { BrowserContextBase, BrowserContext } from '../../browserContext'; import { Events } from '../../events'; -import { Dispatcher, DispatcherScope } from '../dispatcher'; +import { Dispatcher, DispatcherScope, lookupNullableDispatcher, lookupDispatcher } from '../dispatcher'; import { PageDispatcher, BindingCallDispatcher } from './pageDispatcher'; import { PageChannel, BrowserContextChannel, BrowserContextInitializer } from '../channels'; import { RouteDispatcher, RequestDispatcher } from './networkDispatchers'; @@ -25,11 +25,6 @@ import { Page } from '../../page'; export class BrowserContextDispatcher extends Dispatcher implements BrowserContextChannel { private _context: BrowserContextBase; - static from(scope: DispatcherScope, browserContext: BrowserContext): BrowserContextDispatcher { - if ((browserContext as any)[scope.dispatcherSymbol]) - return (browserContext as any)[scope.dispatcherSymbol]; - return new BrowserContextDispatcher(scope, browserContext as BrowserContextBase); - } constructor(scope: DispatcherScope, context: BrowserContextBase) { super(scope, context, 'context', { @@ -59,7 +54,7 @@ export class BrowserContextDispatcher extends Dispatcher { - return PageDispatcher.from(this._scope, await this._context.newPage()); + return lookupDispatcher(await this._context.newPage()); } async cookies(params: { urls: string[] }): Promise { @@ -108,14 +103,14 @@ export class BrowserContextDispatcher extends Dispatcher { - this._dispatchEvent('route', { route: RouteDispatcher.from(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); + this._dispatchEvent('route', { route: new RouteDispatcher(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); }); } async waitForEvent(params: { event: string }): Promise { const result = await this._context.waitForEvent(params.event); if (result instanceof Page) - return PageDispatcher.from(this._scope, result); + return lookupNullableDispatcher(result); return result; } diff --git a/src/rpc/server/browserDispatcher.ts b/src/rpc/server/browserDispatcher.ts index bba6c4fc00..a362b61270 100644 --- a/src/rpc/server/browserDispatcher.ts +++ b/src/rpc/server/browserDispatcher.ts @@ -19,34 +19,22 @@ import { BrowserContextBase } from '../../browserContext'; import * as types from '../../types'; import { BrowserContextDispatcher } from './browserContextDispatcher'; import { BrowserChannel, BrowserContextChannel, PageChannel, BrowserInitializer } from '../channels'; -import { Dispatcher, DispatcherScope } from '../dispatcher'; +import { Dispatcher, DispatcherScope, lookupDispatcher } from '../dispatcher'; import { PageDispatcher } from './pageDispatcher'; import { Events } from '../../events'; export class BrowserDispatcher extends Dispatcher implements BrowserChannel { - static from(scope: DispatcherScope, browser: BrowserBase): BrowserDispatcher { - if ((browser as any)[scope.dispatcherSymbol]) - return (browser as any)[scope.dispatcherSymbol]; - return new BrowserDispatcher(scope, browser); - } - - static fromNullable(scope: DispatcherScope, browser: BrowserBase | null): BrowserDispatcher | null { - if (!browser) - return null; - return BrowserDispatcher.from(scope, browser); - } - constructor(scope: DispatcherScope, browser: BrowserBase) { super(scope, browser, 'browser', {}); browser.on(Events.Browser.Disconnected, () => this._dispatchEvent('close')); } async newContext(params: { options?: types.BrowserContextOptions }): Promise { - return BrowserContextDispatcher.from(this._scope, await this._object.newContext(params.options) as BrowserContextBase); + return new BrowserContextDispatcher(this._scope, await this._object.newContext(params.options) as BrowserContextBase); } async newPage(params: { options?: types.BrowserContextOptions }): Promise { - return PageDispatcher.from(this._scope, await this._object.newPage(params.options)); + return lookupDispatcher(await this._object.newPage(params.options))!; } async close(): Promise { diff --git a/src/rpc/server/browserServerDispatcher.ts b/src/rpc/server/browserServerDispatcher.ts index 8f0291fe64..fe2c90e1c1 100644 --- a/src/rpc/server/browserServerDispatcher.ts +++ b/src/rpc/server/browserServerDispatcher.ts @@ -20,12 +20,6 @@ import { Dispatcher, DispatcherScope } from '../dispatcher'; import { Events } from '../../events'; export class BrowserServerDispatcher extends Dispatcher implements BrowserServerChannel { - static from(scope: DispatcherScope, browserServer: BrowserServer): BrowserServerDispatcher { - if ((browserServer as any)[scope.dispatcherSymbol]) - return (browserServer as any)[scope.dispatcherSymbol]; - return new BrowserServerDispatcher(scope, browserServer); - } - constructor(scope: DispatcherScope, browserServer: BrowserServer) { super(scope, browserServer, 'browserServer', { wsEndpoint: browserServer.wsEndpoint(), diff --git a/src/rpc/server/browserTypeDispatcher.ts b/src/rpc/server/browserTypeDispatcher.ts index 47da4b4e5a..ea818faf0a 100644 --- a/src/rpc/server/browserTypeDispatcher.ts +++ b/src/rpc/server/browserTypeDispatcher.ts @@ -25,12 +25,6 @@ import { BrowserContextDispatcher } from './browserContextDispatcher'; import { BrowserServerDispatcher } from './browserServerDispatcher'; export class BrowserTypeDispatcher extends Dispatcher implements BrowserTypeChannel { - static from(scope: DispatcherScope, browserType: BrowserTypeBase): BrowserTypeDispatcher { - if ((browserType as any)[scope.dispatcherSymbol]) - return (browserType as any)[scope.dispatcherSymbol]; - return new BrowserTypeDispatcher(scope, browserType); - } - constructor(scope: DispatcherScope, browserType: BrowserTypeBase) { super(scope, browserType, 'browserType', { executablePath: browserType.executablePath(), @@ -40,20 +34,20 @@ export class BrowserTypeDispatcher extends Dispatcher { const browser = await this._object.launch(params.options || undefined); - return BrowserDispatcher.from(this._scope, browser as BrowserBase); + return new BrowserDispatcher(this._scope, browser as BrowserBase); } async launchPersistentContext(params: { userDataDir: string, options?: types.LaunchOptions & types.BrowserContextOptions }): Promise { const browserContext = await this._object.launchPersistentContext(params.userDataDir, params.options); - return BrowserContextDispatcher.from(this._scope, browserContext as BrowserContextBase); + return new BrowserContextDispatcher(this._scope, browserContext as BrowserContextBase); } async launchServer(params: { options?: types.LaunchServerOptions }): Promise { - return BrowserServerDispatcher.from(this._scope, await this._object.launchServer(params.options)); + return new BrowserServerDispatcher(this._scope, await this._object.launchServer(params.options)); } async connect(params: { options: types.ConnectOptions }): Promise { const browser = await this._object.connect(params.options); - return BrowserDispatcher.from(this._scope, browser as BrowserBase); + return new BrowserDispatcher(this._scope, browser as BrowserBase); } } diff --git a/src/rpc/server/consoleMessageDispatcher.ts b/src/rpc/server/consoleMessageDispatcher.ts index 9569e6bca5..0627652a43 100644 --- a/src/rpc/server/consoleMessageDispatcher.ts +++ b/src/rpc/server/consoleMessageDispatcher.ts @@ -17,20 +17,14 @@ import { ConsoleMessage } from '../../console'; import { ConsoleMessageChannel, ConsoleMessageInitializer } from '../channels'; import { Dispatcher, DispatcherScope } from '../dispatcher'; -import { fromHandle } from './elementHandlerDispatcher'; +import { createHandle } from './elementHandlerDispatcher'; export class ConsoleMessageDispatcher extends Dispatcher implements ConsoleMessageChannel { - static from(scope: DispatcherScope, message: ConsoleMessage): ConsoleMessageDispatcher { - if ((message as any)[scope.dispatcherSymbol]) - return (message as any)[scope.dispatcherSymbol]; - return new ConsoleMessageDispatcher(scope, message); - } - constructor(scope: DispatcherScope, message: ConsoleMessage) { super(scope, message, 'consoleMessage', { type: message.type(), text: message.text(), - args: message.args().map(a => fromHandle(scope, a)), + args: message.args().map(a => createHandle(scope, a)), location: message.location(), }); } diff --git a/src/rpc/server/dialogDispatcher.ts b/src/rpc/server/dialogDispatcher.ts index 8ae9001fad..304d4898ff 100644 --- a/src/rpc/server/dialogDispatcher.ts +++ b/src/rpc/server/dialogDispatcher.ts @@ -19,12 +19,6 @@ import { DialogChannel, DialogInitializer } from '../channels'; import { Dispatcher, DispatcherScope } from '../dispatcher'; export class DialogDispatcher extends Dispatcher implements DialogChannel { - static from(scope: DispatcherScope, dialog: Dialog): DialogDispatcher { - if ((dialog as any)[scope.dispatcherSymbol]) - return (dialog as any)[scope.dispatcherSymbol]; - return new DialogDispatcher(scope, dialog); - } - constructor(scope: DispatcherScope, dialog: Dialog) { super(scope, dialog, 'dialog', { type: dialog.type(), diff --git a/src/rpc/server/downloadDispatcher.ts b/src/rpc/server/downloadDispatcher.ts index c03af0222d..ac69e60cb4 100644 --- a/src/rpc/server/downloadDispatcher.ts +++ b/src/rpc/server/downloadDispatcher.ts @@ -19,12 +19,6 @@ import { DownloadChannel, DownloadInitializer } from '../channels'; import { Dispatcher, DispatcherScope } from '../dispatcher'; export class DownloadDispatcher extends Dispatcher implements DownloadChannel { - static from(scope: DispatcherScope, Download: Download): DownloadDispatcher { - if ((Download as any)[scope.dispatcherSymbol]) - return (Download as any)[scope.dispatcherSymbol]; - return new DownloadDispatcher(scope, Download); - } - constructor(scope: DispatcherScope, download: Download) { super(scope, download, 'download', { url: download.url(), diff --git a/src/rpc/server/elementHandlerDispatcher.ts b/src/rpc/server/elementHandlerDispatcher.ts index b7a7699eca..526d2e00ab 100644 --- a/src/rpc/server/elementHandlerDispatcher.ts +++ b/src/rpc/server/elementHandlerDispatcher.ts @@ -18,38 +18,23 @@ import { ElementHandle } from '../../dom'; import * as js from '../../javascript'; import * as types from '../../types'; import { ElementHandleChannel, FrameChannel } from '../channels'; -import { DispatcherScope } from '../dispatcher'; +import { DispatcherScope, lookupNullableDispatcher } from '../dispatcher'; import { JSHandleDispatcher, serializeResult, parseArgument } from './jsHandleDispatcher'; import { FrameDispatcher } from './frameDispatcher'; -export function fromHandle(scope: DispatcherScope, handle: js.JSHandle): JSHandleDispatcher { - if ((handle as any)[scope.dispatcherSymbol]) - return (handle as any)[scope.dispatcherSymbol]; +export function createHandle(scope: DispatcherScope, handle: js.JSHandle): JSHandleDispatcher { return handle.asElement() ? new ElementHandleDispatcher(scope, handle.asElement()!) : new JSHandleDispatcher(scope, handle); } -export function fromNullableHandle(scope: DispatcherScope, handle: js.JSHandle | null): JSHandleDispatcher | null { - if (!handle) - return null; - return fromHandle(scope, handle); -} - export class ElementHandleDispatcher extends JSHandleDispatcher implements ElementHandleChannel { readonly _elementHandle: ElementHandle; - static fromElement(scope: DispatcherScope, handle: ElementHandle): ElementHandleDispatcher { - if ((handle as any)[scope.dispatcherSymbol]) - return (handle as any)[scope.dispatcherSymbol]; - return new ElementHandleDispatcher(scope, handle); - } - - static fromNullableElement(scope: DispatcherScope, handle: ElementHandle | null): ElementHandleDispatcher | null { + static createNullable(scope: DispatcherScope, handle: ElementHandle | null): ElementHandleDispatcher | null { if (!handle) return null; - return ElementHandleDispatcher.fromElement(scope, handle); + return new ElementHandleDispatcher(scope, handle); } - constructor(scope: DispatcherScope, elementHandle: ElementHandle) { super(scope, elementHandle); this._elementHandle = elementHandle; @@ -57,11 +42,11 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme } async ownerFrame(): Promise { - return FrameDispatcher.fromNullable(this._scope, await this._elementHandle.ownerFrame()); + return lookupNullableDispatcher(await this._elementHandle.ownerFrame()); } async contentFrame(): Promise { - return FrameDispatcher.fromNullable(this._scope, await this._elementHandle.contentFrame()); + return lookupNullableDispatcher(await this._elementHandle.contentFrame()); } async getAttribute(params: { name: string }): Promise { @@ -145,12 +130,13 @@ export class ElementHandleDispatcher extends JSHandleDispatcher implements Eleme } async querySelector(params: { selector: string }): Promise { - return ElementHandleDispatcher.fromNullableElement(this._scope, await this._elementHandle.$(params.selector)); + const handle = await this._elementHandle.$(params.selector); + return handle ? new ElementHandleDispatcher(this._scope, handle) : null; } async querySelectorAll(params: { selector: string }): Promise { const elements = await this._elementHandle.$$(params.selector); - return elements.map(e => ElementHandleDispatcher.fromElement(this._scope, e)); + return elements.map(e => new ElementHandleDispatcher(this._scope, e)); } async $evalExpression(params: { selector: string, expression: string, isFunction: boolean, arg: any }): Promise { diff --git a/src/rpc/server/frameDispatcher.ts b/src/rpc/server/frameDispatcher.ts index 7c078e157a..81f7421f44 100644 --- a/src/rpc/server/frameDispatcher.ts +++ b/src/rpc/server/frameDispatcher.ts @@ -17,8 +17,8 @@ import { Frame } from '../../frames'; import * as types from '../../types'; import { ElementHandleChannel, FrameChannel, FrameInitializer, JSHandleChannel, ResponseChannel } from '../channels'; -import { Dispatcher, DispatcherScope } from '../dispatcher'; -import { convertSelectOptionValues, ElementHandleDispatcher, fromHandle } from './elementHandlerDispatcher'; +import { Dispatcher, DispatcherScope, lookupNullableDispatcher, existingDispatcher } from '../dispatcher'; +import { convertSelectOptionValues, ElementHandleDispatcher, createHandle } from './elementHandlerDispatcher'; import { parseArgument, serializeResult } from './jsHandleDispatcher'; import { ResponseDispatcher } from './networkDispatchers'; @@ -26,29 +26,22 @@ export class FrameDispatcher extends Dispatcher impleme private _frame: Frame; static from(scope: DispatcherScope, frame: Frame): FrameDispatcher { - if ((frame as any)[scope.dispatcherSymbol]) - return (frame as any)[scope.dispatcherSymbol]; - return new FrameDispatcher(scope, frame); + const result = existingDispatcher(frame); + return result || new FrameDispatcher(scope, frame); } - static fromNullable(scope: DispatcherScope, frame: Frame | null): FrameDispatcher | null { - if (!frame) - return null; - return FrameDispatcher.from(scope, frame); - } - - constructor(scope: DispatcherScope, frame: Frame) { + private constructor(scope: DispatcherScope, frame: Frame) { super(scope, frame, 'frame', { url: frame.url(), name: frame.name(), - parentFrame: FrameDispatcher.fromNullable(scope, frame.parentFrame()) + parentFrame: lookupNullableDispatcher(frame.parentFrame()) }); this._frame = frame; } async goto(params: { url: string, options: types.GotoOptions, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ResponseDispatcher.fromNullable(this._scope, await target.goto(params.url, params.options)); + return lookupNullableDispatcher(await target.goto(params.url, params.options)); } async waitForLoadState(params: { state?: 'load' | 'domcontentloaded' | 'networkidle', options?: types.TimeoutOptions, isPage?: boolean }): Promise { @@ -58,11 +51,11 @@ export class FrameDispatcher extends Dispatcher impleme async waitForNavigation(params: { options?: types.WaitForNavigationOptions, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ResponseDispatcher.fromNullable(this._scope, await target.waitForNavigation(params.options)); + return lookupNullableDispatcher(await target.waitForNavigation(params.options)); } async frameElement(): Promise { - return ElementHandleDispatcher.fromElement(this._scope, await this._frame.frameElement()); + return new ElementHandleDispatcher(this._scope, await this._frame.frameElement()); } async evaluateExpression(params: { expression: string, isFunction: boolean, arg: any, isPage?: boolean }): Promise { @@ -72,12 +65,12 @@ export class FrameDispatcher extends Dispatcher impleme async evaluateExpressionHandle(params: { expression: string, isFunction: boolean, arg: any, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return fromHandle(this._scope, await target._evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))); + return createHandle(this._scope, await target._evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))); } async waitForSelector(params: { selector: string, options: types.WaitForElementOptions, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ElementHandleDispatcher.fromNullableElement(this._scope, await target.waitForSelector(params.selector, params.options)); + return ElementHandleDispatcher.createNullable(this._scope, await target.waitForSelector(params.selector, params.options)); } async dispatchEvent(params: { selector: string, type: string, eventInit: any, options: types.TimeoutOptions, isPage?: boolean }): Promise { @@ -97,13 +90,13 @@ export class FrameDispatcher extends Dispatcher impleme async querySelector(params: { selector: string, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ElementHandleDispatcher.fromNullableElement(this._scope, await target.$(params.selector)); + return ElementHandleDispatcher.createNullable(this._scope, await target.$(params.selector)); } async querySelectorAll(params: { selector: string, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; const elements = await target.$$(params.selector); - return elements.map(e => ElementHandleDispatcher.fromElement(this._scope, e)); + return elements.map(e => new ElementHandleDispatcher(this._scope, e)); } async content(): Promise { @@ -117,12 +110,12 @@ export class FrameDispatcher extends Dispatcher impleme async addScriptTag(params: { options: { url?: string | undefined, path?: string | undefined, content?: string | undefined, type?: string | undefined }, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ElementHandleDispatcher.fromElement(this._scope, await target.addScriptTag(params.options)); + return new ElementHandleDispatcher(this._scope, await target.addScriptTag(params.options)); } async addStyleTag(params: { options: { url?: string | undefined, path?: string | undefined, content?: string | undefined }, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return ElementHandleDispatcher.fromElement(this._scope, await target.addStyleTag(params.options)); + return new ElementHandleDispatcher(this._scope, await target.addStyleTag(params.options)); } async click(params: { selector: string, options: types.PointerActionOptions & types.MouseClickOptions & types.TimeoutOptions & { force?: boolean } & { noWaitAfter?: boolean }, isPage?: boolean }): Promise { @@ -202,7 +195,7 @@ export class FrameDispatcher extends Dispatcher impleme async waitForFunction(params: { expression: string, isFunction: boolean, arg: any; options: types.WaitForFunctionOptions, isPage?: boolean }): Promise { const target = params.isPage ? this._frame._page : this._frame; - return fromHandle(this._scope, await target._waitForFunctionExpression(params.expression, params.isFunction, parseArgument(params.arg), params.options)); + return createHandle(this._scope, await target._waitForFunctionExpression(params.expression, params.isFunction, parseArgument(params.arg), params.options)); } async title(): Promise { diff --git a/src/rpc/server/jsHandleDispatcher.ts b/src/rpc/server/jsHandleDispatcher.ts index 1d6d27521a..d8092786ae 100644 --- a/src/rpc/server/jsHandleDispatcher.ts +++ b/src/rpc/server/jsHandleDispatcher.ts @@ -18,7 +18,7 @@ import * as js from '../../javascript'; import { JSHandleChannel, JSHandleInitializer } from '../channels'; import { Dispatcher, DispatcherScope } from '../dispatcher'; import { parseEvaluationResultValue, serializeAsCallArgument } from '../../common/utilityScriptSerializers'; -import { fromHandle } from './elementHandlerDispatcher'; +import { createHandle } from './elementHandlerDispatcher'; export class JSHandleDispatcher extends Dispatcher implements JSHandleChannel { @@ -35,7 +35,7 @@ export class JSHandleDispatcher extends Dispatcher { const jsHandle = await this._object._evaluateExpression(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg)); - return fromHandle(this._scope, jsHandle); + return createHandle(this._scope, jsHandle); } async getPropertyList(): Promise<{ name: string, value: JSHandleChannel }[]> { diff --git a/src/rpc/server/networkDispatchers.ts b/src/rpc/server/networkDispatchers.ts index cae5c4a8ca..50cd453753 100644 --- a/src/rpc/server/networkDispatchers.ts +++ b/src/rpc/server/networkDispatchers.ts @@ -17,21 +17,21 @@ import { Request, Response, Route } from '../../network'; import * as types from '../../types'; import { RequestChannel, ResponseChannel, RouteChannel, ResponseInitializer, RequestInitializer, RouteInitializer, Binary } from '../channels'; -import { Dispatcher, DispatcherScope } from '../dispatcher'; +import { Dispatcher, DispatcherScope, lookupNullableDispatcher, existingDispatcher } from '../dispatcher'; import { FrameDispatcher } from './frameDispatcher'; export class RequestDispatcher extends Dispatcher implements RequestChannel { + static from(scope: DispatcherScope, request: Request): RequestDispatcher { - if ((request as any)[scope.dispatcherSymbol]) - return (request as any)[scope.dispatcherSymbol]; - return new RequestDispatcher(scope, request); + const result = existingDispatcher(request); + return result || new RequestDispatcher(scope, request); } static fromNullable(scope: DispatcherScope, request: Request | null): RequestDispatcher | null { return request ? RequestDispatcher.from(scope, request) : null; } - constructor(scope: DispatcherScope, request: Request) { + private constructor(scope: DispatcherScope, request: Request) { super(scope, request, 'request', { frame: FrameDispatcher.from(scope, request.frame()), url: request.url(), @@ -45,25 +45,16 @@ export class RequestDispatcher extends Dispatcher i } async response(): Promise { - return ResponseDispatcher.fromNullable(this._scope, await this._object.response()); + return lookupNullableDispatcher(await this._object.response()); } } export class ResponseDispatcher extends Dispatcher implements ResponseChannel { - static from(scope: DispatcherScope, response: Response): ResponseDispatcher { - if ((response as any)[scope.dispatcherSymbol]) - return (response as any)[scope.dispatcherSymbol]; - return new ResponseDispatcher(scope, response); - } - - static fromNullable(scope: DispatcherScope, response: Response | null): ResponseDispatcher | null { - return response ? ResponseDispatcher.from(scope, response) : null; - } - constructor(scope: DispatcherScope, response: Response) { super(scope, response, 'response', { - request: RequestDispatcher.from(scope, response.request())!, + // TODO: responses in popups can point to non-reported requests. + request: RequestDispatcher.from(scope, response.request()), url: response.url(), status: response.status(), statusText: response.statusText(), @@ -82,18 +73,9 @@ export class ResponseDispatcher extends Dispatcher implements RouteChannel { - static from(scope: DispatcherScope, route: Route): RouteDispatcher { - if ((route as any)[scope.dispatcherSymbol]) - return (route as any)[scope.dispatcherSymbol]; - return new RouteDispatcher(scope, route); - } - - static fromNullable(scope: DispatcherScope, route: Route | null): RouteDispatcher | null { - return route ? RouteDispatcher.from(scope, route) : null; - } - constructor(scope: DispatcherScope, route: Route) { super(scope, route, 'route', { + // Context route can point to a non-reported request. request: RequestDispatcher.from(scope, route.request()) }); } diff --git a/src/rpc/server/pageDispatcher.ts b/src/rpc/server/pageDispatcher.ts index 171c240261..00b4a723e1 100644 --- a/src/rpc/server/pageDispatcher.ts +++ b/src/rpc/server/pageDispatcher.ts @@ -21,7 +21,7 @@ import { Request } from '../../network'; import { Page, Worker } from '../../page'; import * as types from '../../types'; import { BindingCallChannel, BindingCallInitializer, ElementHandleChannel, PageChannel, PageInitializer, ResponseChannel, WorkerInitializer, WorkerChannel, JSHandleChannel, Binary } from '../channels'; -import { Dispatcher, DispatcherScope } from '../dispatcher'; +import { Dispatcher, DispatcherScope, lookupDispatcher, lookupNullableDispatcher, existingDispatcher } from '../dispatcher'; import { parseError, serializeError } from '../serializers'; import { ConsoleMessageDispatcher } from './consoleMessageDispatcher'; import { DialogDispatcher } from './dialogDispatcher'; @@ -29,38 +29,35 @@ import { DownloadDispatcher } from './downloadDispatcher'; import { FrameDispatcher } from './frameDispatcher'; import { RequestDispatcher, ResponseDispatcher, RouteDispatcher } from './networkDispatchers'; import { serializeResult, parseArgument } from './jsHandleDispatcher'; -import { fromHandle, ElementHandleDispatcher } from './elementHandlerDispatcher'; +import { ElementHandleDispatcher, createHandle } from './elementHandlerDispatcher'; import { FileChooser } from '../../fileChooser'; export class PageDispatcher extends Dispatcher implements PageChannel { private _page: Page; static from(scope: DispatcherScope, page: Page): PageDispatcher { - if ((page as any)[scope.dispatcherSymbol]) - return (page as any)[scope.dispatcherSymbol]; - return new PageDispatcher(scope, page); + const result = existingDispatcher(page); + return result || new PageDispatcher(scope, page); } - static fromNullable(scope: DispatcherScope, page: Page | null): PageDispatcher | null { - if (!page) - return null; - return PageDispatcher.from(scope, page); + static fromNullable(scope: DispatcherScope, request: Page | null): PageDispatcher | null { + return request ? PageDispatcher.from(scope, request) : null; } - constructor(scope: DispatcherScope, page: Page) { + private constructor(scope: DispatcherScope, page: Page) { super(scope, page, 'page', { mainFrame: FrameDispatcher.from(scope, page.mainFrame()), viewportSize: page.viewportSize() }); this._page = page; page.on(Events.Page.Close, () => this._dispatchEvent('close')); - page.on(Events.Page.Console, message => this._dispatchEvent('console', ConsoleMessageDispatcher.from(this._scope, message))); + page.on(Events.Page.Console, message => this._dispatchEvent('console', new ConsoleMessageDispatcher(this._scope, message))); page.on(Events.Page.Crash, () => this._dispatchEvent('crash')); page.on(Events.Page.DOMContentLoaded, () => this._dispatchEvent('domcontentloaded')); - page.on(Events.Page.Dialog, dialog => this._dispatchEvent('dialog', DialogDispatcher.from(this._scope, dialog))); - page.on(Events.Page.Download, dialog => this._dispatchEvent('download', DownloadDispatcher.from(this._scope, dialog))); + page.on(Events.Page.Dialog, dialog => this._dispatchEvent('dialog', new DialogDispatcher(this._scope, dialog))); + page.on(Events.Page.Download, dialog => this._dispatchEvent('download', new DownloadDispatcher(this._scope, dialog))); page.on(Events.Page.FileChooser, (fileChooser: FileChooser) => this._dispatchEvent('fileChooser', { - element: ElementHandleDispatcher.fromElement(this._scope, fileChooser.element()), + element: new ElementHandleDispatcher(this._scope, fileChooser.element()), isMultiple: fileChooser.isMultiple() })); page.on(Events.Page.FrameAttached, frame => this._onFrameAttached(frame)); @@ -74,9 +71,9 @@ export class PageDispatcher extends Dispatcher implements request: RequestDispatcher.from(this._scope, request), failureText: request._failureText })); - page.on(Events.Page.RequestFinished, request => this._dispatchEvent('requestFinished', RequestDispatcher.from(this._scope, request))); - page.on(Events.Page.Response, response => this._dispatchEvent('response', ResponseDispatcher.from(this._scope, response))); - page.on(Events.Page.Worker, worker => this._dispatchEvent('worker', WorkerDispatcher.from(this._scope, worker))); + page.on(Events.Page.RequestFinished, request => this._dispatchEvent('requestFinished', RequestDispatcher.from(scope, request))); + page.on(Events.Page.Response, response => this._dispatchEvent('response', new ResponseDispatcher(this._scope, response))); + page.on(Events.Page.Worker, worker => this._dispatchEvent('worker', new WorkerDispatcher(this._scope, worker))); } async setDefaultNavigationTimeoutNoReply(params: { timeout: number }) { @@ -88,7 +85,7 @@ export class PageDispatcher extends Dispatcher implements } async opener(): Promise { - return PageDispatcher.fromNullable(this._scope, await this._page.opener()); + return lookupNullableDispatcher(await this._page.opener()); } async exposeBinding(params: { name: string }): Promise { @@ -104,15 +101,15 @@ export class PageDispatcher extends Dispatcher implements } async reload(params: { options?: types.NavigateOptions }): Promise { - return ResponseDispatcher.fromNullable(this._scope, await this._page.reload(params.options)); + return lookupNullableDispatcher(await this._page.reload(params.options)); } async goBack(params: { options?: types.NavigateOptions }): Promise { - return ResponseDispatcher.fromNullable(this._scope, await this._page.goBack(params.options)); + return lookupNullableDispatcher(await this._page.goBack(params.options)); } async goForward(params: { options?: types.NavigateOptions }): Promise { - return ResponseDispatcher.fromNullable(this._scope, await this._page.goForward(params.options)); + return lookupNullableDispatcher(await this._page.goForward(params.options)); } async emulateMedia(params: { options: { media?: 'screen' | 'print', colorScheme?: 'dark' | 'light' | 'no-preference' } }): Promise { @@ -133,7 +130,7 @@ export class PageDispatcher extends Dispatcher implements return; } this._page.route('**/*', (route, request) => { - this._dispatchEvent('route', { route: RouteDispatcher.from(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); + this._dispatchEvent('route', { route: new RouteDispatcher(this._scope, route), request: RequestDispatcher.from(this._scope, request) }); }); } @@ -207,22 +204,16 @@ export class PageDispatcher extends Dispatcher implements } _onFrameNavigated(frame: Frame) { - this._dispatchEvent('frameNavigated', { frame: FrameDispatcher.from(this._scope, frame), url: frame.url(), name: frame.name() }); + this._dispatchEvent('frameNavigated', { frame: lookupDispatcher(frame), url: frame.url(), name: frame.name() }); } _onFrameDetached(frame: Frame) { - this._dispatchEvent('frameDetached', FrameDispatcher.from(this._scope, frame)); + this._dispatchEvent('frameDetached', lookupDispatcher(frame)); } } export class WorkerDispatcher extends Dispatcher implements WorkerChannel { - static from(scope: DispatcherScope, worker: Worker): WorkerDispatcher { - if ((worker as any)[scope.dispatcherSymbol]) - return (worker as any)[scope.dispatcherSymbol]; - return new WorkerDispatcher(scope, worker); - } - constructor(scope: DispatcherScope, worker: Worker) { super(scope, worker, 'worker', { url: worker.url() @@ -235,7 +226,7 @@ export class WorkerDispatcher extends Dispatcher impl } async evaluateExpressionHandle(params: { expression: string, isFunction: boolean, arg: any, isPage?: boolean }): Promise { - return fromHandle(this._scope, await this._object._evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))); + return createHandle(this._scope, await this._object._evaluateExpressionHandle(params.expression, params.isFunction, parseArgument(params.arg))); } } @@ -246,7 +237,7 @@ export class BindingCallDispatcher extends Dispatcher<{}, BindingCallInitializer constructor(scope: DispatcherScope, name: string, source: { context: BrowserContext, page: Page, frame: Frame }, args: any[]) { super(scope, {}, 'bindingCall', { - frame: FrameDispatcher.from(scope, source.frame), + frame: lookupDispatcher(source.frame), name, args }); diff --git a/test/environments.js b/test/environments.js index ffc89ce1dd..1e93e90ff1 100644 --- a/test/environments.js +++ b/test/environments.js @@ -20,6 +20,9 @@ const fs = require('fs'); const path = require('path'); const rm = require('rimraf').sync; const {TestServer} = require('../utils/testserver/'); +const { DispatcherScope } = require('../lib/rpc/dispatcher'); +const { Connection } = require('../lib/rpc/connection'); +const { BrowserTypeDispatcher } = require('../lib/rpc/server/browserTypeDispatcher'); class ServerEnvironment { async beforeAll(state) { @@ -179,7 +182,7 @@ class BrowserTypeEnvironment { await new Promise(f => setImmediate(f)); return result; }; - BrowserTypeDispatcher.from(dispatcherScope, this._browserType); + new BrowserTypeDispatcher(dispatcherScope, this._browserType); overridenBrowserType = await connection.waitForObjectWithKnownName(this._browserType.name()); } state.browserType = overridenBrowserType; diff --git a/test/test.js b/test/test.js index 8db7b31b11..87af708b67 100644 --- a/test/test.js +++ b/test/test.js @@ -18,10 +18,6 @@ const fs = require('fs'); const utils = require('./utils'); const TestRunner = require('../utils/testrunner/'); -const { DispatcherScope } = require('../lib/rpc/dispatcher'); -const { Connection } = require('../lib/rpc/connection'); -const { helper } = require('../lib/helper'); -const { BrowserTypeDispatcher } = require('../lib/rpc/server/browserTypeDispatcher'); const { PlaywrightEnvironment, BrowserTypeEnvironment, BrowserEnvironment, PageEnvironment} = require('./environments.js'); Error.stackTraceLimit = 15;