Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
yury-s committed Feb 29, 2020
1 parent 1107d0e commit 53efc49
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 19 deletions.
14 changes: 7 additions & 7 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ await context.close();

<!-- GEN:toc -->
- [event: 'close'](#event-close)
- [event: 'pageevent'](#event-pageevent)
- [event: 'page'](#event-page)
- [browserContext.addInitScript(script[, ...args])](#browsercontextaddinitscriptscript-args)
- [browserContext.clearCookies()](#browsercontextclearcookies)
- [browserContext.clearPermissions()](#browsercontextclearpermissions)
Expand All @@ -287,7 +287,7 @@ Emitted when Browser context gets closed. This might happen because of one of th
- Browser application is closed or crashed.
- The [`browser.close`](#browserclose) method was called.

#### event: 'pageevent'
#### event: 'page'
- <[PageEvent]>

Emitted when a new Page is created in the BrowserContext. The event will also fire for popup
Expand Down Expand Up @@ -1670,7 +1670,7 @@ This method returns all of the dedicated [WebWorkers](https://developer.mozilla.
### class: PageEvent

Event object passed to the listeners of 'pageevent' on 'BrowserContext'. Provides access
Event object passed to the listeners of ['page'](#event-page) on [`BrowserContext`](#class-browsercontext). Provides access
to the newly created page.

#### pageEvent.page()
Expand Down Expand Up @@ -3619,7 +3619,7 @@ const backgroundPage = await backroundPageTarget.page();
<!-- GEN:stop -->
<!-- GEN:toc-extends-BrowserContext -->
- [event: 'close'](#event-close)
- [event: 'pageevent'](#event-pageevent)
- [event: 'page'](#event-page)
- [browserContext.addInitScript(script[, ...args])](#browsercontextaddinitscriptscript-args)
- [browserContext.clearCookies()](#browsercontextclearcookies)
- [browserContext.clearPermissions()](#browsercontextclearpermissions)
Expand All @@ -3636,14 +3636,14 @@ const backgroundPage = await backroundPageTarget.page();
<!-- GEN:stop -->

#### event: 'backgroundpage'
- <[Page]>
- <[PageEvent]>

Emitted when new background page is created in the context.

> **NOTE** Only works with persistent context.
#### event: 'serviceworker'
- <[Page]>
- <[Worker]>

Emitted when new service worker is created in the context.

Expand Down Expand Up @@ -3884,7 +3884,7 @@ const { chromium } = require('playwright');
[Mouse]: #class-mouse "Mouse"
[Object]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object "Object"
[Page]: #class-page "Page"
[PageEvent]: #class-pageevent "PageEvent"
[PageEvent]: #class-page "PageEvent"
[Playwright]: #class-playwright "Playwright"
[Promise]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise "Promise"
[RegExp]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
Expand Down
3 changes: 2 additions & 1 deletion src/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ export class CRBrowser extends platform.EventEmitter implements Browser {
}
case 'background_page': {
const page = await target.page();
context.emit(Events.CRBrowserContext.BackgroundPage, page);
const event = new PageEvent(page!);
context.emit(Events.CRBrowserContext.BackgroundPage, event);
break;
}
case 'service_worker': {
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: 'pageevent',
PageEvent: 'page',
},

BrowserServer: {
Expand Down
4 changes: 2 additions & 2 deletions src/platform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ export class WebSocketTransport implements ConnectionTransport {
this._ws.addEventListener('error', () => {});
}

async send(message: string) {
const error = await this._connectPromise;
send(message: string) {
this._connectPromise;
if (error)
throw error;
this._ws.send(message);
Expand Down
14 changes: 7 additions & 7 deletions test/chromium/chromium.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
const {it, fit, xit, dit} = testRunner;
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner;

describe('BrowserContext', function() {
describe('BrowserContext', function() {
it('pages() should return all of the pages', async({page, server, context}) => {
const second = await page.context().newPage();
const allPages = await context.pages();
Expand All @@ -35,7 +35,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
});
it('should report when a new page is created and closed', async({browser, page, server, context}) => {
const [otherPage] = await Promise.all([
new Promise(r => context.once('pageevent', async event => r(await event.page()))),
new Promise(r => context.once('page', async event => r(await event.page()))),
page.evaluate(url => window.open(url), server.CROSS_PROCESS_PREFIX + '/empty.html'),
]);
expect(otherPage.url()).toContain(server.CROSS_PROCESS_PREFIX);
Expand Down Expand Up @@ -72,12 +72,12 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
expect(serviceWorkerCreated).not.toBeTruthy();
});
it('should not report uninitialized pages', async({browser, context}) => {
const pagePromise = new Promise(fulfill => context.once('pageevent', async event => fulfill(await event.page())));
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('pageevent', async event => fulfill(await event.page())));
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;
expect(popup.url()).toBe('about:blank');
Expand All @@ -89,7 +89,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
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('pageevent', async event => fulfill(await event.page()))),
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')
]);
Expand All @@ -106,7 +106,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
it('should have an opener', async({browser, page, server, context}) => {
await page.goto(server.EMPTY_PAGE);
const [popup] = await Promise.all([
new Promise(fulfill => context.once('pageevent', async event => fulfill(await event.page()))),
new Promise(fulfill => context.once('page', async event => fulfill(await event.page()))),
page.goto(server.PREFIX + '/popup/window-open.html')
]);
await popup.waitForLoadState();
Expand All @@ -125,7 +125,7 @@ module.exports.describe = function({testRunner, expect, playwright, FFOX, CHROMI
it('should fire page lifecycle events', async function({browser, server}) {
const context = await browser.newContext();
const events = [];
context.on('pageevent', async event => {
context.on('page', async event => {
const page = await event.page();
events.push('CREATED: ' + page.url());
page.on('close', () => events.push('DESTROYED: ' + page.url()))
Expand Down
2 changes: 1 addition & 1 deletion test/chromium/headful.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ module.exports.describe = function({testRunner, expect, playwright, defaultBrows
const backgroundPages = await context.backgroundPages();
let backgroundPage = backgroundPages.length
? backgroundPages[0]
: await new Promise(fulfill => context.once('backgroundpage', fulfill(page)));
: await new Promise(fulfill => context.once('backgroundpage', async event => fulfill(await event.page())));
expect(backgroundPage).toBeTruthy();
expect(await context.backgroundPages()).toContain(backgroundPage);
expect(await context.pages()).not.toContain(backgroundPage);
Expand Down

0 comments on commit 53efc49

Please sign in to comment.