feat(rpc): merge DispatcherScope and Dispatcher (#2918)

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);

src/rpc/server/browserContextDispatcher.ts

@@ -48,7 +48,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, Browser
     context.on(Events.BrowserContext.Page, page => this._dispatchEvent('page', new PageDispatcher(this._scope, page)));
     context.on(Events.BrowserContext.Close, () => {
       this._dispatchEvent('close');
-      this._scope.dispose();
+      this._dispose();
     });
   }
 

src/rpc/server/browserDispatcher.ts

@@ -30,7 +30,7 @@ export class BrowserDispatcher extends Dispatcher<Browser, BrowserInitializer> i
     super(scope, browser, 'browser', {}, true);
     browser.on(Events.Browser.Disconnected, () => {
       this._dispatchEvent('close');
-      this._scope.dispose();
+      this._dispose();
     });
   }
 

src/rpc/server/cdpSessionDispatcher.ts

@@ -24,7 +24,7 @@ export class CDPSessionDispatcher extends Dispatcher<CRSession, CDPSessionInitia
     crSession._eventListener = (method, params) => this._dispatchEvent('event', { method, params });
     crSession.on(CRSessionEvents.Disconnected, () => {
       this._dispatchEvent('disconnected');
-      this._scope.dispose();
+      this._dispose();
     });
   }
 

src/rpc/server/dispatcher.ts

@@ -36,99 +36,95 @@ export function lookupNullableDispatcher<DispatcherType>(object: any | null): Di
 }
 
 export class Dispatcher<Type, Initializer> extends EventEmitter implements Channel {
+  private _connection: DispatcherConnection;
+  private _isScope: boolean;
+  // Parent is always "isScope".
+  private _parent: Dispatcher<any, any> | undefined;
+  // Only "isScope" channel owners have registered dispatchers inside.
+  private _dispatchers = new Map<string, Dispatcher<any, any>>();
+
   readonly _guid: string;
   readonly _type: string;
-  protected _scope: DispatcherScope;
+  readonly _scope: Dispatcher<any, any>;
   _object: Type;
 
-  constructor(scope: DispatcherScope, object: Type, type: string, initializer: Initializer, isScope?: boolean, guid = type + '@' + helper.guid()) {
+  constructor(parent: Dispatcher<any, any> | 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, any> | 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<string, Dispatcher<any, any>>();
-  private _parent: DispatcherScope | undefined;
-  readonly _children = new Set<DispatcherScope>();
-  readonly guid: string;
+export type DispatcherScope = Dispatcher<any, any>;
 
-  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<any, any>) {
-    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<any> {
-    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<string, Dispatcher<any, any>>();
-  private _rootScope: DispatcherScope;
+  private _rootDispatcher: Root;
   onmessage = (message: string) => {};
 
-  async _sendMessageToClient(guid: string, method: string, params: any): Promise<any> {
+  async sendMessageToClient(guid: string, method: string, params: any): Promise<any> {
     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<any, any> {
+    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)

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 };
-}

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')]
     });
   });

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;