refactor: introduce an internal PageTarget subclass (#10167)

docs/api/puppeteer.target.md

@@ -19,12 +19,12 @@ The constructor for this class is marked as internal. Third-party code should no
 ## Methods
 
 | Method                                                       | Modifiers | Description                                                                                                                                                          |
-| ------------------------------------------------------------ | --------- | ------------------------------------------------------------------------------------------------------------------------------------------ |
+| ------------------------------------------------------------ | --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | [browser()](./puppeteer.target.browser.md)                   |           | Get the browser the target belongs to.                                                                                                                               |
 | [browserContext()](./puppeteer.target.browsercontext.md)     |           | Get the browser context the target belongs to.                                                                                                                       |
 | [createCDPSession()](./puppeteer.target.createcdpsession.md) |           | Creates a Chrome Devtools Protocol session attached to the target.                                                                                                   |
 | [opener()](./puppeteer.target.opener.md)                     |           | Get the target that opened this target. Top-level targets return <code>null</code>.                                                                                  |
-| [page()](./puppeteer.target.page.md)                         |           | If the target is not of type <code>&quot;page&quot;</code> or <code>&quot;background_page&quot;</code>, returns <code>null</code>.         |
+| [page()](./puppeteer.target.page.md)                         |           | If the target is not of type <code>&quot;page&quot;</code>, <code>&quot;webview&quot;</code> or <code>&quot;background_page&quot;</code>, returns <code>null</code>. |
 | [type()](./puppeteer.target.type.md)                         |           | Identifies what kind of target this is.                                                                                                                              |
 | [url()](./puppeteer.target.url.md)                           |           |                                                                                                                                                                      |
 | [worker()](./puppeteer.target.worker.md)                     |           | If the target is not of type <code>&quot;service_worker&quot;</code> or <code>&quot;shared_worker&quot;</code>, returns <code>null</code>.                           |

docs/api/puppeteer.target.page.md

@@ -4,7 +4,7 @@ sidebar_label: Target.page
 
 # Target.page() method
 
-If the target is not of type `"page"` or `"background_page"`, returns `null`.
+If the target is not of type `"page"`, `"webview"` or `"background_page"`, returns `null`.
 
 #### Signature:
 

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

@@ -39,7 +39,7 @@ import {ChromeTargetManager} from './ChromeTargetManager.js';
 import {CDPSession, Connection, ConnectionEmittedEvents} from './Connection.js';
 import {FirefoxTargetManager} from './FirefoxTargetManager.js';
 import {Viewport} from './PuppeteerViewport.js';
-import {Target} from './Target.js';
+import {PageTarget, Target} from './Target.js';
 import {TargetManager, TargetManagerEmittedEvents} from './TargetManager.js';
 import {TaskQueue} from './TaskQueue.js';
 import {waitWithTimeout} from './util.js';
@@ -318,7 +318,8 @@ export class CDPBrowser extends BrowserBase {
       throw new Error('Missing browser context');
     }
 
-    return new Target(
+    if (this.#isPageTargetCallback(targetInfo)) {
+      return new PageTarget(
         targetInfo,
         session,
         context,
@@ -331,8 +332,20 @@ export class CDPBrowser extends BrowserBase {
         },
         this.#ignoreHTTPSErrors,
         this.#defaultViewport ?? null,
-      this.#screenshotTaskQueue,
+        this.#screenshotTaskQueue
-      this.#isPageTargetCallback
+      );
+    }
+    return new Target(
+      targetInfo,
+      session,
+      context,
+      this.#targetManager,
+      (isAutoAttachEmulated: boolean) => {
+        return this.#connection._createSession(
+          targetInfo,
+          isAutoAttachEmulated
+        );
+      }
     );
   };
 

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

@@ -16,7 +16,7 @@
 
 import {Protocol} from 'devtools-protocol';
 
-import type {Browser, IsPageTargetCallback} from '../api/Browser.js';
+import type {Browser} from '../api/Browser.js';
 import type {BrowserContext} from '../api/BrowserContext.js';
 import {Page, PageEmittedEvents} from '../api/Page.js';
 
