Skip to content

Commit

Permalink
feat(rpc): support more chromium-specific apis (#2883)
Browse files Browse the repository at this point in the history
This includes page CDPSession, backgroundPages() and serviceWorkers().

This has also revealed an issue with closing order between the context
and the service worker.
  • Loading branch information
dgozman authored Jul 9, 2020
1 parent b3ca4af commit 8fe29fe
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ProxySettings } from './types';
import { LoggerSink } from './loggerSink';

export type BrowserOptions = {
name: string,
loggers: Loggers,
downloadsPath?: string,
headful?: boolean,
Expand Down
12 changes: 9 additions & 3 deletions src/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
readonly _options: BrowserContextOptions;
_routes: { url: types.URLMatch, handler: network.RouteHandler }[] = [];
private _isPersistentContext: boolean;
private _startedClosing = false;
private _closedStatus: 'open' | 'closing' | 'closed' = 'open';
readonly _closePromise: Promise<Error>;
private _closePromiseFulfill: ((error: Error) => void) | undefined;
readonly _permissions = new Map<string, string[]>();
Expand Down Expand Up @@ -109,6 +109,12 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
}

private _didCloseInternal() {
if (this._closedStatus === 'closed') {
// We can come here twice if we close browser context and browser
// at the same time.
return;
}
this._closedStatus = 'closed';
this._downloads.clear();
this._closePromiseFulfill!(new Error('Context closed'));
this.emit(Events.BrowserContext.Close);
Expand Down Expand Up @@ -235,8 +241,8 @@ export abstract class BrowserContextBase extends EventEmitter implements Browser
await this._browserBase.close();
return;
}
if (!this._startedClosing) {
this._startedClosing = true;
if (this._closedStatus === 'open') {
this._closedStatus = 'closing';
await this._doClose();
await Promise.all([...this._downloads].map(d => d.delete()));
this._didCloseInternal();
Expand Down
10 changes: 10 additions & 0 deletions src/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,16 @@ export class CRBrowserContext extends BrowserContextBase {
assert(this._browserContextId);
await this._browser._session.send('Target.disposeBrowserContext', { browserContextId: this._browserContextId });
this._browser._contexts.delete(this._browserContextId);
for (const [targetId, serviceWorker] of this._browser._serviceWorkers) {
if (serviceWorker._browserContext !== this)
continue;
// When closing a browser context, service workers are shutdown
// asynchronously and we get detached from them later.
// To avoid the wrong order of notifications, we manually fire
// "close" event here and forget about the serivce worker.
serviceWorker.emit(CommonEvents.Worker.Close);
this._browser._serviceWorkers.delete(targetId);
}
}

backgroundPages(): Page[] {
Expand Down
11 changes: 8 additions & 3 deletions src/rpc/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export interface BrowserChannel extends Channel {
close(): Promise<void>;
newContext(params: types.BrowserContextOptions): Promise<BrowserContextChannel>;

// Chromium-specific.
newBrowserCDPSession(): Promise<CDPSessionChannel>;
crNewBrowserCDPSession(): Promise<CDPSessionChannel>;
}
export type BrowserInitializer = {};

Expand Down Expand Up @@ -92,9 +91,15 @@ export interface BrowserContextChannel extends Channel {
setNetworkInterceptionEnabled(params: { enabled: boolean }): Promise<void>;
setOffline(params: { offline: boolean }): Promise<void>;
waitForEvent(params: { event: string }): Promise<any>;

on(event: 'crBackgroundPage', callback: (params: PageChannel) => void): this;
on(event: 'crServiceWorker', callback: (params: WorkerChannel) => void): this;
crNewCDPSession(params: { page: PageChannel }): Promise<CDPSessionChannel>;
}
export type BrowserContextInitializer = {
pages: PageChannel[]
pages: PageChannel[],
crBackgroundPages: PageChannel[],
crServiceWorkers: WorkerChannel[],
};


Expand Down
3 changes: 1 addition & 2 deletions src/rpc/client/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ export class Browser extends ChannelOwner<BrowserChannel, BrowserInitializer> {
await this._channel.close();
}

// Chromium-specific.
async newBrowserCDPSession(): Promise<CDPSession> {
return CDPSession.from(await this._channel.newBrowserCDPSession());
return CDPSession.from(await this._channel.crNewBrowserCDPSession());
}
}
42 changes: 41 additions & 1 deletion src/rpc/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@ import { Browser } from './browser';
import { ConnectionScope } from './connection';
import { Events } from '../../events';
import { TimeoutSettings } from '../../timeoutSettings';
import { CDPSession } from './cdpSession';
import { Events as ChromiumEvents } from '../../chromium/events';
import { Worker } from './worker';

export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserContextInitializer> {
_pages = new Set<Page>();
_crBackgroundPages = new Set<Page>();
_crServiceWorkers = new Set<Worker>();
private _routes: { url: types.URLMatch, handler: network.RouteHandler }[] = [];
_browser: Browser | undefined;
readonly _bindings = new Map<string, frames.FunctionWithSource>();
Expand All @@ -46,7 +51,7 @@ export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserC

constructor(scope: ConnectionScope, guid: string, initializer: BrowserContextInitializer) {
super(scope, guid, initializer, true);
initializer.pages.map(p => {
initializer.pages.forEach(p => {
const page = Page.from(p);
this._pages.add(page);
page._setBrowserContext(this);
Expand All @@ -55,6 +60,29 @@ export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserC
this._channel.on('close', () => this._onClose());
this._channel.on('page', page => this._onPage(Page.from(page)));
this._channel.on('route', ({ route, request }) => this._onRoute(network.Route.from(route), network.Request.from(request)));

initializer.crBackgroundPages.forEach(p => {
const page = Page.from(p);
this._crBackgroundPages.add(page);
page._setBrowserContext(this);
});
this._channel.on('crBackgroundPage', pageChannel => {
const page = Page.from(pageChannel);
page._setBrowserContext(this);
this._crBackgroundPages.add(page);
this.emit(ChromiumEvents.CRBrowserContext.BackgroundPage, page);
});
initializer.crServiceWorkers.forEach(w => {
const worker = Worker.from(w);
worker._context = this;
this._crServiceWorkers.add(worker);
});
this._channel.on('crServiceWorker', serviceWorkerChannel => {
const worker = Worker.from(serviceWorkerChannel);
worker._context = this;
this._crServiceWorkers.add(worker);
this.emit(ChromiumEvents.CRBrowserContext.ServiceWorker, worker);
});
}

private _onPage(page: Page): void {
Expand Down Expand Up @@ -199,4 +227,16 @@ export class BrowserContext extends ChannelOwner<BrowserContextChannel, BrowserC
async close(): Promise<void> {
await this._channel.close();
}

async newCDPSession(page: Page): Promise<CDPSession> {
return CDPSession.from(await this._channel.crNewCDPSession({ page: page._channel }));
}

backgroundPages(): Page[] {
return [...this._crBackgroundPages];
}

serviceWorkers(): Worker[] {
return [...this._crServiceWorkers];
}
}
9 changes: 7 additions & 2 deletions src/rpc/client/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import { ConnectionScope } from './connection';
import { ChannelOwner } from './channelOwner';
import { Func1, JSHandle, parseResult, serializeArgument, SmartHandle } from './jsHandle';
import { Page } from './page';
import { BrowserContext } from './browserContext';

export class Worker extends ChannelOwner<WorkerChannel, WorkerInitializer> {
_page: Page | undefined;
_page: Page | undefined; // Set for web workers.
_context: BrowserContext | undefined; // Set for service workers.

static from(worker: WorkerChannel): Worker {
return (worker as any)._object;
Expand All @@ -32,7 +34,10 @@ export class Worker extends ChannelOwner<WorkerChannel, WorkerInitializer> {
constructor(scope: ConnectionScope, guid: string, initializer: WorkerInitializer) {
super(scope, guid, initializer);
this._channel.on('close', () => {
this._page!._workers.delete(this);
if (this._page)
this._page._workers.delete(this);
if (this._context)
this._context._crServiceWorkers.delete(this);
this.emit(Events.Worker.Close, this);
});
}
Expand Down
25 changes: 22 additions & 3 deletions src/rpc/server/browserContextDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,31 @@ import * as types from '../../types';
import { BrowserContextBase, BrowserContext } from '../../browserContext';
import { Events } from '../../events';
import { Dispatcher, DispatcherScope, lookupNullableDispatcher, lookupDispatcher } from './dispatcher';
import { PageDispatcher, BindingCallDispatcher } from './pageDispatcher';
import { PageChannel, BrowserContextChannel, BrowserContextInitializer } from '../channels';
import { PageDispatcher, BindingCallDispatcher, WorkerDispatcher } from './pageDispatcher';
import { PageChannel, BrowserContextChannel, BrowserContextInitializer, CDPSessionChannel } from '../channels';
import { RouteDispatcher, RequestDispatcher } from './networkDispatchers';
import { Page } from '../../page';
import { CRBrowserContext } from '../../chromium/crBrowser';
import { CDPSessionDispatcher } from './cdpSessionDispatcher';
import { Events as ChromiumEvents } from '../../chromium/events';

export class BrowserContextDispatcher extends Dispatcher<BrowserContext, BrowserContextInitializer> implements BrowserContextChannel {
private _context: BrowserContextBase;

constructor(scope: DispatcherScope, context: BrowserContextBase) {
let crBackgroundPages: PageDispatcher[] = [];
let crServiceWorkers: WorkerDispatcher[] = [];
if (context._browserBase._options.name === 'chromium') {
crBackgroundPages = (context as CRBrowserContext).backgroundPages().map(p => new PageDispatcher(scope, p));
context.on(ChromiumEvents.CRBrowserContext.BackgroundPage, page => this._dispatchEvent('crBackgroundPage', new PageDispatcher(this._scope, page)));
crServiceWorkers = (context as CRBrowserContext).serviceWorkers().map(w => new WorkerDispatcher(scope, w));
context.on(ChromiumEvents.CRBrowserContext.ServiceWorker, serviceWorker => this._dispatchEvent('crServiceWorker', new WorkerDispatcher(this._scope, serviceWorker)));
}

super(scope, context, 'context', {
pages: context.pages().map(p => new PageDispatcher(scope, p))
pages: context.pages().map(p => new PageDispatcher(scope, p)),
crBackgroundPages,
crServiceWorkers,
}, true);
this._context = context;
context.on(Events.BrowserContext.Page, page => this._dispatchEvent('page', new PageDispatcher(this._scope, page)));
Expand Down Expand Up @@ -118,4 +132,9 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, Browser
async close(): Promise<void> {
await this._context.close();
}

async crNewCDPSession(params: { page: PageDispatcher }): Promise<CDPSessionChannel> {
const crBrowserContext = this._object as CRBrowserContext;
return new CDPSessionDispatcher(this._scope, await crBrowserContext.newCDPSession(params.page._object));
}
}
3 changes: 1 addition & 2 deletions src/rpc/server/browserDispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ export class BrowserDispatcher extends Dispatcher<Browser, BrowserInitializer> i
await this._object.close();
}

// Chromium-specific.
async newBrowserCDPSession(): Promise<CDPSessionChannel> {
async crNewBrowserCDPSession(): Promise<CDPSessionChannel> {
const crBrowser = this._object as CRBrowser;
return new CDPSessionDispatcher(this._scope, await crBrowser.newBrowserCDPSession());
}
Expand Down
3 changes: 2 additions & 1 deletion src/server/browserType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export abstract class BrowserTypeBase implements BrowserType {
if ((options as any).__testHookBeforeCreateBrowser)
await (options as any).__testHookBeforeCreateBrowser();
const browserOptions: BrowserOptions = {
name: this._name,
slowMo: options.slowMo,
persistent,
headful: !options.headless,
Expand Down Expand Up @@ -142,7 +143,7 @@ export abstract class BrowserTypeBase implements BrowserType {
progress.cleanupWhenAborted(() => transport.closeAndWait());
if ((options as any).__testHookBeforeCreateBrowser)
await (options as any).__testHookBeforeCreateBrowser();
const browser = await this._connectToTransport(transport, { slowMo: options.slowMo, loggers });
const browser = await this._connectToTransport(transport, { name: this._name, slowMo: options.slowMo, loggers });
return browser;
}, loggers.browser, TimeoutSettings.timeout(options), 'browserType.connect');
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ export class Electron {
const chromeMatch = await waitForLine(progress, launchedProcess, launchedProcess.stderr, /^DevTools listening on (ws:\/\/.*)$/);
const chromeTransport = await WebSocketTransport.connect(progress, chromeMatch[1]);
const browserServer = new BrowserServer(launchedProcess, gracefullyClose, kill);
const browser = await CRBrowser.connect(chromeTransport, { headful: true, loggers, persistent: { viewport: null }, ownedServer: browserServer });
const browser = await CRBrowser.connect(chromeTransport, { name: 'electron', headful: true, loggers, persistent: { viewport: null }, ownedServer: browserServer });
app = new ElectronApplication(loggers, browser, nodeConnection);
await app._init();
return app;
Expand Down
15 changes: 14 additions & 1 deletion test/chromium/chromium.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('../utils').testOptions(browserType);

describe.skip(CHANNEL)('ChromiumBrowserContext', function() {
describe('ChromiumBrowserContext', function() {
it('should create a worker from a service worker', async({browser, page, server, context}) => {
const [worker] = await Promise.all([
context.waitForEvent('serviceworker'),
Expand Down Expand Up @@ -50,6 +50,19 @@ describe.skip(CHANNEL)('ChromiumBrowserContext', function() {
});
expect(serviceWorkerCreated).not.toBeTruthy();
});
it('should close service worker together with the context', async({browser, server}) => {
const context = await browser.newContext();
const page = await context.newPage();
const [worker] = await Promise.all([
context.waitForEvent('serviceworker'),
page.goto(server.PREFIX + '/serviceworkers/empty/sw.html')
]);
const messages = [];
context.on('close', () => messages.push('context'));
worker.on('close', () => messages.push('worker'));
await context.close();
expect(messages.join('|')).toBe('worker|context');
});
});

describe('Chromium-Specific Page Tests', function() {
Expand Down
6 changes: 3 additions & 3 deletions test/chromium/launcher.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const utils = require('../utils');
const {makeUserDataDir, removeUserDataDir} = utils;
const {FFOX, CHROMIUM, WEBKIT, WIN, CHANNEL} = utils.testOptions(browserType);

describe.skip(CHANNEL)('launcher', function() {
describe('launcher', function() {
it('should throw with remote-debugging-pipe argument', async({browserType, defaultBrowserOptions}) => {
const options = Object.assign({}, defaultBrowserOptions);
options.args = ['--remote-debugging-pipe'].concat(options.args || []);
Expand Down Expand Up @@ -49,7 +49,7 @@ describe.skip(CHANNEL)('launcher', function() {
});
});

describe.skip(CHANNEL)('extensions', () => {
describe('extensions', () => {
it('should return background pages', async({browserType, defaultBrowserOptions}) => {
const userDataDir = await makeUserDataDir();
const extensionPath = path.join(__dirname, '..', 'assets', 'simple-extension');
Expand All @@ -73,7 +73,7 @@ describe.skip(CHANNEL)('extensions', () => {
});
});

describe.skip(CHANNEL)('BrowserContext', function() {
describe('BrowserContext', function() {
it('should not create pages automatically', async ({browserType, defaultBrowserOptions}) => {
const browser = await browserType.launch(defaultBrowserOptions);
const browserSession = await browser.newBrowserCDPSession();
Expand Down
2 changes: 1 addition & 1 deletion test/chromium/oopif.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('../utils').testOptions(browserType);

describe.skip(CHANNEL)('OOPIF', function() {
describe('OOPIF', function() {
beforeAll(async function(state) {
state.browser = await state.browserType.launch(Object.assign({}, state.defaultBrowserOptions, {
args: (state.defaultBrowserOptions.args || []).concat(['--site-per-process']),
Expand Down
6 changes: 3 additions & 3 deletions test/chromium/session.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

const {FFOX, CHROMIUM, WEBKIT, CHANNEL} = require('../utils').testOptions(browserType);

describe.skip(CHANNEL)('ChromiumBrowserContext.createSession', function() {
describe('ChromiumBrowserContext.createSession', function() {
it('should work', async function({page, browser, server}) {
const client = await page.context().newCDPSession(page);

Expand All @@ -35,7 +35,7 @@ describe.skip(CHANNEL)('ChromiumBrowserContext.createSession', function() {
await page.goto(server.EMPTY_PAGE);
expect(events.length).toBe(1);
});
it('should enable and disable domains independently', async function({page, browser, server}) {
it.skip(CHANNEL)('should enable and disable domains independently', async function({page, browser, server}) {
const client = await page.context().newCDPSession(page);
await client.send('Runtime.enable');
await client.send('Debugger.enable');
Expand Down Expand Up @@ -64,7 +64,7 @@ describe.skip(CHANNEL)('ChromiumBrowserContext.createSession', function() {
} catch (e) {
error = e;
}
expect(error.message).toContain('Session closed.');
expect(error.message).toContain(CHANNEL ? 'Target browser or context has been closed' : 'Session closed.');
});
it('should throw nice errors', async function({page, browser}) {
const client = await page.context().newCDPSession(page);
Expand Down

0 comments on commit 8fe29fe

Please sign in to comment.