Skip to content

Commit c359116

Browse files
yury-syury-s
authored
fix: create _defaultContext only in persistent mode (#1854)
1 parent 022bc67 commit c359116

File tree

6 files changed

+42
-29
lines changed

6 files changed

+42
-29
lines changed

‎src/chromium/crBrowser.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export class CRBrowser extends BrowserBase {
3434
readonly _connection: CRConnection;
3535
_session: CRSession;
3636
private _clientRootSessionPromise: Promise<CRSession> | null = null;
37-
readonly _defaultContext: CRBrowserContext;
37+
readonly _defaultContext: CRBrowserContext | null = null;
3838
readonly _contexts = new Map<string, CRBrowserContext>();
3939
_crPages = new Map<string, CRPage>();
4040
_backgroundPages = new Map<string, CRPage>();
@@ -48,7 +48,7 @@ export class CRBrowser extends BrowserBase {
4848

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

86-
constructor(connection: CRConnection) {
86+
constructor(connection: CRConnection, isPersistent: boolean) {
8787
super();
8888
this._connection = connection;
8989
this._session = this._connection.rootSession;
9090

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

115116
_onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) {
117+
if (targetInfo.type === 'browser')
118+
return;
116119
const session = this._connection.session(sessionId)!;
117-
const context = (targetInfo.browserContextId && this._contexts.has(targetInfo.browserContextId)) ?
118-
this._contexts.get(targetInfo.browserContextId)! : this._defaultContext;
120+
assert(targetInfo.browserContextId, 'targetInfo: ' + JSON.stringify(targetInfo, null, 2));
121+
let context = this._contexts.get(targetInfo.browserContextId) || null;
122+
if (!context) {
123+
// TODO: auto attach only to pages from our contexts.
124+
// assert(this._defaultContext);
125+
context = this._defaultContext;
126+
}
127+
128+
if (targetInfo.type === 'other' || !context) {
129+
if (waitingForDebugger) {
130+
// Ideally, detaching should resume any target, but there is a bug in the backend.
131+
session.send('Runtime.runIfWaitingForDebugger').catch(debugError).then(() => {
132+
this._session.send('Target.detachFromTarget', { sessionId }).catch(debugError);
133+
});
134+
}
135+
return;
136+
}
119137

120138
assert(!this._crPages.has(targetInfo.targetId), 'Duplicate target ' + targetInfo.targetId);
121139
assert(!this._backgroundPages.has(targetInfo.targetId), 'Duplicate target ' + targetInfo.targetId);
@@ -125,7 +143,7 @@ export class CRBrowser extends BrowserBase {
125143
const backgroundPage = new CRPage(session, targetInfo.targetId, context, null);
126144
this._backgroundPages.set(targetInfo.targetId, backgroundPage);
127145
backgroundPage.pageOrError().then(() => {
128-
context.emit(Events.CRBrowserContext.BackgroundPage, backgroundPage._page);
146+
context!.emit(Events.CRBrowserContext.BackgroundPage, backgroundPage._page);
129147
});
130148
return;
131149
}
@@ -140,7 +158,7 @@ export class CRBrowser extends BrowserBase {
140158
}
141159
crPage.pageOrError().then(() => {
142160
this._firstPageCallback();
143-
context.emit(CommonEvents.BrowserContext.Page, crPage._page);
161+
context!.emit(CommonEvents.BrowserContext.Page, crPage._page);
144162
if (opener) {
145163
opener.pageOrError().then(openerPage => {
146164
if (openerPage instanceof Page && !openerPage.isClosed())
@@ -158,13 +176,7 @@ export class CRBrowser extends BrowserBase {
158176
return;
159177
}
160178

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

170182
_onDetachedFromTarget(payload: Protocol.Target.detachFromTargetParameters) {

‎src/firefox/ffBrowser.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,26 @@ import { Protocol } from './protocol';
3131
export class FFBrowser extends BrowserBase {
3232
_connection: FFConnection;
3333
readonly _ffPages: Map<string, FFPage>;
34-
readonly _defaultContext: FFBrowserContext;
34+
readonly _defaultContext: FFBrowserContext | null = null;
3535
readonly _contexts: Map<string, FFBrowserContext>;
3636
private _eventListeners: RegisteredListener[];
3737
readonly _firstPagePromise: Promise<void>;
3838
private _firstPageCallback = () => {};
3939

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

47-
constructor(connection: FFConnection) {
47+
constructor(connection: FFConnection, isPersistent: boolean) {
4848
super();
4949
this._connection = connection;
5050
this._ffPages = new Map();
5151

52-
this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({}));
52+
if (isPersistent)
53+
this._defaultContext = new FFBrowserContext(this, null, validateBrowserContextOptions({}));
5354
this._contexts = new Map();
5455
this._connection.on(ConnectionEvents.Disconnected, () => {
5556
for (const context of this._contexts.values())
@@ -124,6 +125,7 @@ export class FFBrowser extends BrowserBase {
124125
const {targetId, browserContextId, openerId, type} = payload.targetInfo;
125126
assert(type === 'page');
126127
const context = browserContextId ? this._contexts.get(browserContextId)! : this._defaultContext;
128+
assert(context, `Unknown context id:${browserContextId}, _defaultContext: ${this._defaultContext}`);
127129
const session = this._connection.createSession(payload.sessionId, type);
128130
const opener = openerId ? this._ffPages.get(openerId)! : null;
129131
const ffPage = new FFPage(session, context, opener);

‎src/server/chromium.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export class Chromium implements BrowserType<CRBrowser> {
6767
const browser = await CRBrowser.connect(transport!, true, slowMo);
6868
browser._ownedServer = browserServer;
6969
await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout);
70-
return browser._defaultContext;
70+
return browser._defaultContext!;
7171
}
7272

7373
private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string }> {

‎src/server/firefox.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export class Firefox implements BrowserType<FFBrowser> {
7474
browser._ownedServer = browserServer;
7575
browser._downloadsPath = downloadsPath;
7676
await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout);
77-
const browserContext = browser._defaultContext;
77+
const browserContext = browser._defaultContext!;
7878
return browserContext;
7979
}
8080

‎src/server/webkit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export class WebKit implements BrowserType<WKBrowser> {
6767
const browser = await WKBrowser.connect(transport!, slowMo, true);
6868
browser._ownedServer = browserServer;
6969
await helper.waitWithTimeout(browser._waitForFirstPageTarget(), 'first page', timeout);
70-
return browser._defaultContext;
70+
return browser._defaultContext!;
7171
}
7272

7373
private async _launchServer(options: LaunchServerOptions, launchType: LaunchType, userDataDir?: string): Promise<{ browserServer: BrowserServer, transport?: ConnectionTransport, downloadsPath: string }> {

‎src/webkit/wkBrowser.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ const DEFAULT_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_2) Appl
3131

3232
export class WKBrowser extends BrowserBase {
3333
private readonly _connection: WKConnection;
34-
private readonly _attachToDefaultContext: boolean;
3534
readonly _browserSession: WKSession;
36-
readonly _defaultContext: WKBrowserContext;
35+
readonly _defaultContext: WKBrowserContext | null = null;
3736
readonly _contexts = new Map<string, WKBrowserContext>();
3837
readonly _wkPages = new Map<string, WKPage>();
3938
private readonly _eventListeners: RegisteredListener[];
@@ -49,10 +48,10 @@ export class WKBrowser extends BrowserBase {
4948
constructor(transport: ConnectionTransport, attachToDefaultContext: boolean) {
5049
super();
5150
this._connection = new WKConnection(transport, this._onDisconnect.bind(this));
52-
this._attachToDefaultContext = attachToDefaultContext;
5351
this._browserSession = this._connection.browserSession;
5452

55-
this._defaultContext = new WKBrowserContext(this, undefined, validateBrowserContextOptions({}));
53+
if (attachToDefaultContext)
54+
this._defaultContext = new WKBrowserContext(this, undefined, validateBrowserContextOptions({}));
5655

5756
this._eventListeners = [
5857
helper.addEventListener(this._browserSession, 'Playwright.pageProxyCreated', this._onPageProxyCreated.bind(this)),
@@ -117,10 +116,10 @@ export class WKBrowser extends BrowserBase {
117116
// lifecycle events.
118117
context = this._contexts.get(pageProxyInfo.browserContextId) || null;
119118
}
120-
if (!context && !this._attachToDefaultContext)
121-
return;
122119
if (!context)
123-
context = this._defaultContext;
120+
context = this._defaultContext;
121+
if (!context)
122+
return;
124123
const pageProxySession = new WKSession(this._connection, pageProxyId, `The page has been closed.`, (message: any) => {
125124
this._connection.rawSend({ ...message, pageProxyId });
126125
});

0 commit comments

Comments
 (0)