refactor: add browser target (#10642)

packages/puppeteer-core/src/common/bidi/Browser.ts

@@ -37,7 +37,12 @@ import {
   BrowsingContextEmittedEvents,
 } from './BrowsingContext.js';
 import {Connection} from './Connection.js';
-import {BiDiPageTarget, BiDiTarget} from './Target.js';
+import {
+  BiDiBrowserTarget,
+  BiDiBrowsingContextTarget,
+  BiDiPageTarget,
+  BiDiTarget,
+} from './Target.js';
 import {debugError} from './utils.js';
 
 /**
@@ -107,6 +112,7 @@ export class Browser extends BrowserBase {
   #defaultContext: BrowserContext;
   #targets = new Map<string, BiDiTarget>();
   #contexts: BrowserContext[] = [];
+  #browserTarget: BiDiBrowserTarget;
 
   #connectionEventHandlers = new Map<string, Handler<any>>([
     ['browsingContext.contextCreated', this.#onContextCreated.bind(this)],
@@ -137,6 +143,7 @@ export class Browser extends BrowserBase {
       defaultViewport: this.#defaultViewport,
       isDefault: true,
     });
+    this.#browserTarget = new BiDiBrowserTarget(this.#defaultContext);
     this.#contexts.push(this.#defaultContext);
 
     for (const [eventName, handler] of this.#connectionEventHandlers) {
@@ -168,7 +175,7 @@ export class Browser extends BrowserBase {
     }
     const target = !context.parent
       ? new BiDiPageTarget(browserContext, context)
-      : new BiDiTarget(browserContext, context);
+      : new BiDiBrowsingContextTarget(browserContext, context);
     this.#targets.set(event.context, target);
 
     this.emit(BrowserEmittedEvents.TargetCreated, target);
@@ -285,7 +292,7 @@ export class Browser extends BrowserBase {
   }
 
   override targets(): Target[] {
-    return Array.from(this.#targets.values());
+    return [this.#browserTarget, ...Array.from(this.#targets.values())];
   }
 
   _getTargetById(id: string): BiDiTarget {
@@ -295,6 +302,10 @@ export class Browser extends BrowserBase {
     }
     return target;
   }
+
+  override target(): Target {
+    return this.#browserTarget;
+  }
 }
 
 interface Options {

packages/puppeteer-core/src/common/bidi/Target.ts

@@ -25,54 +25,24 @@ import {Page} from './Page.js';
 
 export class BiDiTarget extends Target {
   protected _browserContext: BrowserContext;
-  protected _browsingContext: BrowsingContext;
 
-  constructor(
+  constructor(browserContext: BrowserContext) {
-    browserContext: BrowserContext,
-    browsingContext: BrowsingContext
-  ) {
     super();
-
     this._browserContext = browserContext;
-    this._browsingContext = browsingContext;
   }
 
   override async worker(): Promise<WebWorker | null> {
     return null;
   }
 
-  override url(): string {
-    return this._browsingContext.url;
-  }
-
-  /**
-   * Identifies what kind of target this is.
-   *
-   * @remarks
-   *
-   * See {@link https://developer.chrome.com/extensions/background_pages | docs} for more info about background pages.
-   */
-  override type(): TargetType {
-    return TargetType.PAGE;
-  }
-
-  /**
-   * Get the browser the target belongs to.
-   */
   override browser(): Browser {
-    throw new Error('Not implemented');
+    return this._browserContext.browser();
   }
 
-  /**
-   * Get the browser context the target belongs to.
-   */
   override browserContext(): BrowserContext {
     return this._browserContext;
   }
 
-  /**
-   * Get the target that opened this target. Top-level targets return `null`.
-   */
   override opener(): Target | undefined {
     throw new Error('Not implemented');
   }
@@ -80,10 +50,40 @@ export class BiDiTarget extends Target {
   _setBrowserContext(browserContext: BrowserContext): void {
     this._browserContext = browserContext;
   }
+}
+
+/**
+ * @internal
+ */
+export class BiDiBrowserTarget extends BiDiTarget {
+  override url(): string {
+    return '';
+  }
+
+  override type(): TargetType {
+    return TargetType.BROWSER;
+  }
+}
+
+/**
+ * @internal
+ */
+export class BiDiBrowsingContextTarget extends BiDiTarget {
+  protected _browsingContext: BrowsingContext;
+
+  constructor(
+    browserContext: BrowserContext,
+    browsingContext: BrowsingContext
+  ) {
+    super(browserContext);
+
+    this._browsingContext = browsingContext;
+  }
+
+  override url(): string {
+    return this._browsingContext.url;
+  }
 
-  /**
-   * Creates a Chrome Devtools Protocol session attached to the target.
-   */
   override async createCDPSession(): Promise<CDPSession> {
     const {sessionId} = await this._browsingContext.cdpSession.send(
       'Target.attachToTarget',
@@ -94,12 +94,16 @@ export class BiDiTarget extends Target {
     );
     return new CDPSessionWrapper(this._browsingContext, sessionId);
   }
+
+  override type(): TargetType {
+    return TargetType.PAGE;
+  }
 }
 
 /**
  * @internal
  */
-export class BiDiPageTarget extends BiDiTarget {
+export class BiDiPageTarget extends BiDiBrowsingContextTarget {
   #page: Page;
 
   constructor(

test/TestExpectations.json

@@ -3581,12 +3581,42 @@
     "parameters": ["cdp", "firefox"],
     "expectations": ["PASS"]
   },
+  {
+    "testIdPattern": "[target.spec] Target Browser.targets should return all of the targets",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["chrome", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
+  {
+    "testIdPattern": "[target.spec] Target Browser.waitForTarget should timeout waiting for a non-existent target",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["chrome", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
+  {
+    "testIdPattern": "[target.spec] Target Browser.waitForTarget should wait for a target",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["chrome", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
   {
     "testIdPattern": "[target.spec] Target Browser.waitForTarget should wait for a target",
     "platforms": ["darwin", "linux", "win32"],
     "parameters": ["cdp", "firefox"],
     "expectations": ["SKIP"]
   },
+  {
+    "testIdPattern": "[target.spec] Target should be able to use async waitForTarget",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["chrome", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
+  {
+    "testIdPattern": "[target.spec] Target should contain browser target",
+    "platforms": ["darwin", "linux", "win32"],
+    "parameters": ["chrome", "webDriverBiDi"],
+    "expectations": ["PASS"]
+  },
   {
     "testIdPattern": "[target.spec] Target should create a worker from a service worker",
     "platforms": ["darwin", "linux", "win32"],

test/src/target.spec.ts

@@ -79,11 +79,16 @@ describe('Target', function () {
 
     const [otherPage] = await Promise.all([
       context
-        .waitForTarget(target => {
+        .waitForTarget(
-          return target.page().then(page => {
+          target => {
-            return page!.url() === server.CROSS_PROCESS_PREFIX + '/empty.html';
+            return target.page().then(page => {
-          });
+              return (
-        })
+                page!.url() === server.CROSS_PROCESS_PREFIX + '/empty.html'
+              );
+            });
+          },
+          {timeout: 3000}
+        )
         .then(target => {
           return target.page();
         }),
@@ -101,9 +106,12 @@ describe('Target', function () {
 
     const [otherPage] = await Promise.all([
       context
-        .waitForTarget(target => {
+        .waitForTarget(
-          return target.url() === server.CROSS_PROCESS_PREFIX + '/empty.html';
+          target => {
-        })
+            return target.url() === server.CROSS_PROCESS_PREFIX + '/empty.html';
+          },
+          {timeout: 3000}
+        )
         .then(target => {
           return target.page();
         }),
@@ -167,10 +175,14 @@ describe('Target', function () {
 
     await page.goto(server.PREFIX + '/serviceworkers/empty/sw.html');
 
-    const target = await context.waitForTarget(target => {
+    const target = await context.waitForTarget(
-      return target.type() === 'service_worker';
+      target => {
-    });
+        return target.type() === 'service_worker';
+      },
+      {timeout: 3000}
+    );
     const worker = (await target.worker())!;
+
     expect(
       await worker.evaluate(() => {
         return self.toString();
@@ -184,9 +196,12 @@ describe('Target', function () {
     await page.evaluate(() => {
       new SharedWorker('data:text/javascript,console.log("hi")');
     });
-    const target = await context.waitForTarget(target => {
+    const target = await context.waitForTarget(
-      return target.type() === 'shared_worker';
+      target => {
-    });
+        return target.type() === 'shared_worker';
+      },
+      {timeout: 3000}
+    );
     const worker = (await target.worker())!;
     expect(
       await worker.evaluate(() => {
@@ -246,9 +261,12 @@ describe('Target', function () {
       server.waitForRequest('/one-style.css'),
     ]);
     // Connect to the opened page.
-    const target = await context.waitForTarget(target => {
+    const target = await context.waitForTarget(
-      return target.url().includes('one-style.html');
+      target => {
-    });
+        return target.url().includes('one-style.html');
+      },
+      {timeout: 3000}
+    );
     const newPage = (await target.page())!;
     // Issue a redirect.
     serverResponse.writeHead(302, {location: '/injectedstyle.css'});
@@ -278,9 +296,12 @@ describe('Target', function () {
       const {browser, server} = await getTestState();
 
       let resolved = false;
-      const targetPromise = browser.waitForTarget(target => {
+      const targetPromise = browser.waitForTarget(
-        return target.url() === server.EMPTY_PAGE;
+        target => {
-      });
+          return target.url() === server.EMPTY_PAGE;
+        },
+        {timeout: 3000}
+      );
       targetPromise
         .then(() => {
           return (resolved = true);