From 0fbc7af26d7d7906a6c9f585b69afc565f2dd0d6 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 9 Mar 2020 15:53:45 -0700 Subject: [PATCH] chore(targets): create page targets only when attached to them (#1278) --- src/chromium/crBrowser.ts | 53 ++++++++++++++++++++++++--------------- src/chromium/crTarget.ts | 32 ++++++++++------------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/chromium/crBrowser.ts b/src/chromium/crBrowser.ts index 790a91eae8..8619e5e952 100644 --- a/src/chromium/crBrowser.ts +++ b/src/chromium/crBrowser.ts @@ -102,21 +102,19 @@ export class CRBrowser extends platform.EventEmitter implements Browser { return createPageInNewContext(this, options); } - async _onAttachedToTarget(event: Protocol.Target.attachedToTargetPayload) { - if (!CRTarget.isPageType(event.targetInfo.type)) + async _onAttachedToTarget({targetInfo, sessionId, waitingForDebugger}: Protocol.Target.attachedToTargetPayload) { + const session = this._connection.session(sessionId)!; + if (!CRTarget.isPageType(targetInfo.type)) { + assert(targetInfo.type === 'service_worker' || 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); + }); + } return; - const target = this._targets.get(event.targetInfo.targetId); - const session = this._connection.session(event.sessionId)!; - await target!.initializePageSession(session).catch(debugError); - } - - async _targetCreated({targetInfo}: Protocol.Target.targetCreatedPayload) { - const {browserContextId} = targetInfo; - const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId)! : this._defaultContext; - const target = new CRTarget(this, targetInfo, context, () => this._connection.createSession(targetInfo)); - assert(!this._targets.has(targetInfo.targetId), 'Target should not exist before targetCreated'); - this._targets.set(targetInfo.targetId, target); - + } + const { context, target } = this._createTarget(targetInfo, session); try { switch (targetInfo.type) { case 'page': { @@ -135,11 +133,6 @@ export class CRBrowser extends platform.EventEmitter implements Browser { context.emit(Events.CRBrowserContext.BackgroundPage, event); break; } - case 'service_worker': { - const serviceWorker = await target.serviceWorker(); - context.emit(Events.CRBrowserContext.ServiceWorker, serviceWorker); - break; - } } } catch (e) { // Do not dispatch the event if initialization failed. @@ -147,15 +140,35 @@ export class CRBrowser extends platform.EventEmitter implements Browser { } } + async _targetCreated({targetInfo}: Protocol.Target.targetCreatedPayload) { + if (targetInfo.type !== 'service_worker') + return; + const { context, target } = this._createTarget(targetInfo, null); + const serviceWorker = await target.serviceWorker(); + context.emit(Events.CRBrowserContext.ServiceWorker, serviceWorker); + } + + private _createTarget(targetInfo: Protocol.Target.TargetInfo, session: CRSession | null) { + const {browserContextId} = targetInfo; + const context = (browserContextId && this._contexts.has(browserContextId)) ? this._contexts.get(browserContextId)! : this._defaultContext; + const target = new CRTarget(this, targetInfo, context, session, () => this._connection.createSession(targetInfo)); + assert(!this._targets.has(targetInfo.targetId), 'Target should not exist before targetCreated'); + this._targets.set(targetInfo.targetId, target); + return { context, target }; + } + async _targetDestroyed(event: { targetId: string; }) { const target = this._targets.get(event.targetId)!; + if (!target) + return; this._targets.delete(event.targetId); target._didClose(); } _targetInfoChanged(event: Protocol.Target.targetInfoChangedPayload) { const target = this._targets.get(event.targetInfo.targetId)!; - assert(target, 'target should exist before targetInfoChanged'); + if (!target) + return; target._targetInfoChanged(event.targetInfo); } diff --git a/src/chromium/crTarget.ts b/src/chromium/crTarget.ts index 0d4d901ba6..5dd964cacc 100644 --- a/src/chromium/crTarget.ts +++ b/src/chromium/crTarget.ts @@ -19,7 +19,7 @@ import { CRBrowser, CRBrowserContext } from './crBrowser'; import { CRSession, CRSessionEvents } from './crConnection'; import { Page, Worker } from '../page'; import { Protocol } from './protocol'; -import { debugError } from '../helper'; +import { debugError, assert } from '../helper'; import { CRPage } from './crPage'; import { CRExecutionContext } from './crExecutionContext'; @@ -48,14 +48,21 @@ export class CRTarget { browser: CRBrowser, targetInfo: Protocol.Target.TargetInfo, browserContext: CRBrowserContext, + session: CRSession | null, sessionFactory: () => Promise) { this._targetInfo = targetInfo; this._browser = browser; this._browserContext = browserContext; this._targetId = targetInfo.targetId; this.sessionFactory = sessionFactory; - if (CRTarget.isPageType(targetInfo.type)) - this._pagePromise = new Promise(f => this._pagePromiseCallback = f); + if (CRTarget.isPageType(targetInfo.type)) { + assert(session, 'Page target must be created with existing session'); + this._crPage = new CRPage(session, this._browser, this._browserContext); + const page = this._crPage.page(); + (page as any)[targetSymbol] = this; + session.once(CRSessionEvents.Disconnected, () => page._didDisconnect()); + this._pagePromise = this._crPage.initialize().then(() => page).catch(e => e); + } } _didClose() { @@ -63,23 +70,10 @@ export class CRTarget { this._crPage.didClose(); } - async initializePageSession(session: CRSession) { - this._crPage = new CRPage(session, this._browser, this._browserContext); - const page = this._crPage.page(); - (page as any)[targetSymbol] = this; - session.once(CRSessionEvents.Disconnected, () => page._didDisconnect()); - try { - await this._crPage.initialize(); - this._pagePromiseCallback!(page); - } catch (e) { - this._pagePromiseCallback!(e); - } - } - async pageOrError(): Promise { - if (this._targetInfo.type !== 'page' && this._targetInfo.type !== 'background_page') - throw new Error('Not a page.'); - return this._pagePromise!; + if (CRTarget.isPageType(this.type())) + return this._pagePromise!; + throw new Error('Not a page.'); } async serviceWorker(): Promise {