fix: custom sessions should not emit targetcreated events (#8788)

Closes #8787

src/common/Browser.ts

@@ -492,8 +492,11 @@ export class Browser extends EventEmitter {
       session,
       context,
       this.#targetManager,
-      () => {
+      (isAutoAttachEmulated: boolean) => {
-        return this.#connection.createSession(targetInfo);
+        return this.#connection._createSession(
+          targetInfo,
+          isAutoAttachEmulated
+        );
       },
       this.#ignoreHTTPSErrors,
       this.#defaultViewport ?? null,

src/common/ChromeTargetManager.ts

@@ -236,7 +236,7 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
       // Special case (https://crbug.com/1338156): currently, shared_workers
       // don't get auto-attached. This should be removed once the auto-attach
       // works.
-      await this.#connection.createSession(event.targetInfo);
+      await this.#connection._createSession(event.targetInfo, true);
     }
   };
 
@@ -300,6 +300,10 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
         .catch(debugError);
     };
 
+    if (!this.#connection.isAutoAttached(targetInfo.targetId)) {
+      return;
+    }
+
     // Special case for service workers: being attached to service workers will
     // prevent them from ever being destroyed. Therefore, we silently detach
     // from service workers unless the connection was manually created via
@@ -364,7 +368,9 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
     }
 
     this.#targetsIdsForInit.delete(target._targetId);
+    if (!existingTarget) {
       this.emit(TargetManagerEmittedEvents.TargetAvailable, target);
+    }
     this.#finishInitializationIfReady();
 
     // TODO: the browser might be shutting down here. What do we do with the

src/common/Connection.ts

@@ -228,6 +228,28 @@ export class Connection extends EventEmitter {
     return !this.#manuallyAttached.has(targetId);
   }
 
+  /**
+   * @internal
+   */
+  async _createSession(
+    targetInfo: Protocol.Target.TargetInfo,
+    isAutoAttachEmulated = true
+  ): Promise<CDPSession> {
+    if (!isAutoAttachEmulated) {
+      this.#manuallyAttached.add(targetInfo.targetId);
+    }
+    const {sessionId} = await this.send('Target.attachToTarget', {
+      targetId: targetInfo.targetId,
+      flatten: true,
+    });
+    this.#manuallyAttached.delete(targetInfo.targetId);
+    const session = this.#sessions.get(sessionId);
+    if (!session) {
+      throw new Error('CDPSession creation failed.');
+    }
+    return session;
+  }
+
   /**
    * @param targetInfo - The target info
    * @returns The CDP session that is created
@@ -235,16 +257,7 @@ export class Connection extends EventEmitter {
   async createSession(
     targetInfo: Protocol.Target.TargetInfo
   ): Promise<CDPSession> {
-    this.#manuallyAttached.add(targetInfo.targetId);
+    return await this._createSession(targetInfo, false);
-    const {sessionId} = await this.send('Target.attachToTarget', {
-      targetId: targetInfo.targetId,
-      flatten: true,
-    });
-    const session = this.#sessions.get(sessionId);
-    if (!session) {
-      throw new Error('CDPSession creation failed.');
-    }
-    return session;
   }
 }
 

src/common/Target.ts

@@ -30,7 +30,7 @@ export class Target {
   #browserContext: BrowserContext;
   #session?: CDPSession;
   #targetInfo: Protocol.Target.TargetInfo;
-  #sessionFactory: () => Promise<CDPSession>;
+  #sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>;
   #ignoreHTTPSErrors: boolean;
   #defaultViewport?: Viewport;
   #pagePromise?: Promise<Page>;
@@ -76,7 +76,7 @@ export class Target {
     session: CDPSession | undefined,
     browserContext: BrowserContext,
     targetManager: TargetManager,
-    sessionFactory: () => Promise<CDPSession>,
+    sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>,
     ignoreHTTPSErrors: boolean,
     defaultViewport: Viewport | null,
     screenshotTaskQueue: TaskQueue,
@@ -132,7 +132,7 @@ export class Target {
    * Creates a Chrome Devtools Protocol session attached to the target.
    */
   createCDPSession(): Promise<CDPSession> {
-    return this.#sessionFactory();
+    return this.#sessionFactory(false);
   }
 
   /**
@@ -155,7 +155,9 @@ export class Target {
   async page(): Promise<Page | null> {
     if (this._isPageTargetCallback(this.#targetInfo) && !this.#pagePromise) {
       this.#pagePromise = (
-        this.#session ? Promise.resolve(this.#session) : this.#sessionFactory()
+        this.#session
+          ? Promise.resolve(this.#session)
+          : this.#sessionFactory(true)
       ).then(client => {
         return Page._create(
           client,
@@ -182,7 +184,9 @@ export class Target {
     if (!this.#workerPromise) {
       // TODO(einbinder): Make workers send their console logs.
       this.#workerPromise = (
-        this.#session ? Promise.resolve(this.#session) : this.#sessionFactory()
+        this.#session
+          ? Promise.resolve(this.#session)
+          : this.#sessionFactory(false)
       ).then(client => {
         return new WebWorker(
           client,

test/src/CDPSession.spec.ts

@@ -42,6 +42,20 @@ describeChromeOnly('Target.createCDPSession', function () {
     });
     expect(foo).toBe('bar');
   });
+
+  it('should not report created targets for custom CDP sessions', async () => {
+    const {browser} = getTestState();
+    let called = 0;
+    browser.browserContexts()[0]!.on('targetcreated', async target => {
+      called++;
+      if (called > 1) {
+        throw new Error('Too many targets created');
+      }
+      await target.createCDPSession();
+    });
+    await browser.newPage();
+  });
+
   it('should send events', async () => {
     const {page, server} = getTestState();