chore: reparent context objects into the object (#15689)

packages/playwright-core/src/client/browserContext.ts

@@ -74,7 +74,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
       this._browser = parent;
     this._isChromium = this._browser?._name === 'chromium';
     this.tracing = Tracing.from(initializer.tracing);
-    this.request = APIRequestContext.from(initializer.APIRequestContext);
+    this.request = APIRequestContext.from(initializer.requestContext);
 
     this._channel.on('bindingCall', ({ binding }) => this._onBinding(BindingCall.from(binding)));
     this._channel.on('close', () => this._onClose());

packages/playwright-core/src/client/channelOwner.ts

@@ -57,6 +57,12 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
     this._initializer = initializer;
   }
 
+  _adopt(child: ChannelOwner<any>) {
+    child._parent!._objects.delete(child._guid);
+    this._objects.set(child._guid, child);
+    child._parent = this;
+  }
+
   _dispose() {
     // Clean up from parent and connection.
     if (this._parent)

packages/playwright-core/src/client/connection.ts

@@ -143,16 +143,24 @@ export class Connection extends EventEmitter {
       this._createRemoteObject(guid, params.type, params.guid, params.initializer);
       return;
     }
+
+    const object = this._objects.get(guid);
+    if (!object)
+      throw new Error(`Cannot find object to "${method}": ${guid}`);
+
+    if (method === '__adopt__') {
+      const child = this._objects.get(params.guid);
+      if (!child)
+        throw new Error(`Unknown new child: ${params.guid}`);
+      object._adopt(child);
+      return;
+    }
+
     if (method === '__dispose__') {
-      const object = this._objects.get(guid);
-      if (!object)
-        throw new Error(`Cannot find object to dispose: ${guid}`);
       object._dispose();
       return;
     }
-    const object = this._objects.get(guid);
-    if (!object)
-      throw new Error(`Cannot find object to emit "${method}": ${guid}`);
+
     const validator = findValidator(object._type, method, 'Event');
     (object._channel as any).emit(method, validator(params, '', { tChannelImpl: this._tChannelImplFromWire.bind(this), binary: this.isRemote() ? 'fromBase64' : 'buffer' }));
   }
