Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): fire BrowserContext.Page event in WebKit and Firefox #1186

Merged
merged 6 commits into from
Mar 3, 2020
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
2 changes: 1 addition & 1 deletion src/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
case 'page': {
const page = await target.page();
const event = new PageEvent(page!);
context.emit(CommonEvents.BrowserContext.PageEvent, event);
context.emit(CommonEvents.BrowserContext.Page, event);
break;
}
case 'background_page': {
Expand Down
2 changes: 1 addition & 1 deletion src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const Events = {

BrowserContext: {
Close: 'close',
PageEvent: 'page',
Page: 'page',
},

BrowserServer: {
Expand Down
19 changes: 11 additions & 8 deletions src/firefox/ffBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Events } from '../events';
import { assert, helper, RegisteredListener, debugError } from '../helper';
import * as network from '../network';
import * as types from '../types';
import { Page } from '../page';
import { Page, PageEvent } from '../page';
import { ConnectionEvents, FFConnection, FFSessionEvents, FFSession } from './ffConnection';
import { FFPage } from './ffPage';
import * as platform from '../platform';
Expand Down Expand Up @@ -162,13 +162,16 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
const {targetId} = payload.targetInfo;
const target = this._targets.get(targetId)!;
target._initPagePromise(this._connection.getSession(payload.sessionId)!);
const page = await target.page();
if (!page)
return;
target.context().emit(Events.BrowserContext.Page, new PageEvent(page));

const opener = target.opener();
if (opener && opener._pagePromise) {
const openerPage = await opener._pagePromise;
if (openerPage.listenerCount(Events.Page.Popup)) {
const popupPage = await target.page();
openerPage.emit(Events.Page.Popup, popupPage);
}
if (openerPage.listenerCount(Events.Page.Popup))
openerPage.emit(Events.Page.Popup, page);
}
}

Expand All @@ -189,14 +192,14 @@ class Target {
_pagePromise?: Promise<Page>;
_ffPage: FFPage | null = null;
private readonly _browser: FFBrowser;
private readonly _context: BrowserContext;
private readonly _context: FFBrowserContext;
private readonly _connection: FFConnection;
private readonly _targetId: string;
private readonly _type: 'page' | 'browser';
_url: string;
private readonly _openerId: string | undefined;

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

context(): BrowserContext {
context(): FFBrowserContext {
return this._context;
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/chromium.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class Chromium implements BrowserType {
const { timeout = 30000 } = options || {};
const { browserServer, transport } = await this._launchServer(options, 'persistent', userDataDir);
const browser = await CRBrowser.connect(transport!);
const firstPage = new Promise(r => browser._defaultContext.once(Events.BrowserContext.PageEvent, r));
const firstPage = new Promise(r => browser._defaultContext.once(Events.BrowserContext.Page, r));
await helper.waitWithTimeout(firstPage, 'first page', timeout);
// Hack: for typical launch scenario, ensure that close waits for actual process termination.
const browserContext = browser._defaultContext;
Expand Down
20 changes: 16 additions & 4 deletions src/webkit/wkBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

import { Browser, createPageInNewContext } from '../browser';
import { BrowserContext, BrowserContextOptions, validateBrowserContextOptions, assertBrowserContextIsNotOwned, verifyGeolocation } from '../browserContext';
import { assert, helper, RegisteredListener } from '../helper';
import { assert, helper, RegisteredListener, debugError } from '../helper';
import * as network from '../network';
import { Page } from '../page';
import { Page, PageEvent } from '../page';
import { ConnectionTransport, SlowMoTransport } from '../transport';
import * as types from '../types';
import { Events } from '../events';
Expand Down Expand Up @@ -102,13 +102,13 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
_onPageProxyCreated(event: Protocol.Browser.pageProxyCreatedPayload) {
const { pageProxyInfo } = event;
const pageProxyId = pageProxyInfo.pageProxyId;
let context = null;
let context: WKBrowserContext | null = null;
if (pageProxyInfo.browserContextId) {
// FIXME: we don't know about the default context id, so assume that all targets from
// unknown contexts are created in the 'default' context which can in practice be represented
// by multiple actual contexts in WebKit. Solving this properly will require adding context
// lifecycle events.
context = this._contexts.get(pageProxyInfo.browserContextId);
context = this._contexts.get(pageProxyInfo.browserContextId) || null;
}
if (!context && !this._attachToDefaultContext)
return;
Expand All @@ -125,6 +125,18 @@ export class WKBrowser extends platform.EventEmitter implements Browser {
this._firstPageProxyCallback();
this._firstPageProxyCallback = undefined;
}

pageProxy.page().then(async page => {
if (!page)
return;
context!.emit(Events.BrowserContext.Page, new PageEvent(page));
if (!opener)
return;
const openerPage = await opener.page();
if (!openerPage || page.isClosed())
return;
openerPage.emit(Events.Page.Popup, page);
}).catch(debugError); // Just not emit the event in case of initialization failure.
}

_onPageProxyDestroyed(event: Protocol.Browser.pageProxyDestroyedPayload) {
Expand Down
16 changes: 4 additions & 12 deletions src/webkit/wkPageProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@
* limitations under the License.
*/

import { assert, debugError, helper, RegisteredListener } from '../helper';
import { Page } from '../page';
import { Protocol } from './protocol';
import { WKBrowserContext } from './wkBrowser';
import { WKSession } from './wkConnection';
import { WKPage } from './wkPage';
import { RegisteredListener, helper, assert, debugError } from '../helper';
import { Events } from '../events';
import { WKBrowserContext } from './wkBrowser';

const isPovisionalSymbol = Symbol('isPovisional');

Expand Down Expand Up @@ -122,19 +121,12 @@ export class WKPageProxy {
if (!this._pageProxySession.isDisposed())
error = e;
}
if (targetInfo.isPaused)
this._resumeTarget(targetInfo.targetId);
if (error)
this._pagePromiseReject(error);
else
this._pagePromiseFulfill(page);
if (targetInfo.isPaused)
this._resumeTarget(targetInfo.targetId);
if (page && this._opener) {
this._opener.page().then(openerPage => {
if (!openerPage || page!.isClosed())
return;
openerPage.emit(Events.Page.Popup, page);
});
}
} else {
assert(targetInfo.isProvisional);
(session as any)[isPovisionalSymbol] = true;
Expand Down
119 changes: 118 additions & 1 deletion test/browsercontext.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const utils = require('./utils');
/**
* @type {BrowserTestSuite}
*/
module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WEBKIT}) {
module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, FFOX, WEBKIT}) {
const {describe, xdescribe, fdescribe} = testRunner;
const {it, fit, xit, dit} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;
Expand Down Expand Up @@ -292,4 +292,121 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
await context.close();
});
});

