refactor: move context actions to the browser (#10621)

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

@@ -26,11 +26,16 @@ import {
 } from '../../api/Browser.js';
 import {BrowserContext as BrowserContextBase} from '../../api/BrowserContext.js';
 import {Page} from '../../api/Page.js';
-import {Target} from '../../puppeteer-core.js';
+import {Target} from '../../api/Target.js';
 import {Viewport} from '../PuppeteerViewport.js';
 
 import {BrowserContext} from './BrowserContext.js';
+import {
+  BrowsingContext,
+  BrowsingContextEmittedEvents,
+} from './BrowsingContext.js';
 import {Connection} from './Connection.js';
+import {BiDiPageTarget, BiDiTarget} from './Target.js';
 import {debugError} from './utils.js';
 
 /**
@@ -54,9 +59,6 @@ export class Browser extends BrowserBase {
     'cdp.Debugger.scriptParsed',
   ];
 
-  #browserName = '';
-  #browserVersion = '';
-
   static async create(opts: Options): Promise<Browser> {
     let browserName = '';
     let browserVersion = '';
@@ -83,18 +85,25 @@ export class Browser extends BrowserBase {
         : [...Browser.subscribeModules, ...Browser.subscribeCdpEvents],
     });
 
-    return new Browser({
+    const browser = new Browser({
       ...opts,
       browserName,
       browserVersion,
     });
+
+    await browser.#getTree();
+
+    return browser;
   }
 
+  #browserName = '';
+  #browserVersion = '';
   #process?: ChildProcess;
   #closeCallback?: BrowserCloseCallback;
   #connection: Connection;
   #defaultViewport: Viewport | null;
   #defaultContext: BrowserContext;
+  #targets = new Map<string, BiDiTarget>();
 
   constructor(
     opts: Options & {
@@ -118,8 +127,54 @@ export class Browser extends BrowserBase {
       defaultViewport: this.#defaultViewport,
       isDefault: true,
     });
+    this.#connection.on(
+      'browsingContext.contextCreated',
+      this.#onContextCreated
+    );
+    this.#connection.on(
+      'browsingContext.contextDestroyed',
+      this.#onContextDestroyed
+    );
   }
 
+  #onContextCreated = (
+    event: Bidi.BrowsingContext.ContextCreatedEvent['params']
+  ) => {
+    const context = new BrowsingContext(this.#connection, event);
+    this.#connection.registerBrowsingContexts(context);
+    // TODO: once more browsing context types are supported, this should be
+    // updated to support those. Currently, all top-level contexts are treated
+    // as pages.
+    const target = !context.parent
+      ? new BiDiPageTarget(this.defaultBrowserContext(), context)
+      : new BiDiTarget(this.defaultBrowserContext(), context);
+    this.#targets.set(event.context, target);
+
+    if (context.parent) {
+      const topLevel = this.#connection.getTopLevelContext(context.parent);
+      topLevel.emit(BrowsingContextEmittedEvents.Created, context);
+    }
+  };
+
+  async #getTree(): Promise<void> {
+    const {result} = await this.#connection.send('browsingContext.getTree', {});
+    for (const context of result.contexts) {
+      this.#onContextCreated(context);
+    }
+  }
+
+  #onContextDestroyed = async (
+    event: Bidi.BrowsingContext.ContextDestroyedEvent['params']
+  ) => {
+    const context = this.#connection.getBrowsingContext(event.context);
+    const topLevelContext = this.#connection.getTopLevelContext(event.context);
+    topLevelContext.emit(BrowsingContextEmittedEvents.Destroyed, context);
+    const target = this.#targets.get(event.context);
+    const page = await target?.page();
+    await page?.close().catch(debugError);
+    this.#targets.delete(event.context);
+  };
+
   get connection(): Connection {
     return this.#connection;
   }
@@ -129,6 +184,14 @@ export class Browser extends BrowserBase {
   }
 
   override async close(): Promise<void> {
+    this.#connection.off(
+      'browsingContext.contextDestroyed',
+      this.#onContextDestroyed
+    );
+    this.#connection.off(
+      'browsingContext.contextCreated',
+      this.#onContextCreated
+    );
     if (this.#connection.closed) {
       return;
     }
@@ -181,9 +244,15 @@ export class Browser extends BrowserBase {
   }
 
   override targets(): Target[] {
-    return this.browserContexts().flatMap(c => {
+    return Array.from(this.#targets.values());
-      return c.targets();
+  }
-    });
+
+  _getTargetById(id: string): BiDiTarget {
+    const target = this.#targets.get(id);
+    if (!target) {
+      throw new Error('Target not found');
+    }
+    return target;
   }
 }
 

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

@@ -14,18 +14,14 @@
  * limitations under the License.
  */
 
-import * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js';
-
 import {BrowserContext as BrowserContextBase} from '../../api/BrowserContext.js';
 import {Page as PageBase} from '../../api/Page.js';
 import {Target} from '../../api/Target.js';
-import {Deferred} from '../../util/Deferred.js';
 import {Viewport} from '../PuppeteerViewport.js';
 
 import {Browser} from './Browser.js';
 import {Connection} from './Connection.js';
 import {Page} from './Page.js';
-import {BiDiTarget} from './Target.js';
 import {debugError} from './utils.js';
 
 interface BrowserContextOptions {
@@ -40,9 +36,6 @@ export class BrowserContext extends BrowserContextBase {
   #browser: Browser;
   #connection: Connection;
   #defaultViewport: Viewport | null;
-  #targets = new Map<string, BiDiTarget>();
-  #onContextDestroyedBind = this.#onContextDestroyed.bind(this);
-  #init = Deferred.create<void>();
   #isDefault = false;
 
   constructor(browser: Browser, options: BrowserContextOptions) {
@@ -50,12 +43,7 @@ export class BrowserContext extends BrowserContextBase {
     this.#browser = browser;
     this.#connection = this.#browser.connection;
     this.#defaultViewport = options.defaultViewport;
-    this.#connection.on(
-      'browsingContext.contextDestroyed',
-      this.#onContextDestroyedBind
-    );
     this.#isDefault = options.isDefault;
-    this.#getTree().catch(debugError);
   }
 
   override targets(): Target[] {
@@ -77,49 +65,23 @@ export class BrowserContext extends BrowserContextBase {
     return this.#connection;
   }
 
-  async #getTree(): Promise<void> {
-    if (!this.#isDefault) {
-      this.#init.resolve();
-      return;
-    }
-    try {
-      const {result} = await this.#connection.send(
-        'browsingContext.getTree',
-        {}
-      );
-      for (const context of result.contexts) {
-        const page = new Page(this, context);
-        const target = new BiDiTarget(page.mainFrame().context(), page);
-        this.#targets.set(context.context, target);
-      }
-      this.#init.resolve();
-    } catch (err) {
-      this.#init.reject(err as Error);
-    }
-  }
-
-  async #onContextDestroyed(
-    event: Bidi.BrowsingContext.ContextDestroyedEvent['params']
-  ) {
-    const target = this.#targets.get(event.context);
-    const page = await target?.page();
-    await page?.close().catch(error => {
-      debugError(error);
-    });
-    this.#targets.delete(event.context);
-  }
-
   override async newPage(): Promise<PageBase> {
-    await this.#init.valueOrThrow();
-
     const {result} = await this.#connection.send('browsingContext.create', {
       type: 'tab',
     });
