chore(targets): create page targets only when attached to them (#1278)

Author: yury-s

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,27 +133,42 @@ 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.
       debugError(e);
     }
   }
 
+  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);
   }
 

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,38 +48,32 @@ export class CRTarget {
     browser: CRBrowser,
     targetInfo: Protocol.Target.TargetInfo,
     browserContext: CRBrowserContext,
+    session: CRSession | null,
     sessionFactory: () => Promise<CRSession>) {
     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<Page | Error>(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() {
     if (this._crPage)
       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<Page | Error> {
-    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<Worker | null> {