describe('BrowserContext.pages()', function() {
it('should return all of the pages', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const second = await context.newPage();
const allPages = await context.pages();
expect(allPages.length).toBe(2);
expect(allPages).toContain(page);
expect(allPages).toContain(second);
await context.close();
});
});

describe('Events.BrowserContext.Page', function() {
it('should report when a new page is created and closed', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [otherPage] = await Promise.all([
new Promise(r => context.once('page', async event => r(await event.page()))),
page.evaluate(url => window.open(url), server.CROSS_PROCESS_PREFIX + '/empty.html').catch(e => console.log('eee = ' + e)),
]);
await otherPage.waitForLoadState();
expect(otherPage.url()).toContain(server.CROSS_PROCESS_PREFIX);
expect(await otherPage.evaluate(() => ['Hello', 'world'].join(' '))).toBe('Hello world');
expect(await otherPage.$('body')).toBeTruthy();

let allPages = await context.pages();
expect(allPages).toContain(page);
expect(allPages).toContain(otherPage);

let closeEventReceived;
otherPage.once('close', () => closeEventReceived = true);
await otherPage.close();
expect(closeEventReceived).toBeTruthy();

allPages = await context.pages();
expect(allPages).toContain(page);
expect(allPages).not.toContain(otherPage);
await context.close();
});
it('should not report uninitialized pages', async({browser, server}) => {
const context = await browser.newContext();
const pagePromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page())));
context.newPage();
const newPage = await pagePromise;
expect(newPage.url()).toBe('about:blank');

const popupPromise = new Promise(fulfill => context.once('page', async event => fulfill(await event.page())));
const evaluatePromise = newPage.evaluate(() => window.open('about:blank'));
const popup = await popupPromise;
await popup.waitForLoadState();
expect(popup.url()).toBe('about:blank');
await evaluatePromise;
await context.close();
});
it('should not crash while redirecting if original request was missed', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
let serverResponse = null;
server.setRoute('/one-style.css', (req, res) => serverResponse = res);
// Open a new page. Use window.open to connect to the page later.
const [newPage] = await Promise.all([
new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))),
page.evaluate(url => window.open(url), server.PREFIX + '/one-style.html'),
server.waitForRequest('/one-style.css')
]);
// Issue a redirect.
serverResponse.writeHead(302, { location: '/injectedstyle.css' });
serverResponse.end();
// Wait for the new page to load.
await newPage.waitForLoadState();
// Connect to the opened page.
expect(newPage.url()).toBe(server.PREFIX + '/one-style.html');
// Cleanup.
await context.close();
});
it.fail(WEBKIT)('should have an opener', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
await page.goto(server.EMPTY_PAGE);
const [popup] = await Promise.all([
new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))),
page.goto(server.PREFIX + '/popup/window-open.html')
]);
await popup.waitForLoadState();
expect(popup.url()).toBe(server.PREFIX + '/popup/popup.html');
expect(await popup.opener()).toBe(page);
expect(await page.opener()).toBe(null);
await context.close();
});
it('should close all belonging targets once closing context', async function({browser}) {
const context = await browser.newContext();
await context.newPage();
expect((await context.pages()).length).toBe(1);

await context.close();
expect((await context.pages()).length).toBe(0);
});
it('should fire page lifecycle events', async function({browser, server}) {
const context = await browser.newContext();
const events = [];
context.on('page', async event => {
const page = await event.page();
events.push('CREATED: ' + page.url());
page.on('close', () => events.push('DESTROYED: ' + page.url()));
});
const page = await context.newPage();
await page.goto(server.EMPTY_PAGE);
await page.close();
expect(events).toEqual([
'CREATED: about:blank',
`DESTROYED: ${server.EMPTY_PAGE}`
]);
await context.close();
});
});
};
Loading