Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/client/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,18 @@ export class ElectronApplication extends ChannelOwner<channels.ElectronApplicati
constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.ElectronApplicationInitializer) {
super(parent, type, guid, initializer);
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));
});
for (const page of this._context._pages)
this._onPage(page);
this._context.on(Events.BrowserContext.Page, page => this._onPage(page));
this._channel.on('close', () => this.emit(Events.ElectronApplication.Close));
}

_onPage(page: Page) {
this._windows.add(page);
this.emit(Events.ElectronApplication.Window, page);
page.once(Events.Page.Close, () => this._windows.delete(page));
}

windows(): Page[] {
// TODO: add ElectronPage class inherting from Page.
return [...this._windows];
Expand Down
14 changes: 11 additions & 3 deletions src/server/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,16 @@ export class CRBrowser extends Browser {
return browser;
}
browser._defaultContext = new CRBrowserContext(browser, undefined, options.persistent);

await Promise.all([
session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }),
session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }).then(async () => {
// Target.setAutoAttach has a bug where it does not wait for new Targets being attached.
// However making a dummy call afterwards fixes this.
// This can be removed after https://chromium-review.googlesource.com/c/chromium/src/+/2885888 lands in stable.
await session.send('Target.getTargetInfo');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to have code with setDiscoverTargets which did this, maybe it's easier to just revert that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code with setDiscoverTargets was way more complicated had bugs in it related to connecting twice to service workers.

}),
(browser._defaultContext as CRBrowserContext)._initialize(),
]);

await browser._waitForAllPagesToBeInitialized();
return browser;
}

Expand Down Expand Up @@ -104,6 +108,10 @@ export class CRBrowser extends Browser {
return this.options.name === 'clank';
}

async _waitForAllPagesToBeInitialized() {
await Promise.all([...this._crPages.values()].map(page => page.pageOrError()));
}

_onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
if (targetInfo.type === 'browser')
return;
Expand Down
4 changes: 3 additions & 1 deletion src/server/electron/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ 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;
Expand All @@ -77,7 +79,7 @@ export class ElectronApplication extends SdkObject {
this._nodeSession.send('Runtime.enable', {}).catch(e => {});
}

private async _onPage(page: Page) {
private _onPage(page: Page) {
// Needs to be sync.
const windowId = ++this._lastWindowId;
(page as any)._browserWindowId = windowId;
Expand Down
1 change: 0 additions & 1 deletion tests/chromium/chromium.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,6 @@ playwrightTest('should send extra headers with connect request', async ({browser
});

playwrightTest('should report all pages in an existing browser', async ({ browserType, browserOptions }, testInfo) => {
playwrightTest.fail();
const port = 9339 + testInfo.workerIndex;
const browserServer = await browserType.launch({
...browserOptions,
Expand Down
2 changes: 1 addition & 1 deletion tests/electron/electron-app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ test('should return browser window', async ({ playwright }) => {
const electronApp = await playwright._electron.launch({
args: [path.join(__dirname, 'electron-window-app.js')],
});
const page = await electronApp.waitForEvent('window');
const page = await electronApp.firstWindow();
const bwHandle = await electronApp.browserWindow(page);
expect(await bwHandle.evaluate((bw: BrowserWindow) => bw.title)).toBe('Electron');
await electronApp.close();
Expand Down