Skip to content

Commit 31e26a2

Browse files
authored
fix(api): fire BrowserContext.Page event in WebKit and Firefox (#1186)
1 parent 497a74d commit 31e26a2

File tree

8 files changed

+153
-127
lines changed

8 files changed

+153
-127
lines changed

src/chromium/crBrowser.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
9797
case 'page': {
9898
const page = await target.page();
9999
const event = new PageEvent(page!);
100-
context.emit(CommonEvents.BrowserContext.PageEvent, event);
100+
context.emit(CommonEvents.BrowserContext.Page, event);
101101
break;
102102
}
103103
case 'background_page': {

src/events.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export const Events = {
2222

2323
BrowserContext: {
2424
Close: 'close',
25-
PageEvent: 'page',
25+
Page: 'page',
2626
},
2727

2828
BrowserServer: {

src/firefox/ffBrowser.ts

+11-8
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { Events } from '../events';
2121
import { assert, helper, RegisteredListener, debugError } from '../helper';
2222
import * as network from '../network';
2323
import * as types from '../types';
24-
import { Page } from '../page';
24+
import { Page, PageEvent } from '../page';
2525
import { ConnectionEvents, FFConnection, FFSessionEvents, FFSession } from './ffConnection';
2626
import { FFPage } from './ffPage';
2727
import * as platform from '../platform';
@@ -162,13 +162,16 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
162162
const {targetId} = payload.targetInfo;
163163
const target = this._targets.get(targetId)!;
164164
target._initPagePromise(this._connection.getSession(payload.sessionId)!);
165+
const page = await target.page();
166+
if (!page)
167+
return;
168+
target.context().emit(Events.BrowserContext.Page, new PageEvent(page));
169+
165170
const opener = target.opener();
166171
if (opener && opener._pagePromise) {
167172
const openerPage = await opener._pagePromise;
168-
if (openerPage.listenerCount(Events.Page.Popup)) {
169-
const popupPage = await target.page();
170-
openerPage.emit(Events.Page.Popup, popupPage);
171-
}
173+
if (openerPage.listenerCount(Events.Page.Popup))
174+
openerPage.emit(Events.Page.Popup, page);
172175
}
173176
}
174177

@@ -189,14 +192,14 @@ class Target {
189192
_pagePromise?: Promise<Page>;
190193
_ffPage: FFPage | null = null;
191194
private readonly _browser: FFBrowser;
192-
private readonly _context: BrowserContext;
195+
private readonly _context: FFBrowserContext;
193196
private readonly _connection: FFConnection;
194197
private readonly _targetId: string;
195198
private readonly _type: 'page' | 'browser';
196199
_url: string;
197200
private readonly _openerId: string | undefined;
198201

199-
constructor(connection: any, browser: FFBrowser, context: BrowserContext, targetId: string, type: 'page' | 'browser', url: string, openerId: string | undefined) {
202+
constructor(connection: any, browser: FFBrowser, context: FFBrowserContext, targetId: string, type: 'page' | 'browser', url: string, openerId: string | undefined) {
200203
this._browser = browser;
201204
this._context = context;
202205
this._connection = connection;
@@ -223,7 +226,7 @@ class Target {
223226
return this._url;
224227
}
225228

226-
context(): BrowserContext {
229+
context(): FFBrowserContext {
227230
return this._context;
228231
}
229232

src/server/chromium.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class Chromium implements BrowserType {
7070
const { timeout = 30000 } = options || {};
7171
const { browserServer, transport } = await this._launchServer(options, 'persistent', userDataDir);
7272
const browser = await CRBrowser.connect(transport!);
73-
const firstPage = new Promise(r => browser._defaultContext.once(Events.BrowserContext.PageEvent, r));
73+
const firstPage = new Promise(r => browser._defaultContext.once(Events.BrowserContext.Page, r));
7474
await helper.waitWithTimeout(firstPage, 'first page', timeout);
7575
// Hack: for typical launch scenario, ensure that close waits for actual process termination.
7676
const browserContext = browser._defaultContext;

src/webkit/wkBrowser.ts

+16-4
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
import { Browser, createPageInNewContext } from '../browser';
1919
import { BrowserContext, BrowserContextOptions, validateBrowserContextOptions, assertBrowserContextIsNotOwned, verifyGeolocation } from '../browserContext';
20-
import { assert, helper, RegisteredListener } from '../helper';
20+
import { assert, helper, RegisteredListener, debugError } from '../helper';
2121
import * as network from '../network';
22-
import { Page } from '../page';
22+
import { Page, PageEvent } from '../page';
2323
import { ConnectionTransport, SlowMoTransport } from '../transport';
2424
import * as types from '../types';
2525
import { Events } from '../events';
@@ -102,13 +102,13 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
102102
_onPageProxyCreated(event: Protocol.Browser.pageProxyCreatedPayload) {
103103
const { pageProxyInfo } = event;
104104
const pageProxyId = pageProxyInfo.pageProxyId;
105-
let context = null;
105+
let context: WKBrowserContext | null = null;
106106
if (pageProxyInfo.browserContextId) {
107107
// FIXME: we don't know about the default context id, so assume that all targets from
108108
// unknown contexts are created in the 'default' context which can in practice be represented
109109
// by multiple actual contexts in WebKit. Solving this properly will require adding context
110110
// lifecycle events.
111-
context = this._contexts.get(pageProxyInfo.browserContextId);
111+
context = this._contexts.get(pageProxyInfo.browserContextId) || null;
112112
}
113113
if (!context && !this._attachToDefaultContext)
114114
return;
@@ -125,6 +125,18 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
125125
this._firstPageProxyCallback();
126126
this._firstPageProxyCallback = undefined;
127127
}
128+
129+
pageProxy.page().then(async page => {
130+
if (!page)
131+
return;
132+
context!.emit(Events.BrowserContext.Page, new PageEvent(page));
133+
if (!opener)
134+
return;
135+
const openerPage = await opener.page();
136+
if (!openerPage || page.isClosed())
137+
return;
138+
openerPage.emit(Events.Page.Popup, page);
139+
}).catch(debugError); // Just not emit the event in case of initialization failure.
128140
}
129141

130142
_onPageProxyDestroyed(event: Protocol.Browser.pageProxyDestroyedPayload) {

src/webkit/wkPageProxy.ts

+4-12
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@
1414
* limitations under the License.
1515
*/
1616

17+
import { assert, debugError, helper, RegisteredListener } from '../helper';
1718
import { Page } from '../page';
1819
import { Protocol } from './protocol';
20+
import { WKBrowserContext } from './wkBrowser';
1921
import { WKSession } from './wkConnection';
2022
import { WKPage } from './wkPage';
21-
import { RegisteredListener, helper, assert, debugError } from '../helper';
22-
import { Events } from '../events';
23-
import { WKBrowserContext } from './wkBrowser';
2423

2524
const isPovisionalSymbol = Symbol('isPovisional');
2625

@@ -122,19 +121,12 @@ export class WKPageProxy {
122121
if (!this._pageProxySession.isDisposed())
123122
error = e;
124123
}
124+
if (targetInfo.isPaused)
125+
this._resumeTarget(targetInfo.targetId);
125126
if (error)
126127
this._pagePromiseReject(error);
127128
else
128129
this._pagePromiseFulfill(page);
129-
if (targetInfo.isPaused)
130-
this._resumeTarget(targetInfo.targetId);
131-
if (page && this._opener) {
132-
this._opener.page().then(openerPage => {
133-
if (!openerPage || page!.isClosed())
134-
return;
135-
openerPage.emit(Events.Page.Popup, page);
136-
});
137-
}
138130
} else {
139131
assert(targetInfo.isProvisional);
140132
(session as any)[isPovisionalSymbol] = true;

test/browsercontext.spec.js

+118-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const utils = require('./utils');
2020
/**
2121
* @type {BrowserTestSuite}
2222
*/
23-
module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WEBKIT}) {
23+
module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FFOX, WEBKIT}) {
2424
const {describe, xdescribe, fdescribe} = testRunner;
2525
const {it, fit, xit, dit} = testRunner;
2626
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
@@ -292,4 +292,121 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
292292
await context.close();
293293
});
294294
});
295+
296+
describe('BrowserContext.pages()', function() {
297+
it('should return all of the pages', async({browser, server}) => {
298+
const context = await browser.newContext();
299+
const page = await context.newPage();
300+
const second = await context.newPage();
301+
const allPages = await context.pages();
302+
expect(allPages.length).toBe(2);
303+
expect(allPages).toContain(page);
304+
expect(allPages).toContain(second);
305+
await context.close();
306+
});
307+
});
308+
309+
describe('Events.BrowserContext.Page', function() {
310+
it('should report when a new page is created and closed', async({browser, server}) => {
311+
const context = await browser.newContext();
312+
const page = await context.newPage();
313+
const [otherPage] = await Promise.all([
314+
new Promise(r => context.once('page', async event => r(await event.page()))),
315+
page.evaluate(url => window.open(url), server.CROSS_PROCESS_PREFIX + '/empty.html').catch(e => console.log('eee = ' + e)),
316+
]);
317+
await otherPage.waitForLoadState();
318+
expect(otherPage.url()).toContain(server.CROSS_PROCESS_PREFIX);
319+
expect(await otherPage.evaluate(() => ['Hello', 'world'].join(' '))).toBe('Hello world');
320+
expect(await otherPage.$('body')).toBeTruthy();
321+
322+
let allPages = await context.pages();
323+
expect(allPages).toContain(page);
324+
expect(allPages).toContain(otherPage);
325+
326+
let closeEventReceived;
327+
otherPage.once('close', () => closeEventReceived = true);
328+
await otherPage.close();
329+
expect(closeEventReceived).toBeTruthy();
330+
331+
allPages = await context.pages();
332+
expect(allPages).toContain(page);
333+
expect(allPages).not.toContain(otherPage);
334+
await context.close();
335+
});
336+
it('should not report uninitialized pages', async({browser, server}) => {
337+
const context = await browser.newContext();
338+
const pagePromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page())));
339+
context.newPage();
340+
const newPage = await pagePromise;
341+
expect(newPage.url()).toBe('about:blank');
342+
343+
const popupPromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page())));
344+
const evaluatePromise = newPage.evaluate(() => window.open('about:blank'));
345+
const popup = await popupPromise;
346+
await popup.waitForLoadState();
347+
expect(popup.url()).toBe('about:blank');
348+
await evaluatePromise;
349+
await context.close();
350+
});
351+
it('should not crash while redirecting if original request was missed', async({browser, server}) => {
352+
const context = await browser.newContext();
353+
const page = await context.newPage();
354+
let serverResponse = null;
355+
server.setRoute('/one-style.css', (req, res) => serverResponse = res);
356+
// Open a new page. Use window.open to connect to the page later.
357+
const [newPage] = await Promise.all([
358+
new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))),
359+
page.evaluate(url => window.open(url), server.PREFIX + '/one-style.html'),
360+
server.waitForRequest('/one-style.css')
361+
]);
362+
// Issue a redirect.
363+
serverResponse.writeHead(302, { location: '/injectedstyle.css' });
364+
serverResponse.end();
365+
// Wait for the new page to load.
366+
await newPage.waitForLoadState();
367+
// Connect to the opened page.
368+
expect(newPage.url()).toBe(server.PREFIX + '/one-style.html');
369+
// Cleanup.
370+
await context.close();
371+
});
372+
it.fail(WEBKIT)('should have an opener', async({browser, server}) => {
373+
const context = await browser.newContext();
374+
const page = await context.newPage();
375+
await page.goto(server.EMPTY_PAGE);
376+
const [popup] = await Promise.all([
377+
new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))),
378+
page.goto(server.PREFIX + '/popup/window-open.html')
379+
]);
380+
await popup.waitForLoadState();
381+
expect(popup.url()).toBe(server.PREFIX + '/popup/popup.html');
382+
expect(await popup.opener()).toBe(page);
383+
expect(await page.opener()).toBe(null);
384+
await context.close();
385+
});
386+
it('should close all belonging targets once closing context', async function({browser}) {
387+
const context = await browser.newContext();
388+
await context.newPage();
389+
expect((await context.pages()).length).toBe(1);
390+
391+
await context.close();
392+
expect((await context.pages()).length).toBe(0);
393+
});
394+
it('should fire page lifecycle events', async function({browser, server}) {
395+
const context = await browser.newContext();
396+
const events = [];
397+
context.on('page', async event => {
398+
const page = await event.page();
399+
events.push('CREATED: ' + page.url());
400+
page.on('close', () => events.push('DESTROYED: ' + page.url()));
401+
});
402+
const page = await context.newPage();
403+
await page.goto(server.EMPTY_PAGE);
404+
await page.close();
405+
expect(events).toEqual([
406+
'CREATED: about:blank',
407+
`DESTROYED: ${server.EMPTY_PAGE}`
408+
]);
409+
await context.close();
410+
});
411+
});
295412
};

0 commit comments

Comments
 (0)