From c359116a3b438cc74f112348c52f21103c7488f4 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Sat, 18 Apr 2020 19:58:11 -0700 Subject: [PATCH] fix: create _defaultContext only in persistent mode (#1854) --- src/chromium/crBrowser.ts | 42 +++++++++++++++++++++++++-------------- src/firefox/ffBrowser.ts | 10 ++++++---- src/server/chromium.ts | 2 +- src/server/firefox.ts | 2 +- src/server/webkit.ts | 2 +- src/webkit/wkBrowser.ts | 13 ++++++------ 6 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index 6ec8218b11eb2..a72950074739d 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -34,7 +34,7 @@ export class CRBrowser extends BrowserBase { readonly _connection: CRConnection; _session: CRSession; private _clientRootSessionPromise: Promise | null = null; - readonly _defaultContext: CRBrowserContext; + readonly _defaultContext: CRBrowserContext | null = null; readonly _contexts = new Map(); _crPages = new Map(); _backgroundPages = new Map(); @@ -48,7 +48,7 @@ export class CRBrowser extends BrowserBase { static async connect(transport: ConnectionTransport, isPersistent: boolean, slowMo?: number): Promise { 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 }); @@ -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(); @@ -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); @@ -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; } @@ -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()) @@ -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) { diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index a0e3732e1c877..4295a3ea5506e 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -31,7 +31,7 @@ import { Protocol } from './protocol'; export class FFBrowser extends BrowserBase { _connection: FFConnection; readonly _ffPages: Map; - readonly _defaultContext: FFBrowserContext; + readonly _defaultContext: FFBrowserContext | null = null; readonly _contexts: Map; private _eventListeners: RegisteredListener[]; readonly _firstPagePromise: Promise; @@ -39,17 +39,18 @@ export class FFBrowser extends BrowserBase { static async connect(transport: ConnectionTransport, attachToDefaultContext: boolean, slowMo?: number): Promise { 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()) @@ -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); diff --git a/src/server/chromium.ts b/src/server/chromium.ts index eefd77b8015b0..df92fdb30af01 100644 --- a/src/server/chromium.ts +++ b/src/server/chromium.ts @@ -67,7 +67,7 @@ export class Chromium implements BrowserType { 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 }> { diff --git a/src/server/firefox.ts b/src/server/firefox.ts index 4ca982def1fee..9860b7690d2f6 100644 --- a/src/server/firefox.ts +++ b/src/server/firefox.ts @@ -74,7 +74,7 @@ export class Firefox implements BrowserType { browser._ownedServer = browserServer; browser._downloadsPath = downloadsPath; await helper.waitWithTimeout(browser._firstPagePromise, 'first page', timeout); - const browserContext = browser._defaultContext; + const browserContext = browser._defaultContext!; return browserContext; } diff --git a/src/server/webkit.ts b/src/server/webkit.ts index bc3a15823539d..128569daba2e4 100644 --- a/src/server/webkit.ts +++ b/src/server/webkit.ts @@ -67,7 +67,7 @@ export class WebKit implements BrowserType { 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 }> { diff --git a/src/webkit/wkBrowser.ts b/src/webkit/wkBrowser.ts index 449e2613185f4..468183b3d1e3d 100644 --- a/src/webkit/wkBrowser.ts +++ b/src/webkit/wkBrowser.ts @@ -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(); readonly _wkPages = new Map(); private readonly _eventListeners: RegisteredListener[]; @@ -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)), @@ -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 }); });