diff --git a/src/rpc/server.ts b/src/rpc/server.ts index 05c67006a9..68695ec961 100644 --- a/src/rpc/server.ts +++ b/src/rpc/server.ts @@ -25,4 +25,4 @@ transport.onmessage = message => dispatcherConnection.dispatch(message); dispatcherConnection.onmessage = message => transport.send(message); const playwright = new Playwright(__dirname, require('../../browsers.json')['browsers']); -new PlaywrightDispatcher(dispatcherConnection.rootScope(), playwright); +new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), playwright); diff --git a/src/rpc/server/browserContextDispatcher.ts b/src/rpc/server/browserContextDispatcher.ts index b825bdb58a..666ef89f07 100644 --- a/src/rpc/server/browserContextDispatcher.ts +++ b/src/rpc/server/browserContextDispatcher.ts @@ -48,7 +48,7 @@ export class BrowserContextDispatcher extends Dispatcher this._dispatchEvent('page', new PageDispatcher(this._scope, page))); context.on(Events.BrowserContext.Close, () => { this._dispatchEvent('close'); - this._scope.dispose(); + this._dispose(); }); } diff --git a/src/rpc/server/browserDispatcher.ts b/src/rpc/server/browserDispatcher.ts index 24b6b8b7d8..7a5a6f9787 100644 --- a/src/rpc/server/browserDispatcher.ts +++ b/src/rpc/server/browserDispatcher.ts @@ -30,7 +30,7 @@ export class BrowserDispatcher extends Dispatcher i super(scope, browser, 'browser', {}, true); browser.on(Events.Browser.Disconnected, () => { this._dispatchEvent('close'); - this._scope.dispose(); + this._dispose(); }); } diff --git a/src/rpc/server/cdpSessionDispatcher.ts b/src/rpc/server/cdpSessionDispatcher.ts index 26e3ff7d57..973ea791c6 100644 --- a/src/rpc/server/cdpSessionDispatcher.ts +++ b/src/rpc/server/cdpSessionDispatcher.ts @@ -24,7 +24,7 @@ export class CDPSessionDispatcher extends Dispatcher this._dispatchEvent('event', { method, params }); crSession.on(CRSessionEvents.Disconnected, () => { this._dispatchEvent('disconnected'); - this._scope.dispose(); + this._dispose(); }); } diff --git a/src/rpc/server/dispatcher.ts b/src/rpc/server/dispatcher.ts index 718dc8a717..6c78f16155 100644 --- a/src/rpc/server/dispatcher.ts +++ b/src/rpc/server/dispatcher.ts @@ -36,99 +36,95 @@ export function lookupNullableDispatcher(object: any | null): Di } export class Dispatcher extends EventEmitter implements Channel { + private _connection: DispatcherConnection; + private _isScope: boolean; + // Parent is always "isScope". + private _parent: Dispatcher | undefined; + // Only "isScope" channel owners have registered dispatchers inside. + private _dispatchers = new Map>(); + readonly _guid: string; readonly _type: string; - protected _scope: DispatcherScope; + readonly _scope: Dispatcher; _object: Type; - constructor(scope: DispatcherScope, object: Type, type: string, initializer: Initializer, isScope?: boolean, guid = type + '@' + helper.guid()) { + constructor(parent: Dispatcher | DispatcherConnection, object: Type, type: string, initializer: Initializer, isScope?: boolean, guid = type + '@' + helper.guid()) { super(); + + this._connection = parent instanceof DispatcherConnection ? parent : parent._connection; + this._isScope = !!isScope; + this._parent = parent instanceof DispatcherConnection ? undefined : parent; + this._scope = isScope ? this : this._parent!; + + assert(!this._connection._dispatchers.has(guid)); + this._connection._dispatchers.set(guid, this); + if (this._parent) { + assert(!this._parent._dispatchers.has(guid)); + this._parent._dispatchers.set(guid, this); + } + this._type = type; this._guid = guid; this._object = object; - this._scope = isScope ? scope.createChild(guid) : scope; - scope.bind(this._guid, this); + (object as any)[dispatcherSymbol] = this; - this._scope.sendMessageToClient(scope.guid, '__create__', { type, initializer, guid }); + if (this._parent) + this._connection.sendMessageToClient(this._parent._guid, '__create__', { type, initializer, guid }); } _dispatchEvent(method: string, params: Dispatcher | any = {}) { - this._scope.sendMessageToClient(this._guid, method, params); + this._connection.sendMessageToClient(this._guid, method, params); + } + + _dispose() { + assert(this._isScope); + + // Clean up from parent and connection. + if (this._parent) + this._parent._dispatchers.delete(this._guid); + this._connection._dispatchers.delete(this._guid); + + // Dispose all children. + for (const [guid, dispatcher] of [...this._dispatchers]) { + if (dispatcher._isScope) + dispatcher._dispose(); + else + this._connection._dispatchers.delete(guid); + } + this._dispatchers.clear(); + } + + _debugScopeState(): any { + return { + _guid: this._guid, + objects: this._isScope ? Array.from(this._dispatchers.values()).map(o => o._debugScopeState()) : undefined, + }; } } -export class DispatcherScope { - private _connection: DispatcherConnection; - private _dispatchers = new Map>(); - private _parent: DispatcherScope | undefined; - readonly _children = new Set(); - readonly guid: string; +export type DispatcherScope = Dispatcher; - constructor(connection: DispatcherConnection, guid: string, parent?: DispatcherScope) { - this._connection = connection; - this._parent = parent; - this.guid = guid; - if (parent) - parent._children.add(this); - } - - createChild(guid: string): DispatcherScope { - return new DispatcherScope(this._connection, guid, this); - } - - bind(guid: string, arg: Dispatcher) { - assert(!this._dispatchers.has(guid)); - this._dispatchers.set(guid, arg); - this._connection._dispatchers.set(guid, arg); - } - - dispose() { - // Take care of hierarchy. - for (const child of [...this._children]) - child.dispose(); - this._children.clear(); - - // Delete self from scopes and objects. - this._connection._dispatchers.delete(this.guid); - - // Delete all of the objects from connection. - for (const guid of this._dispatchers.keys()) - this._connection._dispatchers.delete(guid); - - if (this._parent) { - this._parent._children.delete(this); - this._parent._dispatchers.delete(this.guid); - } - } - - async sendMessageToClient(guid: string, method: string, params: any): Promise { - this._connection._sendMessageToClient(guid, method, params); - } - - _dumpScopeState(scopes: any[]): any { - const scopeState: any = { _guid: this.guid }; - scopeState.objects = [...this._dispatchers.keys()]; - scopes.push(scopeState); - [...this._children].map(c => c._dumpScopeState(scopes)); - return scopeState; +class Root extends Dispatcher<{}, {}> { + constructor(connection: DispatcherConnection) { + super(connection, {}, '', {}, true, ''); } } export class DispatcherConnection { readonly _dispatchers = new Map>(); - private _rootScope: DispatcherScope; + private _rootDispatcher: Root; onmessage = (message: string) => {}; - async _sendMessageToClient(guid: string, method: string, params: any): Promise { + async sendMessageToClient(guid: string, method: string, params: any): Promise { this.onmessage(JSON.stringify({ guid, method, params: this._replaceDispatchersWithGuids(params) })); } constructor() { - this._rootScope = new DispatcherScope(this, ''); + this._rootDispatcher = new Root(this); } - rootScope(): DispatcherScope { - return this._rootScope; + rootDispatcher(): Dispatcher { + return this._rootDispatcher; } async dispatch(message: string) { @@ -140,11 +136,7 @@ export class DispatcherConnection { return; } if (method === 'debugScopeState') { - const dispatcherState: any = {}; - dispatcherState.objects = [...this._dispatchers.keys()]; - dispatcherState.scopes = []; - this._rootScope._dumpScopeState(dispatcherState.scopes); - this.onmessage(JSON.stringify({ id, result: dispatcherState })); + this.onmessage(JSON.stringify({ id, result: this._rootDispatcher._debugScopeState() })); return; } try { @@ -155,7 +147,7 @@ export class DispatcherConnection { } } - _replaceDispatchersWithGuids(payload: any): any { + private _replaceDispatchersWithGuids(payload: any): any { if (!payload) return payload; if (payload instanceof Dispatcher) diff --git a/test/channels.spec.js b/test/channels.spec.js index 23b8374cfe..713e38bedd 100644 --- a/test/channels.spec.js +++ b/test/channels.spec.js @@ -24,10 +24,13 @@ describe.skip(!CHANNEL)('Channels', function() { it('should scope context handles', async({browser, server}) => { const GOLDEN_PRECONDITION = { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ] }, - { _guid: 'browser', objects: [] } + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [] }, ] }; await expectScopeState(browser, GOLDEN_PRECONDITION); @@ -36,11 +39,20 @@ describe.skip(!CHANNEL)('Channels', function() { const page = await context.newPage(); await page.goto(server.EMPTY_PAGE); await expectScopeState(browser, { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser', 'context', 'frame', 'page', 'request', 'response' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ] }, - { _guid: 'browser', objects: ['context'] }, - { _guid: 'context', objects: ['frame', 'page', 'request', 'response'] } + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [ + { _guid: 'context', objects: [ + { _guid: 'frame' }, + { _guid: 'page' }, + { _guid: 'request' }, + { _guid: 'response' }, + ]}, + ] }, ] }); @@ -50,21 +62,28 @@ describe.skip(!CHANNEL)('Channels', function() { it.skip(!CHROMIUM)('should scope CDPSession handles', async({browserType, browser, server}) => { const GOLDEN_PRECONDITION = { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ] }, - { _guid: 'browser', objects: [] } + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [] }, ] }; await expectScopeState(browserType, GOLDEN_PRECONDITION); const session = await browser.newBrowserCDPSession(); await expectScopeState(browserType, { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser', 'cdpSession' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ] }, - { _guid: 'browser', objects: ['cdpSession'] }, - { _guid: 'cdpSession', objects: [] }, + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [ + { _guid: 'cdpSession', objects: [] }, + ] }, ] }); @@ -74,10 +93,13 @@ describe.skip(!CHANNEL)('Channels', function() { it('should scope browser handles', async({browserType, defaultBrowserOptions}) => { const GOLDEN_PRECONDITION = { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser' ] }, - { _guid: 'browser', objects: [] } + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [] }, ] }; await expectScopeState(browserType, GOLDEN_PRECONDITION); @@ -85,12 +107,16 @@ describe.skip(!CHANNEL)('Channels', function() { const browser = await browserType.launch(defaultBrowserOptions); await browser.newContext(); await expectScopeState(browserType, { - objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser', 'browser', 'context' ], - scopes: [ - { _guid: '', objects: [ 'chromium', 'firefox', 'webkit', 'playwright', 'browser', 'browser' ] }, + _guid: '', + objects: [ + { _guid: 'chromium' }, + { _guid: 'firefox' }, + { _guid: 'webkit' }, + { _guid: 'playwright' }, + { _guid: 'browser', objects: [ + { _guid: 'context', objects: [] }, + ] }, { _guid: 'browser', objects: [] }, - { _guid: 'browser', objects: ['context'] }, - { _guid: 'context', objects: [] }, ] }); @@ -100,15 +126,28 @@ describe.skip(!CHANNEL)('Channels', function() { }); async function expectScopeState(object, golden) { + golden = trimGuids(golden); const remoteState = trimGuids(await object._channel.debugScopeState()); const localState = trimGuids(object._connection._debugScopeState()); - expect(processLocalState(localState)).toEqual(golden); + expect(localState).toEqual(golden); expect(remoteState).toEqual(golden); } +function compareObjects(a, b) { + if (a._guid !== b._guid) + return a._guid.localeCompare(b._guid); + if (a.objects && !b.objects) + return -1; + if (!a.objects && b.objects) + return 1; + if (!a.objects && !b.objects) + return 0; + return a.objects.length - b.objects.length; +} + function trimGuids(object) { if (Array.isArray(object)) - return object.map(trimGuids); + return object.map(trimGuids).sort(compareObjects); if (typeof object === 'object') { const result = {}; for (const key in object) @@ -119,21 +158,3 @@ function trimGuids(object) { return object ? object.match(/[^@]+/)[0] : ''; return object; } - -function processLocalState(root) { - const objects = []; - const scopes = []; - const visit = (object, scope) => { - if (object._guid !== '') - objects.push(object._guid); - scope.push(object._guid); - if (object.objects) { - scope = []; - scopes.push({ _guid: object._guid, objects: scope }); - for (const child of object.objects) - visit(child, scope); - } - }; - visit(root, []); - return { objects, scopes }; -} diff --git a/test/electron/electron.spec.js b/test/electron/electron.spec.js index b5fe8fe136..20988d1b12 100644 --- a/test/electron/electron.spec.js +++ b/test/electron/electron.spec.js @@ -17,14 +17,15 @@ const path = require('path'); const config = require('../test.config'); const utils = require('../utils'); +const {CHANNEL} = utils.testOptions(browserType); const electronName = process.platform === 'win32' ? 'electron.cmd' : 'electron'; -describe('Electron', function() { +describe.skip(CHANNEL)('Electron', function() { beforeEach(async (state, testRun) => { const electronPath = path.join(__dirname, '..', '..', 'node_modules', '.bin', electronName); state.logger = utils.createTestLogger(config.dumpLogOnFailure, testRun); - state.application = await playwright.electron.launch(electronPath, { + state.application = await state.playwright.electron.launch(electronPath, { args: [path.join(__dirname, 'testApp.js')], // This is for our own extensive protocol logging, customers don't need it. logger: state.logger, @@ -116,10 +117,10 @@ describe('Electron', function() { }); }); -describe('Electron per window', function() { +describe.skip(CHANNEL)('Electron per window', function() { beforeAll(async state => { const electronPath = path.join(__dirname, '..', '..', 'node_modules', '.bin', electronName); - state.application = await playwright.electron.launch(electronPath, { + state.application = await state.playwright.electron.launch(electronPath, { args: [path.join(__dirname, 'testApp.js')] }); }); diff --git a/test/environments.js b/test/environments.js index a2cbbd610c..cda1ede946 100644 --- a/test/environments.js +++ b/test/environments.js @@ -174,7 +174,7 @@ class PlaywrightEnvironment { await new Promise(f => setImmediate(f)); return result; }; - new PlaywrightDispatcher(dispatcherConnection.rootScope(), this._playwright); + new PlaywrightDispatcher(dispatcherConnection.rootDispatcher(), this._playwright); this.overriddenPlaywright = await connection.waitForObjectWithKnownName('playwright'); } state.playwright = this.overriddenPlaywright;