fix: resolve target manager init if no existing targets detected (#8748)

Closes #8747
puppeteer · Aug 6, 2022 · 8cb5043 · 8cb5043
1 parent 9588072
commit 8cb5043
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 13 deletions.
diff --git a/src/common/ChromeTargetManager.ts b/src/common/ChromeTargetManager.ts
@@ -103,30 +103,37 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
 
     // TODO: remove `as any` once the protocol definitions are updated with the
     // next Chromium roll.
-    this.#connection.send('Target.setDiscoverTargets', {
-      discover: true,
-      filter: [{type: 'tab', exclude: true}, {}],
-    } as any);
+    this.#connection
+      .send('Target.setDiscoverTargets', {
+        discover: true,
+        filter: [{type: 'tab', exclude: true}, {}],
+      } as any)
+      .then(this.#storeExistingTargetsForInit)
+      .catch(debugError);
   }
 
-  async initialize(): Promise<void> {
-    this.#targetsIdsForInit = new Set();
+  #storeExistingTargetsForInit = () => {
     for (const [
       targetId,
       targetInfo,
     ] of this.#discoveredTargetsByTargetId.entries()) {
       if (
-        !this.#targetFilterCallback ||
-        this.#targetFilterCallback(targetInfo)
+        (!this.#targetFilterCallback ||
+          this.#targetFilterCallback(targetInfo)) &&
+        targetInfo.type !== 'browser'
       ) {
         this.#targetsIdsForInit.add(targetId);
       }
     }
+  };
+
+  async initialize(): Promise<void> {
     await this.#connection.send('Target.setAutoAttach', {
       waitForDebuggerOnStart: true,
       flatten: true,
       autoAttach: true,
     });
+    this.#finishInitializationIfReady();
     await this.#initializePromise;
   }
 
@@ -358,9 +365,7 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
 
     this.#targetsIdsForInit.delete(target._targetId);
     this.emit(TargetManagerEmittedEvents.TargetAvailable, target);
-    if (this.#targetsIdsForInit.size === 0) {
-      this.#initializeCallback();
-    }
+    this.#finishInitializationIfReady();
 
     // TODO: the browser might be shutting down here. What do we do with the
     // error?
@@ -374,8 +379,8 @@ export class ChromeTargetManager extends EventEmitter implements TargetManager {
     ]).catch(debugError);
   };
 
-  #finishInitializationIfReady(targetId: string): void {
-    this.#targetsIdsForInit.delete(targetId);
+  #finishInitializationIfReady(targetId?: string): void {
+    targetId !== undefined && this.#targetsIdsForInit.delete(targetId);
     if (this.#targetsIdsForInit.size === 0) {
       this.#initializeCallback();
     }

diff --git a/test/src/launcher.spec.ts b/test/src/launcher.spec.ts
@@ -699,6 +699,24 @@ describe('Launcher specs', function () {
           remoteBrowser.close(),
         ]);
       });
+      it('should be able to connect to a browser with no page targets', async () => {
+        const {defaultBrowserOptions, puppeteer} = getTestState();
+
+        const originalBrowser = await puppeteer.launch(defaultBrowserOptions);
+        const pages = await originalBrowser.pages();
+        await Promise.all(
+          pages.map(page => {
+            return page.close();
+          })
+        );
+        const remoteBrowser = await puppeteer.connect({
+          browserWSEndpoint: originalBrowser.wsEndpoint(),
+        });
+        await Promise.all([
+          utils.waitEvent(originalBrowser, 'disconnected'),
+          remoteBrowser.close(),
+        ]);
+      });
       it('should support ignoreHTTPSErrors option', async () => {
         const {httpsServer, puppeteer, defaultBrowserOptions} = getTestState();