-    const page = new Page(this, {
+    const target = this.#browser._getTargetById(result.context);
-      context: result.context,
+
-      children: [],
+    // TODO: once BiDi has some concept matching BrowserContext, the newly
-    });
+    // created contexts should get automatically assigned to the right
-    const target = new BiDiTarget(page.mainFrame().context(), page);
+    // BrowserContext. For now, we assume that only explicitly created pages go
+    // to the current BrowserContext. Otherwise, the contexts get assigned to
+    // the default BrowserContext by the Browser.
+    target._setBrowserContext(this);
+
+    const page = await target.page();
+    if (!page) {
+      throw new Error('Page is not found');
+    }
     if (this.#defaultViewport) {
       try {
         await page.setViewport(this.#defaultViewport);
@@ -128,25 +90,20 @@ export class BrowserContext extends BrowserContextBase {
       }
     }
 
-    this.#targets.set(result.context, target);
-
     return page;
   }
 
   override async close(): Promise<void> {
-    await this.#init.valueOrThrow();
-
     if (this.#isDefault) {
       throw new Error('Default context cannot be closed!');
     }
 
-    for (const target of this.#targets.values()) {
+    for (const target of this.targets()) {
       const page = await target?.page();
       await page?.close().catch(error => {
         debugError(error);
       });
     }
-    this.#targets.clear();
   }
 
   override browser(): Browser {
@@ -154,9 +111,8 @@ export class BrowserContext extends BrowserContextBase {
   }
 
   override async pages(): Promise<PageBase[]> {
-    await this.#init.valueOrThrow();
     const results = await Promise.all(
-      [...this.#targets.values()].map(t => {
+      [...this.targets()].map(t => {
         return t.page();
       })
     );

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

@@ -106,6 +106,23 @@ export class CDPSessionWrapper extends EventEmitter implements CDPSession {
   }
 }
 
+/**
+ * Internal events that the BrowsingContext class emits.
+ *
+ * @internal
+ */
+export const BrowsingContextEmittedEvents = {
+  /**
+   * Emitted on the top-level context, when a descendant context is created.
+   */
+  Created: Symbol('BrowsingContext.created'),
+  /**
+   * Emitted on the top-level context, when a descendant context or the
+   * top-level context itself is destroyed.
+   */
+  Destroyed: Symbol('BrowsingContext.destroyed'),
+} as const;
+
 /**
  * @internal
  */
@@ -113,12 +130,14 @@ export class BrowsingContext extends Realm {
   #id: string;
   #url: string;
   #cdpSession: CDPSession;
+  #parent?: string | null;
 
   constructor(connection: Connection, info: Bidi.BrowsingContext.Info) {
     super(connection, info.context);
     this.connection = connection;
     this.#id = info.context;
     this.#url = info.url;
+    this.#parent = info.parent;
     this.#cdpSession = new CDPSessionWrapper(this);
 
     this.on(
@@ -141,6 +160,10 @@ export class BrowsingContext extends Realm {
     return this.#id;
   }
 
+  get parent(): string | undefined | null {
+    return this.#parent;
+  }
+
   get cdpSession(): CDPSession {
     return this.#cdpSession;
   }

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

@@ -246,6 +246,29 @@ export class Connection extends EventEmitter {
     this.#browsingContexts.set(context.id, context);
   }
 
+  getBrowsingContext(contextId: string): BrowsingContext {
+    const currentContext = this.#browsingContexts.get(contextId);
+    if (!currentContext) {
+      throw new Error(`BrowsingContext ${contextId} does not exist.`);
+    }
+    return currentContext;
+  }
+
+  getTopLevelContext(contextId: string): BrowsingContext {
+    let currentContext = this.#browsingContexts.get(contextId);
+    if (!currentContext) {
+      throw new Error(`BrowsingContext ${contextId} does not exist.`);
+    }
+    while (currentContext.parent) {
+      contextId = currentContext.parent;
+      currentContext = this.#browsingContexts.get(contextId);
+      if (!currentContext) {
+        throw new Error(`BrowsingContext ${contextId} does not exist.`);
+      }
+    }
+    return currentContext;
+  }
+
   unregisterBrowsingContexts(id: string): void {
     this.#browsingContexts.delete(id);
   }

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

@@ -53,7 +53,11 @@ import {
 
 import {Browser} from './Browser.js';
 import {BrowserContext} from './BrowserContext.js';
-import {BrowsingContext, CDPSessionWrapper} from './BrowsingContext.js';
+import {
+  BrowsingContext,
+  BrowsingContextEmittedEvents,
+  CDPSessionWrapper,
+} from './BrowsingContext.js';
 import {Connection} from './Connection.js';
 import {Frame} from './Frame.js';
 import {HTTPRequest} from './HTTPRequest.js';
@@ -69,7 +73,6 @@ import {BidiSerializer} from './Serializer.js';
 export class Page extends PageBase {
   #accessibility: Accessibility;
   #timeoutSettings = new TimeoutSettings();
-  #browserContext: BrowserContext;
   #connection: Connection;
   #frameTree = new FrameTree<Frame>();
   #networkManager: NetworkManager;
@@ -82,8 +85,6 @@ export class Page extends PageBase {
       'browsingContext.domContentLoaded',
       this.#onFrameDOMContentLoaded.bind(this),
     ],
-    ['browsingContext.contextCreated', this.#onFrameAttached.bind(this)],
-    ['browsingContext.contextDestroyed', this.#onFrameDetached.bind(this)],
     ['browsingContext.fragmentNavigated', this.#onFrameNavigated.bind(this)],
   ]) as Map<Bidi.Session.SubscriptionRequestEvent, Handler>;
   #networkManagerEvents = new Map<symbol, Handler<any>>([
@@ -108,29 +109,37 @@ export class Page extends PageBase {
       this.emit.bind(this, PageEmittedEvents.Response),
     ],
   ]);
+
+  #browsingContextEvents = new Map<symbol, Handler<any>>([
+    [BrowsingContextEmittedEvents.Created, this.#onContextCreated.bind(this)],
+    [
+      BrowsingContextEmittedEvents.Destroyed,
+      this.#onContextDestroyed.bind(this),
+    ],
+  ]);
   #tracing: Tracing;
   #coverage: Coverage;
   #emulationManager: EmulationManager;
   #mouse: Mouse;
   #touchscreen: Touchscreen;
   #keyboard: Keyboard;
+  #browsingContext: BrowsingContext;
+  #browserContext: BrowserContext;
 
   constructor(
-    browserContext: BrowserContext,
+    browsingContext: BrowsingContext,
-    info: Omit<Bidi.BrowsingContext.Info, 'url'> & {
+    browserContext: BrowserContext
-      url?: string;
-    }
   ) {
     super();
+    this.#browsingContext = browsingContext;
     this.#browserContext = browserContext;
-    this.#connection = browserContext.connection;
+    this.#connection = browsingContext.connection;
+
+    for (const [event, subscriber] of this.#browsingContextEvents) {
+      this.#browsingContext.on(event, subscriber);
+    }
 
     this.#networkManager = new NetworkManager(this.#connection, this);
-    this.#onFrameAttached({
-      ...info,
-      url: info.url ?? 'about:blank',
-      children: info.children ?? [],
-    });
 
     for (const [event, subscriber] of this.#subscribedEvents) {
       this.#connection.on(event, subscriber);
@@ -140,6 +149,15 @@ export class Page extends PageBase {
       this.#networkManager.on(event, subscriber);
     }
 
+    const frame = new Frame(
+      this,
+      this.#browsingContext,
+      this.#timeoutSettings,
+      this.#browsingContext.parent
+    );
+    this.#frameTree.addFrame(frame);
+    this.emit(PageEmittedEvents.FrameAttached, frame);
+
     // TODO: https://github.com/w3c/webdriver-bidi/issues/443
     this.#accessibility = new Accessibility(
       this.mainFrame().context().cdpSession
@@ -154,6 +172,10 @@ export class Page extends PageBase {
     this.#keyboard = new Keyboard(this.mainFrame().context());
   }
 
+  _setBrowserContext(browserContext: BrowserContext): void {
+    this.#browserContext = browserContext;
+  }
+
   override get accessibility(): Accessibility {
     return this.#accessibility;
   }
@@ -179,7 +201,7 @@ export class Page extends PageBase {
   }
 
   override browser(): Browser {
-    return this.#browserContext.browser();
+    return this.browserContext().browser();
   }
 
   override browserContext(): BrowserContext {
@@ -218,19 +240,16 @@ export class Page extends PageBase {
     }
   }
 
-  #onFrameAttached(info: Bidi.BrowsingContext.Info): void {
+  #onContextCreated(context: BrowsingContext): void {
     if (
-      !this.frame(info.context) &&
+      !this.frame(context.id) &&
-      (this.frame(info.parent ?? '') || !this.#frameTree.getMainFrame())
+      (this.frame(context.parent ?? '') || !this.#frameTree.getMainFrame())
     ) {
-      const context = new BrowsingContext(this.#connection, info);
-      this.#connection.registerBrowsingContexts(context);
-
       const frame = new Frame(
         this,
         context,
         this.#timeoutSettings,
-        info.parent
+        context.parent
       );
       this.#frameTree.addFrame(frame);
       this.emit(PageEmittedEvents.FrameAttached, frame);
@@ -250,8 +269,8 @@ export class Page extends PageBase {
     }
   }
 
-  #onFrameDetached(info: Bidi.BrowsingContext.Info): void {
+  #onContextDestroyed(context: BrowsingContext): void {
-    const frame = this.frame(info.context);
+    const frame = this.frame(context.id);
 
     if (frame) {
       if (frame === this.mainFrame()) {

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

@@ -24,40 +24,25 @@ import {BrowsingContext, CDPSessionWrapper} from './BrowsingContext.js';
 import {Page} from './Page.js';
 
 export class BiDiTarget extends Target {
-  #browsingContext: BrowsingContext;
+  protected _browserContext: BrowserContext;
-  #page: Page;
+  protected _browsingContext: BrowsingContext;
 
-  constructor(browsingContext: BrowsingContext, page: Page) {
+  constructor(
+    browserContext: BrowserContext,
+    browsingContext: BrowsingContext
+  ) {
     super();
 
-    this.#browsingContext = browsingContext;
+    this._browserContext = browserContext;
-    this.#page = page;
+    this._browsingContext = browsingContext;
   }
 
   override async worker(): Promise<WebWorker | null> {
     return null;
   }
 
-  override async page(): Promise<Page | null> {
-    return this.#page;
-  }
-
   override url(): string {
-    return this.#browsingContext.url;
+    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',
-      {
-        targetId: this.#page.mainFrame()._id,
-        flatten: true,
-      }
-    );
-    return new CDPSessionWrapper(this.#browsingContext, sessionId);
   }
 
   /**
@@ -82,7 +67,7 @@ export class BiDiTarget extends Target {
    * Get the browser context the target belongs to.
    */
   override browserContext(): BrowserContext {
-    throw new Error('Not implemented');
+    return this._browserContext;
   }
 
   /**
@@ -91,4 +76,47 @@ export class BiDiTarget extends Target {
   override opener(): Target | undefined {
     throw new Error('Not implemented');
   }
+
+  _setBrowserContext(browserContext: BrowserContext): void {
+    this._browserContext = browserContext;
+  }
+
+  /**
+   * Creates a Chrome Devtools Protocol session attached to the target.
+   */
+  override async createCDPSession(): Promise<CDPSession> {
+    const {sessionId} = await this._browsingContext.cdpSession.send(
+      'Target.attachToTarget',
+      {
+        targetId: this._browsingContext.id,
+        flatten: true,
+      }
+    );
+    return new CDPSessionWrapper(this._browsingContext, sessionId);
+  }
+}
+
+/**
+ * @internal
+ */
+export class BiDiPageTarget extends BiDiTarget {
+  #page: Page;
+
+  constructor(
+    browserContext: BrowserContext,
+    browsingContext: BrowsingContext
+  ) {
+    super(browserContext, browsingContext);
+
+    this.#page = new Page(browsingContext, browserContext);
+  }
+
+  override async page(): Promise<Page | null> {
+    return this.#page;
+  }
+
+  override _setBrowserContext(browserContext: BrowserContext): void {
+    super._setBrowserContext(browserContext);
+    this.#page._setBrowserContext(browserContext);
+  }
 }