@@ -25,6 +25,7 @@ import {CDPPage} from './Page.js';
 import {Viewport} from './PuppeteerViewport.js';
 import {TargetManager} from './TargetManager.js';
 import {TaskQueue} from './TaskQueue.js';
+import {debugError} from './util.js';
 import {WebWorker} from './WebWorker.js';
 
 /**
@@ -40,11 +41,7 @@ export class Target {
   #session?: CDPSession;
   #targetInfo: Protocol.Target.TargetInfo;
   #sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>;
-  #ignoreHTTPSErrors: boolean;
-  #defaultViewport?: Viewport;
-  #pagePromise?: Promise<Page>;
   #workerPromise?: Promise<WebWorker>;
-  #screenshotTaskQueue: TaskQueue;
 
   /**
    * @internal
@@ -65,15 +62,11 @@ export class Target {
   /**
    * @internal
    */
-  _isInitialized: boolean;
+  _isInitialized = false;
   /**
    * @internal
    */
   _targetId: string;
-  /**
-   * @internal
-   */
-  _isPageTargetCallback: IsPageTargetCallback;
 
   #targetManager: TargetManager;
 
@@ -85,11 +78,7 @@ export class Target {
     session: CDPSession | undefined,
     browserContext: BrowserContext,
     targetManager: TargetManager,
-    sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>,
+    sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>
-    ignoreHTTPSErrors: boolean,
-    defaultViewport: Viewport | null,
-    screenshotTaskQueue: TaskQueue,
-    isPageTargetCallback: IsPageTargetCallback
   ) {
     this.#session = session;
     this.#targetManager = targetManager;
@@ -97,37 +86,13 @@ export class Target {
     this.#browserContext = browserContext;
     this._targetId = targetInfo.targetId;
     this.#sessionFactory = sessionFactory;
-    this.#ignoreHTTPSErrors = ignoreHTTPSErrors;
-    this.#defaultViewport = defaultViewport ?? undefined;
-    this.#screenshotTaskQueue = screenshotTaskQueue;
-    this._isPageTargetCallback = isPageTargetCallback;
     this._initializedPromise = new Promise<boolean>(fulfill => {
       return (this._initializedCallback = fulfill);
-    }).then(async success => {
-      if (!success) {
-        return false;
-      }
-      const opener = this.opener();
-      if (!opener || !opener.#pagePromise || this.type() !== 'page') {
-        return true;
-      }
-      const openerPage = await opener.#pagePromise;
-      if (!openerPage.listenerCount(PageEmittedEvents.Popup)) {
-        return true;
-      }
-      const popupPage = await this.page();
-      openerPage.emit(PageEmittedEvents.Popup, popupPage);
-      return true;
     });
     this._isClosedPromise = new Promise<void>(fulfill => {
       return (this._closedCallback = fulfill);
     });
-    this._isInitialized =
+    this._initialize();
-      !this._isPageTargetCallback(this.#targetInfo) ||
-      this.#targetInfo.url !== '';
-    if (this._isInitialized) {
-      this._initializedCallback(true);
-    }
   }
 
   /**
@@ -137,6 +102,15 @@ export class Target {
     return this.#session;
   }
 
+  /**
+   * @internal
+   */
+  protected _sessionFactory(): (
+    isAutoAttachEmulated: boolean
+  ) => Promise<CDPSession> {
+    return this.#sessionFactory;
+  }
+
   /**
    * Creates a Chrome Devtools Protocol session attached to the target.
    */
@@ -158,28 +132,6 @@ export class Target {
     return this.#targetInfo;
   }
 
-  /**
-   * If the target is not of type `"page"` or `"background_page"`, returns `null`.
-   */
-  async page(): Promise<Page | null> {
-    if (this._isPageTargetCallback(this.#targetInfo) && !this.#pagePromise) {
-      this.#pagePromise = (
-        this.#session
-          ? Promise.resolve(this.#session)
-          : this.#sessionFactory(true)
-      ).then(client => {
-        return CDPPage._create(
-          client,
-          this,
-          this.#ignoreHTTPSErrors,
-          this.#defaultViewport ?? null,
-          this.#screenshotTaskQueue
-        );
-      });
-    }
-    return (await this.#pagePromise) ?? null;
-  }
-
   /**
    * If the target is not of type `"service_worker"` or `"shared_worker"`, returns `null`.
    */
@@ -271,15 +223,118 @@ export class Target {
    */
   _targetInfoChanged(targetInfo: Protocol.Target.TargetInfo): void {
     this.#targetInfo = targetInfo;
+    this._checkIfInitialized();
+  }
 
-    if (
+  /**
-      !this._isInitialized &&
+   * @internal
-      (!this._isPageTargetCallback(this.#targetInfo) ||
+   */
-        this.#targetInfo.url !== '')
+  protected _initialize(): void {
-    ) {
+    // TODO: refactor to deferred promises.
+    this._isInitialized = true;
+    if (this._isInitialized) {
+      this._initializedCallback(true);
+    }
+  }
+
+  /**
+   * @internal
+   */
+  protected _checkIfInitialized(): void {
+    if (!this._isInitialized) {
       this._isInitialized = true;
       this._initializedCallback(true);
       return;
     }
   }
+
+  /**
+   * If the target is not of type `"page"`, `"webview"` or `"background_page"`,
+   * returns `null`.
+   */
+  async page(): Promise<Page | null> {
+    return null;
+  }
+}
+
+/**
+ * @internal
+ */
+export class PageTarget extends Target {
+  #defaultViewport?: Viewport;
+  protected pagePromise?: Promise<Page>;
+  #screenshotTaskQueue: TaskQueue;
+  #ignoreHTTPSErrors: boolean;
+
+  /**
+   * @internal
+   */
+  constructor(
+    targetInfo: Protocol.Target.TargetInfo,
+    session: CDPSession | undefined,
+    browserContext: BrowserContext,
+    targetManager: TargetManager,
+    sessionFactory: (isAutoAttachEmulated: boolean) => Promise<CDPSession>,
+    ignoreHTTPSErrors: boolean,
+    defaultViewport: Viewport | null,
+    screenshotTaskQueue: TaskQueue
+  ) {
+    super(targetInfo, session, browserContext, targetManager, sessionFactory);
+    this.#ignoreHTTPSErrors = ignoreHTTPSErrors;
+    this.#defaultViewport = defaultViewport ?? undefined;
+    this.#screenshotTaskQueue = screenshotTaskQueue;
+  }
+
+  protected override _initialize(): void {
+    this._initializedPromise
+      .then(async success => {
+        if (!success) {
+          return false;
+        }
+        const opener = this.opener();
+        if (!(opener instanceof PageTarget)) {
+          return true;
+        }
+        if (!opener || !opener.pagePromise || this.type() !== 'page') {
+          return true;
+        }
+        const openerPage = await opener.pagePromise;
+        if (!openerPage.listenerCount(PageEmittedEvents.Popup)) {
+          return true;
+        }
+        const popupPage = await this.page();
+        openerPage.emit(PageEmittedEvents.Popup, popupPage);
+        return true;
+      })
+      .catch(debugError);
+    this._checkIfInitialized();
+  }
+
+  override async page(): Promise<Page | null> {
+    if (!this.pagePromise) {
+      const session = this._session();
+      this.pagePromise = (
+        session ? Promise.resolve(session) : this._sessionFactory()(true)
+      ).then(client => {
+        return CDPPage._create(
+          client,
+          this,
+          this.#ignoreHTTPSErrors,
+          this.#defaultViewport ?? null,
+          this.#screenshotTaskQueue
+        );
+      });
+    }
+    return (await this.pagePromise) ?? null;
+  }
+
+  override _checkIfInitialized(): void {
+    if (this._isInitialized) {
+      return;
+    }
+    this._isInitialized = this._getTargetInfo().url !== '';
+    if (this._isInitialized) {
+      this._initializedCallback(true);
+    }
+  }
 }