@@ -166,8 +174,10 @@ export class Connection extends EventEmitter {
   }
 
   private _tChannelImplFromWire(names: '*' | string[], arg: any, path: string, context: ValidatorContext) {
-    if (arg && typeof arg === 'object' && typeof arg.guid === 'string' && this._objects.has(arg.guid)) {
+    if (arg && typeof arg === 'object' && typeof arg.guid === 'string') {
       const object = this._objects.get(arg.guid)!;
+      if (!object)
+        throw new Error(`Object with guid ${arg.guid} was not bound in the connection`);
       if (names !== '*' && !names.includes(object._type))
         throw new ValidationError(`${path}: expected channel ${names.toString()}`);
       return object._channel;

packages/playwright-core/src/protocol/channels.ts

@@ -1207,7 +1207,7 @@ export interface EventTargetEvents {
 // ----------- BrowserContext -----------
 export type BrowserContextInitializer = {
   isChromium: boolean,
-  APIRequestContext: APIRequestContextChannel,
+  requestContext: APIRequestContextChannel,
   tracing: TracingChannel,
 };
 export interface BrowserContextEventTarget {

packages/playwright-core/src/protocol/protocol.yml

@@ -835,7 +835,7 @@ BrowserContext:
 
   initializer:
     isChromium: boolean
-    APIRequestContext: APIRequestContext
+    requestContext: APIRequestContext
     tracing: Tracing
 
   commands:

packages/playwright-core/src/protocol/validator.ts

@@ -644,7 +644,7 @@ scheme.ElectronApplicationWaitForEventInfoResult = tType('EventTargetWaitForEven
 scheme.AndroidDeviceWaitForEventInfoResult = tType('EventTargetWaitForEventInfoResult');
 scheme.BrowserContextInitializer = tObject({
   isChromium: tBoolean,
-  APIRequestContext: tChannel(['APIRequestContext']),
+  requestContext: tChannel(['APIRequestContext']),
   tracing: tChannel(['Tracing']),
 });
 scheme.BrowserContextBindingCallEvent = tObject({

packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts

@@ -39,12 +39,20 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   _type_BrowserContext = true;
   private _context: BrowserContext;
 
-  constructor(scope: DispatcherScope, context: BrowserContext) {
-    super(scope, context, 'BrowserContext', {
+  constructor(parentScope: DispatcherScope, context: BrowserContext) {
+    // We will reparent these to the context below.
+    const requestContext = APIRequestContextDispatcher.from(parentScope, context.fetchRequest);
+    const tracing = TracingDispatcher.from(parentScope, context.tracing);
+
+    super(parentScope, context, 'BrowserContext', {
       isChromium: context._browser.options.isChromium,
-      APIRequestContext: APIRequestContextDispatcher.from(scope, context.fetchRequest),
-      tracing: TracingDispatcher.from(scope, context.tracing),
+      requestContext,
+      tracing,
     }, true);
+
+    this.adopt(requestContext);
+    this.adopt(tracing);
+
     this._context = context;
     // Note: when launching persistent context, dispatcher is created very late,
     // so we can already have pages, videos and everything else.
@@ -52,7 +60,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
     const onVideo = (artifact: Artifact) => {
       // Note: Video must outlive Page and BrowserContext, so that client can saveAs it
       // after closing the context. We use |scope| for it.
-      const artifactDispatcher = new ArtifactDispatcher(scope, artifact);
+      const artifactDispatcher = new ArtifactDispatcher(parentScope, artifact);
       this._dispatchEvent('video', { artifact: artifactDispatcher });
     };
     this.addObjectListener(BrowserContext.Events.VideoStarted, onVideo);
@@ -94,8 +102,8 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
       page: PageDispatcher.fromNullable(this._scope, request.frame()?._page.initializedOrUndefined())
     }));
     this.addObjectListener(BrowserContext.Events.RequestFinished, ({ request, response }: { request: Request, response: Response | null }) => this._dispatchEvent('requestFinished', {
-      request: RequestDispatcher.from(scope, request),
-      response: ResponseDispatcher.fromNullable(scope, response),
+      request: RequestDispatcher.from(this._scope, request),
+      response: ResponseDispatcher.fromNullable(this._scope, response),
       responseEndTiming: request._responseEndTiming,
       page: PageDispatcher.fromNullable(this._scope, request.frame()?._page.initializedOrUndefined()),
     }));

packages/playwright-core/src/server/dispatchers/dispatcher.ts

@@ -88,6 +88,15 @@ export class Dispatcher<Type extends { guid: string }, ChannelType> extends Even
     this._eventListeners.push(eventsHelper.addEventListener(this._object as unknown as EventEmitter, eventName, handler));
   }
 
+  adopt(child: Dispatcher<any, any>) {
+    assert(this._isScope);
+    const oldParent = child._parent!;
+    oldParent._dispatchers.delete(child._guid);
+    this._dispatchers.set(child._guid, child);
+    child._parent = this;
+    this._connection.sendAdopt(this, child);
+  }
+
   _dispatchEvent<T extends keyof channels.EventsTraits<ChannelType>>(method: T, params?: channels.EventsTraits<ChannelType>[T]) {
     if (this._disposed) {
       if (isUnderTest())
@@ -100,7 +109,7 @@ export class Dispatcher<Type extends { guid: string }, ChannelType> extends Even
   }
 
   _dispose() {
-    assert(!this._disposed);
+    assert(!this._disposed, `${this._guid} is disposed more than once`);
     this._disposed = true;
     eventsHelper.removeEventListeners(this._eventListeners);
 
@@ -171,6 +180,10 @@ export class DispatcherConnection {
     this._sendMessageToClient(parent._guid, type, '__create__', { type, initializer, guid }, sdkObject);
   }
 
+  sendAdopt(parent: Dispatcher<any, any>, dispatcher: Dispatcher<any, any>) {
+    this._sendMessageToClient(parent._guid, dispatcher._type, '__adopt__', { guid: dispatcher._guid });
+  }
+
   sendDispose(dispatcher: Dispatcher<any, any>) {
     this._sendMessageToClient(dispatcher._guid, dispatcher._type, '__dispose__', {});
   }

packages/playwright-core/src/server/dispatchers/networkDispatchers.ts

@@ -15,7 +15,7 @@
  */
 
 import type * as channels from '../../protocol/channels';
-import { APIRequestContext } from '../fetch';
+import type { APIRequestContext } from '../fetch';
 import type { CallMetadata } from '../instrumentation';
 import type { Request, Response, Route } from '../network';
 import { WebSocket } from '../network';
@@ -172,14 +172,15 @@ export class APIRequestContextDispatcher extends Dispatcher<APIRequestContext, c
     return request ? APIRequestContextDispatcher.from(scope, request) : undefined;
   }
 
-  private constructor(scope: DispatcherScope, request: APIRequestContext) {
-    super(scope, request, 'APIRequestContext', {
-      tracing: TracingDispatcher.from(scope, request.tracing()),
+  private constructor(parentScope: DispatcherScope, request: APIRequestContext) {
+    // We will reparent these to the context below.
+    const tracing = TracingDispatcher.from(parentScope, request.tracing());
+
+    super(parentScope, request, 'APIRequestContext', {
+      tracing,
     }, true);
-    request.once(APIRequestContext.Events.Dispose, () => {
-      if (!this._disposed)
-        super._dispose();
-    });
+
+    this.adopt(tracing);
   }
 
   async storageState(params?: channels.APIRequestContextStorageStateParams): Promise<channels.APIRequestContextStorageStateResult> {

packages/playwright-core/src/server/dispatchers/pageDispatcher.ts

@@ -42,22 +42,29 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel> imple
   _type_Page = true;
   private _page: Page;
 
-  static fromNullable(scope: DispatcherScope, page: Page | undefined): PageDispatcher | undefined {
+  static fromNullable(parentScope: DispatcherScope, page: Page | undefined): PageDispatcher | undefined {
     if (!page)
       return undefined;
     const result = existingDispatcher<PageDispatcher>(page);
-    return result || new PageDispatcher(scope, page);
+    return result || new PageDispatcher(parentScope, page);
   }
 
-  constructor(scope: DispatcherScope, page: Page) {
+  constructor(parentScope: DispatcherScope, page: Page) {
     // TODO: theoretically, there could be more than one frame already.
     // If we split pageCreated and pageReady, there should be no main frame during pageCreated.
-    super(scope, page, 'Page', {
-      mainFrame: FrameDispatcher.from(scope, page.mainFrame()),
+
+    // We will reparent it to the page below using adopt.
+    const mainFrame = FrameDispatcher.from(parentScope, page.mainFrame());
+
+    super(parentScope, page, 'Page', {
+      mainFrame,
       viewportSize: page.viewportSize() || undefined,
       isClosed: page.isClosed(),
-      opener: PageDispatcher.fromNullable(scope, page.opener())
+      opener: PageDispatcher.fromNullable(parentScope, page.opener())
     }, true);
+
+    this.adopt(mainFrame);
+
     this._page = page;
     this.addObjectListener(Page.Events.Close, () => {
       this._dispatchEvent('close');
@@ -67,7 +74,8 @@ export class PageDispatcher extends Dispatcher<Page, channels.PageChannel> imple
     this.addObjectListener(Page.Events.Crash, () => this._dispatchEvent('crash'));
     this.addObjectListener(Page.Events.Dialog, dialog => this._dispatchEvent('dialog', { dialog: new DialogDispatcher(this._scope, dialog) }));
     this.addObjectListener(Page.Events.Download, (download: Download) => {
-      this._dispatchEvent('download', { url: download.url, suggestedFilename: download.suggestedFilename(), artifact: new ArtifactDispatcher(scope, download.artifact) });
+      // Artifact can outlive the page, so bind to the context scope.
+      this._dispatchEvent('download', { url: download.url, suggestedFilename: download.suggestedFilename(), artifact: new ArtifactDispatcher(parentScope, download.artifact) });
     });
     this.addObjectListener(Page.Events.FileChooser, (fileChooser: FileChooser) => this._dispatchEvent('fileChooser', {
       element: ElementHandleDispatcher.from(this._scope, fileChooser.element()),

packages/playwright-core/src/server/dispatchers/tracingDispatcher.ts

@@ -15,7 +15,7 @@
  */
 
 import type * as channels from '../../protocol/channels';
-import { Tracing } from '../trace/recorder/tracing';
+import type { Tracing } from '../trace/recorder/tracing';
 import { ArtifactDispatcher } from './artifactDispatcher';
 import type { DispatcherScope } from './dispatcher';
 import { Dispatcher, existingDispatcher } from './dispatcher';
@@ -30,7 +30,6 @@ export class TracingDispatcher extends Dispatcher<Tracing, channels.TracingChann
 
   constructor(scope: DispatcherScope, tracing: Tracing) {
     super(scope, tracing, 'Tracing', {}, true);
-    this.addObjectListener(Tracing.Events.Dispose, () => this._dispose());
   }
 
   async tracingStart(params: channels.TracingTracingStartParams): Promise<channels.TracingTracingStartResult> {

packages/playwright-core/src/server/fetch.ts

@@ -89,7 +89,7 @@ export abstract class APIRequestContext extends SdkObject {
   }
 
   constructor(parent: SdkObject) {
-    super(parent, 'fetchRequest');
+    super(parent, 'request-context');
     APIRequestContext.allInstances.add(this);
   }
 

packages/playwright-core/src/server/trace/recorder/tracing.ts

@@ -67,10 +67,6 @@ type RecordingState = {
 const kScreencastOptions = { width: 800, height: 600, quality: 90 };
 
 export class Tracing extends SdkObject implements InstrumentationListener, SnapshotterDelegate, HarTracerDelegate {
-  static Events = {
-    Dispose: 'dispose',
-  };
-
   private _writeChain = Promise.resolve();
   private _snapshotter?: Snapshotter;
   private _harTracer: HarTracer;
@@ -85,7 +81,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
   private _contextCreatedEvent: trace.ContextCreatedTraceEvent;
 
   constructor(context: BrowserContext | APIRequestContext, tracesDir: string | undefined) {
-    super(context, 'Tracing');
+    super(context, 'tracing');
     this._context = context;
     this._precreatedTracesDir = tracesDir;
     this._harTracer = new HarTracer(context, null, this, {
@@ -211,7 +207,6 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
 
   async dispose() {
     this._snapshotter?.dispose();
-    this.emit(Tracing.Events.Dispose);
   }
 
   async stopChunk(params: TracingTracingStopChunkParams): Promise<{ artifact: Artifact | null, sourceEntries: NameValue[] | undefined }> {

tests/library/channels.spec.ts

@@ -69,13 +69,14 @@ it('should scope context handles', async ({ browserType, server, expectScopeStat
       { _guid: 'browser-type', objects: [
         { _guid: 'browser', objects: [
           { _guid: 'browser-context', objects: [
-            { _guid: 'frame', objects: [] },
-            { _guid: 'page', objects: [] },
+            { _guid: 'page', objects: [
+              { _guid: 'frame', objects: [] },
+            ] },
             { _guid: 'request', objects: [] },
+            { _guid: 'request-context', objects: [] },
             { _guid: 'response', objects: [] },
+            { _guid: 'tracing', objects: [] }
           ] },
-          { _guid: 'fetchRequest', objects: [] },
-          { _guid: 'Tracing', objects: [] }
         ] },
       ] },
       { _guid: 'electron', objects: [] },
@@ -163,9 +164,10 @@ it('should scope browser handles', async ({ browserType, expectScopeState }) =>
       { _guid: 'browser-type', objects: [
         {
           _guid: 'browser', objects: [
-            { _guid: 'browser-context', objects: [] },
-            { _guid: 'fetchRequest', objects: [] },
-            { _guid: 'Tracing', objects: [] }
+            { _guid: 'browser-context', objects: [
+              { _guid: 'request-context', objects: [] },
+              { _guid: 'tracing', objects: [] },
+            ] },
           ]
         },
       ]