fix(electron): browser window ids out of sync (#8759)

src/server/electron/electron.ts

@@ -19,6 +19,7 @@ import os from 'os';
 import path from 'path';
 import { CRBrowser, CRBrowserContext } from '../chromium/crBrowser';
 import { CRConnection, CRSession } from '../chromium/crConnection';
+import { CRPage } from '../chromium/crPage';
 import { CRExecutionContext } from '../chromium/crExecutionContext';
 import * as js from '../javascript';
 import { Page } from '../page';
@@ -49,7 +50,6 @@ export class ElectronApplication extends SdkObject {
   private _nodeSession: CRSession;
   private _nodeExecutionContext: js.ExecutionContext | undefined;
   _nodeElectronHandlePromise: Promise<js.JSHandle<any>>;
-  private _lastWindowId = 0;
   readonly _timeoutSettings = new TimeoutSettings();
 
   constructor(parent: SdkObject, browser: CRBrowser, nodeConnection: CRConnection) {
@@ -59,9 +59,6 @@ export class ElectronApplication extends SdkObject {
       // Emit application closed after context closed.
       Promise.resolve().then(() => this.emit(ElectronApplication.Events.Close));
     });
-    for (const page of this._browserContext.pages())
-      this._onPage(page);
-    this._browserContext.on(BrowserContext.Events.Page, event => this._onPage(event));
     this._nodeConnection = nodeConnection;
     this._nodeSession = nodeConnection.rootSession;
     this._nodeElectronHandlePromise = new Promise(f => {
@@ -75,12 +72,6 @@ export class ElectronApplication extends SdkObject {
     this._nodeSession.send('Runtime.enable', {}).catch(e => {});
   }
 
-  private _onPage(page: Page) {
-    // Needs to be sync.
-    const windowId = ++this._lastWindowId;
-    (page as any)._browserWindowId = windowId;
-  }
-
   context(): BrowserContext {
     return this._browserContext;
   }
@@ -95,8 +86,13 @@ export class ElectronApplication extends SdkObject {
   }
 
   async browserWindow(page: Page): Promise<js.JSHandle<BrowserWindow>> {
+    // Assume CRPage as Electron is always Chromium.
+    const targetId = (page._delegate as CRPage)._targetId;
     const electronHandle = await this._nodeElectronHandlePromise;
-    return await electronHandle.evaluateHandle(({ BrowserWindow }, windowId) => BrowserWindow.fromId(windowId), (page as any)._browserWindowId);
+    return await electronHandle.evaluateHandle(({ BrowserWindow, webContents }, targetId) => {
+      const wc = webContents.fromDevToolsTargetId(targetId);
+      return BrowserWindow.fromWebContents(wc);
+    }, targetId);
   }
 }
 

tests/electron/electron-app.spec.ts

@@ -126,3 +126,44 @@ test('should bypass csp', async ({ playwright, server }) => {
   expect(await page.evaluate('window["__injected"]')).toBe(42);
   await app.close();
 });
+
+test('should create page for browser view', async ({ playwright }) => {
+  const app = await playwright._electron.launch({
+    args: [path.join(__dirname, 'electron-window-app.js')],
+  });
+  const browserViewPagePromise = app.waitForEvent('window');
+  await app.evaluate(async electron => {
+    const window = electron.BrowserWindow.getAllWindows()[0];
+    const view = new electron.BrowserView();
+    window.addBrowserView(view);
+    await view.webContents.loadURL('about:blank');
+    view.setBounds({ x: 0, y: 0, width: 256, height: 256 });
+  });
+  await browserViewPagePromise;
+  expect(app.windows()).toHaveLength(2);
+  await app.close();
+});
+
+test('should return same browser window for browser view pages', async ({ playwright }) => {
+  const app = await playwright._electron.launch({
+    args: [path.join(__dirname, 'electron-window-app.js')],
+  });
+  const browserViewPagePromise = app.waitForEvent('window');
+  await app.evaluate(async electron => {
+    const window = electron.BrowserWindow.getAllWindows()[0];
+    const view = new electron.BrowserView();
+    window.addBrowserView(view);
+    await view.webContents.loadURL('about:blank');
+    view.setBounds({ x: 0, y: 0, width: 256, height: 256 });
+  });
+  await browserViewPagePromise;
+  const [firstWindowId, secondWindowId] = await Promise.all(
+      app.windows().map(async page => {
+        const bwHandle = await app.browserWindow(page);
+        const id = await bwHandle.evaluate((bw: BrowserWindow) => bw.id);
+        return id;
+      })
+  );
+  expect(firstWindowId).toEqual(secondWindowId);
+  await app.close();
+});