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.

Sign up for GitHub

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

Jump to bottom

fix: create _defaultContext only in persistent mode #1854

Merged
merged 1 commit into from
Apr 19, 2020
Merged

fix: create _defaultContext only in persistent mode #1854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 27 additions & 15 deletions src/chromium/crBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class CRBrowser extends BrowserBase {
readonly _connection: CRConnection;
_session: CRSession;
private _clientRootSessionPromise: Promise<CRSession> | null = null;
readonly _defaultContext: CRBrowserContext;
readonly _defaultContext: CRBrowserContext | null = null;
readonly _contexts = new Map<string, CRBrowserContext>();
_crPages = new Map<string, CRPage>();
_backgroundPages = new Map<string, CRPage>();
Expand All @@ -48,7 +48,7 @@ export class CRBrowser extends BrowserBase {

static async connect(transport: ConnectionTransport, isPersistent: boolean, slowMo?: number): Promise<CRBrowser> {
const connection = new CRConnection(SlowMoTransport.wrap(transport, slowMo));
const browser = new CRBrowser(connection);
const browser = new CRBrowser(connection, isPersistent);
const session = connection.rootSession;
if (!isPersistent) {
await session.send('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: true, flatten: true });
Expand Down Expand Up @@ -83,12 +83,13 @@ export class CRBrowser extends BrowserBase {
return browser;
}

constructor(connection: CRConnection) {
constructor(connection: CRConnection, isPersistent: boolean) {
super();
this._connection = connection;
this._session = this._connection.rootSession;

this._defaultContext = new CRBrowserContext(this, null, validateBrowserContextOptions({}));
if (isPersistent)
this._defaultContext = new CRBrowserContext(this, null, validateBrowserContextOptions({}));
this._connection.on(ConnectionEvents.Disconnected, () => {
for (const context of this._contexts.values())
context._browserClosed();
Expand All @@ -113,9 +114,26 @@ export class CRBrowser extends BrowserBase {
}

_onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
if (targetInfo.type === 'browser')
return;
const session = this._connection.session(sessionId)!;
const context = (targetInfo.browserContextId && this._contexts.has(targetInfo.browserContextId)) ?
this._contexts.get(targetInfo.browserContextId)! : this._defaultContext;
assert(targetInfo.browserContextId, 'targetInfo: ' + JSON.stringify(targetInfo, null, 2));
let context = this._contexts.get(targetInfo.browserContextId) || null;
if (!context) {
// TODO: auto attach only to pages from our contexts.
// assert(this._defaultContext);
context = this._defaultContext;
}

if (targetInfo.type === 'other' || !context) {
if (waitingForDebugger) {
// Ideally, detaching should resume any target, but there is a bug in the backend.
session.send('Runtime.runIfWaitingForDebugger').catch(debugError).then(() => {
this._session.send('Target.detachFromTarget', { sessionId }).catch(debugError);
});
}
return;
}

assert(!this._crPages.has(targetInfo.targetId), 'Duplicate target ' + targetInfo.targetId);
assert(!this._backgroundPages.has(targetInfo.targetId), 'Duplicate target ' + targetInfo.targetId);
Expand All @@ -125,7 +143,7 @@ export class CRBrowser extends BrowserBase {
const backgroundPage = new CRPage(session, targetInfo.targetId, context, null);
this._backgroundPages.set(targetInfo.targetId, backgroundPage);
backgroundPage.pageOrError().then(() => {
context.emit(Events.CRBrowserContext.BackgroundPage, backgroundPage._page);
context!.emit(Events.CRBrowserContext.BackgroundPage, backgroundPage._page);
});
return;
}
Expand All @@ -140,7 +158,7 @@ export class CRBrowser extends BrowserBase {
}
crPage.pageOrError().then(() => {
this._firstPageCallback();
context.emit(CommonEvents.BrowserContext.Page, crPage._page);
context!.emit(CommonEvents.BrowserContext.Page, crPage._page);
if (opener) {
opener.pageOrError().then(openerPage => {
if (openerPage instanceof Page && !openerPage.isClosed())
Expand All @@ -158,13 +176,7 @@ export class CRBrowser extends BrowserBase {
return;
}

assert(targetInfo.type === 'browser' || targetInfo.type === 'other');
if (waitingForDebugger) {
// Ideally, detaching should resume any target, but there is a bug in the backend.
session.send('Runtime.runIfWaitingForDebugger').catch(debugError).then(() => {
this._session.send('Target.detachFromTarget', { sessionId }).catch(debugError);
});
}
assert(false, 'Unknown target type: ' + targetInfo.type);
}

_onDetachedFromTarget(payload: Protocol.Target.detachFromTargetParameters) {
Expand Down
10 changes: 6 additions & 4 deletions src/firefox/ffBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,26 @@ import { Protocol } from './protocol';
export class FFBrowser extends BrowserBase {
_connection: FFConnection;
readonly _ffPages: Map<string, FFPage>;
readonly _defaultContext: FFBrowserContext;
readonly _defaultContext: FFBrowserContext | null = null;
readonly _contexts: Map<string, FFBrowserContext>;
private _eventListeners: RegisteredListener[];
readonly _firstPagePromise: Promise<void>;
private _firstPageCallback = () => {};

static async connect(transport: ConnectionTransport, attachToDefaultContext: boolean, slowMo?: number): Promise<FFBrowser> {
const connection = new FFConnection(SlowMoTransport.wrap(transport, slowMo));
const browser = new FFBrowser(connection);
const browser = new FFBrowser(connection, attachToDefaultContext);
await connection.send('Browser.enable', { attachToDefaultContext });
return browser;
}

constructor(connection: FFConnection) {
constructor(connection: FFConnection, isPersistent: boolean) {
super();
this._connection = connection;
this._ffPages = new Map();

this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({}));
if (isPersistent)
this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({}));
this._contexts = new Map();
this._connection.on(ConnectionEvents.Disconnected, () => {
for (const context of this._contexts.values())
Expand Down Expand Up @@ -124,6 +125,7 @@ export class FFBrowser extends BrowserBase {
const {targetId, browserContextId, openerId, type} = payload.targetInfo;
assert(type === 'page');
const context = browserContextId ? this._contexts.get(browserContextId)! : this._defaultContext;
assert(context, `Unknown context id:${browserContextId}, _defaultContext: ${this._defaultContext}`);
const session = this._connection.createSession(payload.sessionId, type);
const opener = openerId ? this._ffPages.get(openerId)! : null;
const ffPage = new FFPage(session, context, opener);
Expand Down
2 changes: 1 addition & 1 deletion src/server/chromium.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class Chromium implements BrowserType<CRBrowser> {
const browser = await CRBrowser.connect(transport!, true, slowMo);
browser._ownedServer = browserServer;
await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout);
return browser._defaultContext;
return browser._defaultContext!;
}

private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string }> {
Expand Down
2 changes: 1 addition & 1 deletion src/server/firefox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class Firefox implements BrowserType<FFBrowser> {
browser._ownedServer = browserServer;
browser._downloadsPath = downloadsPath;
await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout);
const browserContext = browser._defaultContext;
const browserContext = browser._defaultContext!;
return browserContext;
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/webkit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class WebKit implements BrowserType<WKBrowser> {
const browser = await WKBrowser.connect(transport!, slowMo, true);
browser._ownedServer = browserServer;
await helper.waitWithTimeout(browser._waitForFirstPageTarget(), 'first page', timeout);
return browser._defaultContext;
return browser._defaultContext!;
}

private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string }> {
Expand Down
13 changes: 6 additions & 7 deletions src/webkit/wkBrowser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ const DEFAULT_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_2) Appl

export class WKBrowser extends BrowserBase {
private readonly _connection: WKConnection;
private readonly _attachToDefaultContext: boolean;
readonly _browserSession: WKSession;
readonly _defaultContext: WKBrowserContext;
readonly _defaultContext: WKBrowserContext | null = null;
readonly _contexts = new Map<string, WKBrowserContext>();
readonly _wkPages = new Map<string, WKPage>();
private readonly _eventListeners: RegisteredListener[];
Expand All @@ -49,10 +48,10 @@ export class WKBrowser extends BrowserBase {
constructor(transport: ConnectionTransport, attachToDefaultContext: boolean) {
super();
this._connection = new WKConnection(transport, this._onDisconnect.bind(this));
this._attachToDefaultContext = attachToDefaultContext;
this._browserSession = this._connection.browserSession;

this._defaultContext = new WKBrowserContext(this, undefined, validateBrowserContextOptions({}));
if (attachToDefaultContext)
this._defaultContext = new WKBrowserContext(this, undefined, validateBrowserContextOptions({}));

this._eventListeners = [
helper.addEventListener(this._browserSession, 'Playwright.pageProxyCreated', this._onPageProxyCreated.bind(this)),
Expand Down Expand Up @@ -117,10 +116,10 @@ export class WKBrowser extends BrowserBase {
// lifecycle events.
context = this._contexts.get(pageProxyInfo.browserContextId) || null;
}
if (!context && !this._attachToDefaultContext)
return;
if (!context)
context = this._defaultContext;
context = this._defaultContext;
if (!context)
return;
const pageProxySession = new WKSession(this._connection, pageProxyId, `The page has been closed.`, (message: any) => {
this._connection.rawSend({ ...message, pageProxyId });
});
Expand Down