chore(electron): fix node/browser race conditions, expose browser window asynchronously (#6381)

Author: pavelfeldman

docs/src/api/class-electronapplication.md

@@ -45,6 +45,16 @@ This event is issued when the application closes.
 This event is issued for every window that is created **and loaded** in Electron. It contains a [Page] that can
 be used for Playwright automation.
 
+## async method: ElectronApplication.browserWindow
+- returns: <[JSHandle]>
+
+Returns the BrowserWindow object that corresponds to the given Playwright page.
+
+### param: ElectronApplication.browserWindow.page
+- `page` <[Page]>
+
+Page to retrieve the window for.
+
 ## async method: ElectronApplication.close
 
 Closes Electron application.

src/client/electron.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import type { BrowserWindow } from 'electron';
 import * as structs from '../../types/structs';
 import * as api from '../../types/types';
 import * as channels from '../protocol/channels';
@@ -55,7 +56,7 @@ export class Electron extends ChannelOwner<channels.ElectronChannel, channels.El
 }
 
 export class ElectronApplication extends ChannelOwner<channels.ElectronApplicationChannel, channels.ElectronApplicationInitializer> implements api.ElectronApplication {
-  private _context?: BrowserContext;
+  private _context: BrowserContext;
   private _windows = new Set<Page>();
   private _timeoutSettings = new TimeoutSettings();
 
@@ -65,13 +66,11 @@ export class ElectronApplication extends ChannelOwner<channels.ElectronApplicati
 
   constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.ElectronApplicationInitializer) {
     super(parent, type, guid, initializer);
-    this._channel.on('context', ({ context }) => this._context = BrowserContext.from(context));
-    this._channel.on('window', ({ page, browserWindow }) => {
-      const window = Page.from(page);
-      (window as any).browserWindow = JSHandle.from(browserWindow);
-      this._windows.add(window);
-      this.emit(Events.ElectronApplication.Window, window);
-      window.once(Events.Page.Close, () => this._windows.delete(window));
+    this._context = BrowserContext.from(initializer.context);
+    this._context.on(Events.BrowserContext.Page, page => {
+      this._windows.add(page);
+      this.emit(Events.ElectronApplication.Window, page);
+      page.once(Events.Page.Close, () => this._windows.delete(page));
     });
     this._channel.on('close', () => this.emit(Events.ElectronApplication.Close));
   }
@@ -109,6 +108,13 @@ export class ElectronApplication extends ChannelOwner<channels.ElectronApplicati
     return result;
   }
 
+  async browserWindow(page: Page): Promise<JSHandle<BrowserWindow>> {
+    return this._wrapApiCall('electronApplication.browserWindow', async (channel: channels.ElectronApplicationChannel) => {
+      const result = await channel.browserWindow({ page: page._channel });
+      return JSHandle.from(result.handle);
+    });
+  }
+
   async evaluate<R, Arg>(pageFunction: structs.PageFunctionOn<ElectronAppType, Arg, R>, arg: Arg): Promise<R> {
     return this._wrapApiCall('electronApplication.evaluate', async (channel: channels.ElectronApplicationChannel) => {
       const result = await channel.evaluateExpression({ expression: String(pageFunction), isFunction: typeof pageFunction === 'function', arg: serializeArgument(arg) });

src/dispatchers/electronDispatcher.ts

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-import { Dispatcher, DispatcherScope, lookupDispatcher } from './dispatcher';
-import { Electron, ElectronApplication, ElectronPage } from '../server/electron/electron';
+import { Dispatcher, DispatcherScope } from './dispatcher';
+import { Electron, ElectronApplication } from '../server/electron/electron';
 import * as channels from '../protocol/channels';
 import { BrowserContextDispatcher } from './browserContextDispatcher';
 import { PageDispatcher } from './pageDispatcher';
@@ -35,27 +35,27 @@ export class ElectronDispatcher extends Dispatcher<Electron, channels.ElectronIn
 
 export class ElectronApplicationDispatcher extends Dispatcher<ElectronApplication, channels.ElectronApplicationInitializer> implements channels.ElectronApplicationChannel {
   constructor(scope: DispatcherScope, electronApplication: ElectronApplication) {
-    super(scope, electronApplication, 'ElectronApplication', {}, true);
-    this._dispatchEvent('context', { context: new BrowserContextDispatcher(this._scope, electronApplication.context()) });
+    super(scope, electronApplication, 'ElectronApplication', {
+      context: new BrowserContextDispatcher(scope, electronApplication.context())
+    }, true);
     electronApplication.on(ElectronApplication.Events.Close, () => {
       this._dispatchEvent('close');
       this._dispose();
     });
-    electronApplication.on(ElectronApplication.Events.Window, (page: ElectronPage) => {
-      this._dispatchEvent('window', {
-        page: lookupDispatcher<PageDispatcher>(page),
-        browserWindow: ElementHandleDispatcher.fromJSHandle(this._scope, page.browserWindow),
-      });
-    });
+  }
+
+  async browserWindow(params: channels.ElectronApplicationBrowserWindowParams): Promise<channels.ElectronApplicationBrowserWindowResult> {
+    const handle = await this._object.browserWindow((params.page as PageDispatcher).page());
+    return { handle: ElementHandleDispatcher.fromJSHandle(this._scope, handle) };
   }
 
   async evaluateExpression(params: channels.ElectronApplicationEvaluateExpressionParams): Promise<channels.ElectronApplicationEvaluateExpressionResult> {
-    const handle = await this._object._nodeElectronHandlePromised;
+    const handle = await this._object._nodeElectronHandlePromise;
     return { value: serializeResult(await handle.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, true /* returnByValue */, parseArgument(params.arg))) };
   }
 
   async evaluateExpressionHandle(params: channels.ElectronApplicationEvaluateExpressionHandleParams): Promise<channels.ElectronApplicationEvaluateExpressionHandleResult> {
-    const handle = await this._object._nodeElectronHandlePromised;
+    const handle = await this._object._nodeElectronHandlePromise;
     const result = await handle.evaluateExpressionAndWaitForSignals(params.expression, params.isFunction, false /* returnByValue */, parseArgument(params.arg));
     return { handle: ElementHandleDispatcher.fromJSHandle(this._scope, result) };
   }

src/dispatchers/pageDispatcher.ts

@@ -93,6 +93,10 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageInitializer> i
       this._dispatchEvent('video', { artifact: existingDispatcher<ArtifactDispatcher>(page._video) });
   }
 
+  page(): Page {
+    return this._page;
+  }
+
   async setDefaultNavigationTimeoutNoReply(params: channels.PageSetDefaultNavigationTimeoutNoReplyParams, metadata: CallMetadata): Promise<void> {
     this._page.setDefaultNavigationTimeout(params.timeout);
   }

src/protocol/channels.ts

@@ -2564,22 +2564,25 @@ export type ElectronLaunchResult = {
 };
 
 // ----------- ElectronApplication -----------
-export type ElectronApplicationInitializer = {};
+export type ElectronApplicationInitializer = {
+  context: BrowserContextChannel,
+};
 export interface ElectronApplicationChannel extends Channel {
-  on(event: 'context', callback: (params: ElectronApplicationContextEvent) => void): this;
   on(event: 'close', callback: (params: ElectronApplicationCloseEvent) => void): this;
-  on(event: 'window', callback: (params: ElectronApplicationWindowEvent) => void): this;
+  browserWindow(params: ElectronApplicationBrowserWindowParams, metadata?: Metadata): Promise<ElectronApplicationBrowserWindowResult>;
   evaluateExpression(params: ElectronApplicationEvaluateExpressionParams, metadata?: Metadata): Promise<ElectronApplicationEvaluateExpressionResult>;
   evaluateExpressionHandle(params: ElectronApplicationEvaluateExpressionHandleParams, metadata?: Metadata): Promise<ElectronApplicationEvaluateExpressionHandleResult>;
   close(params?: ElectronApplicationCloseParams, metadata?: Metadata): Promise<ElectronApplicationCloseResult>;
 }
-export type ElectronApplicationContextEvent = {
-  context: BrowserContextChannel,
-};
 export type ElectronApplicationCloseEvent = {};
-export type ElectronApplicationWindowEvent = {
+export type ElectronApplicationBrowserWindowParams = {
   page: PageChannel,
-  browserWindow: JSHandleChannel,
+};
+export type ElectronApplicationBrowserWindowOptions = {
+
+};
+export type ElectronApplicationBrowserWindowResult = {
+  handle: JSHandleChannel,
 };
 export type ElectronApplicationEvaluateExpressionParams = {
   expression: string,

src/protocol/protocol.yml

@@ -2097,8 +2097,17 @@ Electron:
 ElectronApplication:
   type: interface
 
+  initializer:
+    context: BrowserContext
+
   commands:
 
+    browserWindow:
+      parameters:
+        page: Page
+      returns:
+        handle: JSHandle
+
     evaluateExpression:
       parameters:
         expression: string
@@ -2118,20 +2127,8 @@ ElectronApplication:
     close:
 
   events:
-
-    # This event happens once immediately after creation.
-    context:
-      parameters:
-        context: BrowserContext
-
     close:
 
-    window:
-      parameters:
-        page: Page
-        browserWindow: JSHandle
-
-
 
 Android:
   type: interface

src/protocol/validator.ts

@@ -974,6 +974,9 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
     env: tOptional(tArray(tType('NameValue'))),
     timeout: tOptional(tNumber),
   });
+  scheme.ElectronApplicationBrowserWindowParams = tObject({
+    page: tChannel('Page'),
+  });
   scheme.ElectronApplicationEvaluateExpressionParams = tObject({
     expression: tString,
     isFunction: tOptional(tBoolean),

src/server/electron/electron.ts

@@ -43,24 +43,16 @@ export type ElectronLaunchOptionsBase = {
   timeout?: number,
 };
 
-export interface ElectronPage extends Page {
-  browserWindow: js.JSHandle<BrowserWindow>;
-  _browserWindowId: number;
-}
-
 export class ElectronApplication extends SdkObject {
   static Events = {
     Close: 'close',
-    Window: 'window',
   };
 
   private _browserContext: CRBrowserContext;
   private _nodeConnection: CRConnection;
   private _nodeSession: CRSession;
   private _nodeExecutionContext: js.ExecutionContext | undefined;
-  _nodeElectronHandlePromised: Promise<js.JSHandle<any>>;
-  private _resolveNodeElectronHandle!: (handle: js.JSHandle<any>) => void;
-  private _windows = new Set<ElectronPage>();
+  _nodeElectronHandlePromise: Promise<js.JSHandle<any>>;
   private _lastWindowId = 0;
   readonly _timeoutSettings = new TimeoutSettings();
 
@@ -74,28 +66,21 @@ export class ElectronApplication extends SdkObject {
     this._browserContext.on(BrowserContext.Events.Page, event => this._onPage(event));
     this._nodeConnection = nodeConnection;
     this._nodeSession = nodeConnection.rootSession;
-    this._nodeElectronHandlePromised = new Promise(resolve => this._resolveNodeElectronHandle = resolve);
+    this._nodeElectronHandlePromise = new Promise(f => {
+      this._nodeSession.on('Runtime.executionContextCreated', async (event: any) => {
+        if (event.context.auxData && event.context.auxData.isDefault) {
+          this._nodeExecutionContext = new js.ExecutionContext(this, new CRExecutionContext(this._nodeSession, event.context));
+          f(await js.evaluate(this._nodeExecutionContext, false /* returnByValue */, `process.mainModule.require('electron')`));
+        }
+      });
+    });
+    this._nodeSession.send('Runtime.enable', {}).catch(e => {});
   }
 
-  private async _onPage(page: ElectronPage) {
+  private async _onPage(page: Page) {
     // Needs to be sync.
     const windowId = ++this._lastWindowId;
-    page.on(Page.Events.Close, () => {
-      if (page.browserWindow)
-        page.browserWindow.dispose();
-      this._windows.delete(page);
-    });
-    page._browserWindowId = windowId;
-    this._windows.add(page);
-
-    // Below is async.
-    const handle = await (await this._nodeElectronHandlePromised).evaluateHandle(({ BrowserWindow }, windowId) => BrowserWindow.fromId(windowId), windowId).catch(e => {});
-    if (!handle)
-      return;
-    page.browserWindow = handle;
-    const controller = new ProgressController(internalCallMetadata(), this);
-    await controller.run(progress => page.mainFrame()._waitForLoadState(progress, 'domcontentloaded'), page._timeoutSettings.navigationTimeout({})).catch(e => {}); // can happen after detach
-    this.emit(ElectronApplication.Events.Window, page);
+    (page as any)._browserWindowId = windowId;
   }
 
   context(): BrowserContext {
@@ -105,19 +90,15 @@ export class ElectronApplication extends SdkObject {
   async close() {
     const progressController = new ProgressController(internalCallMetadata(), this);
     const closed = progressController.run(progress => helper.waitForEvent(progress, this, ElectronApplication.Events.Close).promise, this._timeoutSettings.timeout({}));
-    await (await this._nodeElectronHandlePromised).evaluate(({ app }) => app.quit());
+    const electronHandle = await this._nodeElectronHandlePromise;
+    await electronHandle.evaluate(({ app }) => app.quit());
     this._nodeConnection.close();
     await closed;
   }
 
-  async _init()  {
-    this._nodeSession.on('Runtime.executionContextCreated', (event: any) => {
-      if (event.context.auxData && event.context.auxData.isDefault)
-        this._nodeExecutionContext = new js.ExecutionContext(this, new CRExecutionContext(this._nodeSession, event.context));
-    });
-    await this._nodeSession.send('Runtime.enable', {}).catch(e => {});
-    js.evaluate(this._nodeExecutionContext!, false /* returnByValue */, `process.mainModule.require('electron')`)
-        .then(this._resolveNodeElectronHandle);
+  async browserWindow(page: Page): Promise<js.JSHandle<BrowserWindow>> {
+    const electronHandle = await this._nodeElectronHandlePromise;
+    return await electronHandle.evaluateHandle(({ BrowserWindow }, windowId) => BrowserWindow.fromId(windowId), (page as any)._browserWindowId);
   }
 }
 
@@ -188,7 +169,6 @@ export class Electron extends SdkObject {
       };
       const browser = await CRBrowser.connect(chromeTransport, browserOptions);
       app = new ElectronApplication(this, browser, nodeConnection);
-      await app._init();
       return app;
     }, TimeoutSettings.timeout(options));
   }

tests/config/electron-app.js

@@ -1,3 +1,3 @@
-const { app, BrowserWindow } = require('electron');
+const { app } = require('electron');
 
 app.on('window-all-closed', e => e.preventDefault());

tests/config/electron-window-app.js

@@ -0,0 +1,11 @@
+const { app, BrowserWindow } = require('electron');
+
+app.whenReady().then(() => {
+  const win = new BrowserWindow({
+    width: 800,
+    height: 600,
+  });
+  win.loadURL('about:blank');
+})
+
+app.on('window-all-closed', e => e.preventDefault());